Create shop orders #272

MatheusBuss · 2022-08-04T01:33:05Z

What issue does this PR close

Closes #198

Changes Proposed ( a list of new changes introduced by this PR)

Create shop orders abstraction

MatheusBuss · 2022-08-04T01:41:23Z

@lucca65 criei um contexto novo pra orders. Mas acho que deveria ter jogado ele pra dentro do shop mesmo. Acho que fica mais organizado. Daí teria que reconstruir o arquivo lib/cambiatus/shop/order.ex.

lucca65 · 2022-08-04T23:12:05Z

+1 bro, vamos manter td no shop!

lucca65 · 2022-08-12T00:14:05Z

lib/cambiatus/orders.ex

+    |> join(:left, [o], b in assoc(o, :buyer))
+    |> where([o, b], b.account == ^buyer.account)


can't we simply preload the buyer? Something like this:

Suggested change

|> join(:left, [o], b in assoc(o, :buyer))

|> where([o, b], b.account == ^buyer.account)

|> Repo.preload(:buyer)

I don't think so. Because this is getting an order where the status is cart and belongs to the current user. Since we're getting the order from these SQL statements I don't believe it's possible to preload the buyer.

lib/cambiatus/orders.ex

lib/cambiatus/orders/item.ex

lucca65 · 2022-08-12T00:25:05Z

lib/cambiatus/orders/order.ex

+    with order_id <- get_field(changeset, :id),
+         {:ok, order} <- Orders.get_order(order_id) do
+      if Map.get(order, :status) == "cart" and get_field(changeset, :status) == "checkout" do


This could be piped

Suggested change

with order_id <- get_field(changeset, :id),

{:ok, order} <- Orders.get_order(order_id) do

if Map.get(order, :status) == "cart" and get_field(changeset, :status) == "checkout" do

order_status =

changeset

|> get_field(:id)

|> Orders.get()

|> Map.get(:status)

if Enum.member(["cart", "checkout"], order_status) do

. . .

The intent of this one is to check if the order stored in the db has the status cart and the incoming changeset is changing this status to checkout. Then we should make some more validations to ensure that everything is valid before the user is has the chance to make a payment.

This has also changed a bit. If you want to make another review it'll be very welcome.

This cart I've added a few comments about state machines, but we should leverage its structure to make sure its valid, instead of relying on manual verification.

If an order is on a certain state, we should trust our backend ingested it properly so its correct to have this state. This is an old article and probably outdated, but it contain precious concepts about this type of abstraction: https://medium.com/@joaomdmoura/state-machine-in-elixir-with-machinery-8ee6f9def2da

lucca65 · 2022-08-12T00:27:26Z

I know its a draft, some general directions 😄

lucca65 · 2022-08-16T15:31:56Z

lib/cambiatus/shop/order.ex

+    )
+
+    field(:total, :float)
+    field(:status, :string)


Maybe we need to review possible states bro. Can you implement this as an Ecto.Enum? This way we can talk more about control flow and state changes. I recommend you take a look at state machines, as this will be a nice way to think about this abstractions. A gist of it could be something like

