Marketplace: Refactor to turn Checkout into a Command #1062
Conversation
zspencer
requested review from
KellyAH,
anaulin and
a team
and removed request for
KellyAH
There was a problem hiding this comment.
OK, I think this gets us closer to a more reasonable design; without the weird cart/checkout/order trifecta.
I didn't inherit from Cart for Order and OrderedProduct from CartProduct (or vice versa ) but instead have them sharing the same database table...
I could imagine a world where the Cart class inherits from Order and adds the default scope; which would probably allow us to get rid of the CartProduct table and class definition as well; but I ran out of steam before getting htere.
| self.location_parent = :marketplace | ||
|
|
||
| default_scope { where(status: :pre_checkout) } |
There was a problem hiding this comment.
This ensures whenever we try and find a Cart it doesn't include anything that has been checked out.
| has_many :cart_products, dependent: :destroy, inverse_of: :cart | ||
| has_many :products, through: :cart_products, inverse_of: :carts | ||
| has_one :checkout, inverse_of: :cart | ||
| has_one :order, inverse_of: :cart |
There was a problem hiding this comment.
Now we don't need these anymore, since the Checkout table is dead! DEAD!
| } | ||
|
|
||
| def self.model_name | ||
| @_model_name ||= ActiveModel::Name.new(self, ::Marketplace) | ||
| end |
There was a problem hiding this comment.
I pulled this up into Marketplace::Record so all the Marketplace domain-objects don't need to define their own model-name.
| } | ||
| }, { | ||
| api_key: marketplace.stripe_api_key | ||
| }) | ||
| end | ||
|
|
||
| def complete(stripe_session_id:) | ||
| update!(status: :paid, stripe_session_id: stripe_session_id) | ||
| cart.update!(status: :checked_out) | ||
| cart.update!(status: :paid, stripe_session_id: stripe_session_id) |
| @@ -14,6 +13,9 @@ class Product < ApplicationRecord | |||
| has_many :cart_products, inverse_of: :product, dependent: :destroy | |||
| has_many :carts, through: :cart_products, inverse_of: :products | |||
|
|
|||
| has_many :ordered_products, inverse_of: :product, dependent: :destroy | |||
There was a problem hiding this comment.
This makes me think that maybe we don't want to be able to destroy Products that have been Ordered!
There was a problem hiding this comment.
- so we'll always have to keep a record of any products in the database if anyone has ordered them even if the product is removed from the Marketplace/is no longer for sale?
- Also have we decided on if products will be soft or hard deleted?
Guess that's okay. Shouldn't take up too much database space right?
There was a problem hiding this comment.
Yea, it would be scary if a Product getting destroyed the Orders OrderedProducts because then how would Vendors or Shoppers be able to see what was bought when?
There was a problem hiding this comment.
(Note: our hope is that Database costs are covered on a per-space basis; but I'm not entirely sure how we would go about doing that yet :D)
|
Wooops, the tests aren't passing jus a sec. |
#831 This "Demotes" `Checkout` from an Active Record and makes it serve more as a Command object than a Domain Model. It's still ActiveModel'y so it can be rendered using `render checkout` or routed to using `redirect_to checkout.location`, but otherwise it's not holding any data or what not.
zspencer
force-pushed
the
marketplace/eliminate-checkout-table
branch
from
397f8ef to
feba391
Compare
| has_many :products, through: :ordered_products, inverse_of: :orders | ||
|
|
||
| enum status: { | ||
| paid: "paid" |
There was a problem hiding this comment.
note: I can imagine more order statuses will need to be added once customers start using the feature. refunded, canceled, delivered and maybe some of the ones listed here.
But yeah let's Keep It Simple for now.
#831
This "Demotes"
Checkoutfrom an Active Record and makes it serve more as a Command object than a Domain Model. It's still ActiveModel'y so it can be rendered usingrender checkoutor routed to usingredirect_to checkout.location, but otherwise it's not holding any data or what not.This continues to allow us to leverage Rails' view-and-path-lookup ergonomics when rendering or redirecting to a
Marketplace::Cartor aMarketplace::Order; but also adds a touch of duplication now that the domain models don't share a common ancestor.