states: created, processed, approved, completed
actions: new_draft (makes a new one), approve (created → approved or processed → approved, pay (no state change), archive (any → completed

this is just an example on how to approach an order state as its heavily implied that it will be treated different depending on it. This means we will have specific actions that will change state. Makes easier to understand and admin

I was thinking about elaborating states. If you want we can have a more thorough discussion about states on a meeting of some kind, maybe even with other teams to get the right flow.

But I was thinking something like this:
1 - Cart: There's no more than one order with this status for each user
2 - Checkout:Validate everything that was put on the cart, since availability may have changed since adding to cart
3 - Awaiting payment confirmation: This should be a brief state, just to ensure that the items on checkout have been payed for but we're making sure that everything went through
4 - Delivering: This could change name or encompass other steps. Some orders may need preparation after payment but before beign archive/completed
5 - Completed: End step for every order, maybe there should be another end step if something goes wrong along the way

yeah, you are totally right, nailing the states will go a long way for us, migrating types is a pain in the ass so the better we design our states, the best it is.

I haven't thought about adding cart as an state, this is a good idea. I would add other auxiliary states as well. Cancelled by vendor, cancelled by buyer (maybe by not paying during the accepted window?) payment denied and finally payment incomplete (this is a must as the main motivation to building orders is to allow multiple payments on the same order)

+1 on scheduling a meeting. We should do it between ourselves first to build a first draft and then present it to the rest of the team to gather feedback

lucca65 · 2022-08-16T15:34:32Z

priv/repo/migrations/20220803194923_create_orders.exs

+
+      add(:total, :float, comment: "Order total")
+
+      # TODO: Elaborate shipping and status


Shipping will be an entity that we will attach on later, we can do a preview of states but don't worry about tables/logic for shipping

lucca65 · 2022-08-16T15:35:07Z

priv/repo/migrations/20220803140956_rename_orders_to_items.exs

+  use Ecto.Migration
+
+  def change do
+    rename(table(:orders), to: table(:items))


Since every order until now has been a single product purchase it is probably easier to treat them as items, since most of the information is being contained within the items table. Then we can create the related orders with some info on the buyer.

We can always do it the other way as well, shouldn't be too different.

hmm, Idk. We will still need to major abstraction of orders, which is the main one, that can start with a product_id as a reference to the product and later on can be migrate into an intermediary table order_product_id.

About the majority of the information, it should be on orders, not on items.

The order table could be scoped like this:

id

product_id

from

to

amount

units

community

blockchainstuff

We could refactor it like this:

id

product_id (from can be inferred from product, the same with community)

buyer

amount (can be different from product due to discounts cupons and other stuff so we should keep it)

units

timestamps

Later we could trim it even further by adding a middle table

Order

id

buyer

timestamps

Order_items

order_id (Primary key)

product_id (Primary key)

Order_payments

id

type (enum with something like btc/paypal/venmo/eth/etc)

status (created/confirmed/rejected/etc)

amount

price_modifier_id (maybe a certain payment method is cheaper or more expensive than others. Crypto has no fees, as PayPal do)

Then we could go to other attached tables order_shipping, order_refunds, etc

We should try to go for the higher level abstraction first, then add the auxiliary items. It will make our migration harder, but we can go in small steps as described in the beginning of the comment

First implementation of shop orders

466ebb7

Created order and item factories

5c1c4f3

MatheusBuss self-assigned this Aug 10, 2022

MatheusBuss added the ✨ feature A new feature label Aug 10, 2022

MatheusBuss added 2 commits August 11, 2022 10:01

Created shopping cart and vinculated order to buyer

13f778f

Merge branch 'master' into feature/shop-orders

387390a

lucca65 reviewed Aug 12, 2022

View reviewed changes

MatheusBuss added 4 commits August 15, 2022 15:27

Added validation to check if order has items

f5db052

Added item validation to orders

7116ab4

Moved items and orders to the shop context

66c77b1

Removed add_current user to query TODO's

3f1bff6

lucca65 reviewed Aug 16, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create shop orders #272

Create shop orders #272

MatheusBuss commented Aug 4, 2022

MatheusBuss commented Aug 4, 2022

lucca65 commented Aug 4, 2022

lucca65 Aug 12, 2022

MatheusBuss Aug 16, 2022

lucca65 Aug 12, 2022

MatheusBuss Aug 16, 2022

lucca65 Aug 16, 2022

lucca65 commented Aug 12, 2022

lucca65 Aug 16, 2022

lucca65 Aug 16, 2022

MatheusBuss Aug 16, 2022

lucca65 Aug 16, 2022

lucca65 Aug 16, 2022

lucca65 Aug 16, 2022

MatheusBuss Aug 16, 2022

lucca65 Aug 16, 2022 •

edited

Loading

		\|> join(:left, [o], b in assoc(o, :buyer))
		\|> where([o, b], b.account == ^buyer.account)

	\|> join(:left, [o], b in assoc(o, :buyer))
	\|> where([o, b], b.account == ^buyer.account)
	\|> Repo.preload(:buyer)

-    with order_id <- get_field(changeset, :id),
-         {:ok, order} <- Orders.get_order(order_id) do
-      if Map.get(order, :status) == "cart" and get_field(changeset, :status) == "checkout" do
+order_status =
+    changeset
+    |> get_field(:id)
+    |> Orders.get()
+    |> Map.get(:status)
+    if Enum.member(["cart", "checkout"], order_status) do
+      . . .


		add(:total, :float, comment: "Order total")

		# TODO: Elaborate shipping and status

Create shop orders #272

Are you sure you want to change the base?

Create shop orders #272

Conversation

MatheusBuss commented Aug 4, 2022

What issue does this PR close

Changes Proposed ( a list of new changes introduced by this PR)

MatheusBuss commented Aug 4, 2022

lucca65 commented Aug 4, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucca65 commented Aug 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucca65 Aug 16, 2022 • edited Loading

Choose a reason for hiding this comment

lucca65 Aug 16, 2022 •

edited

Loading