🥗🧹 Marketplace: Delivery constraints live on DeliveryArea
#1420
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zspencer
force-pushed
the
marketplace/delivery/migrate-delivery-info-off-marketplace
branch
3 times, most recently
from
2b5efd5
to
fede865
Compare
zspencer
added a commit
that referenced
this pull request
Apr 30, 2023
zspencer
force-pushed
the
marketplace/delivery/migrate-delivery-info-off-marketplace
branch
from
fede865
to
19fa3a2
Compare
zspencer
changed the base branch from
main
to
marketplace/wire-carts-and-delivery-areas-together
zspencer
changed the title
Marketplace: Delivery constraints live on DeliveryAreaMarketplace: Delivery constraints live on DeliveryArea
zspencer
force-pushed
the
marketplace/delivery/migrate-delivery-info-off-marketplace
branch
from
19fa3a2
to
83318a4
Compare
zspencer
added
🧹 refactor
zspencer
added a commit
that referenced
this pull request
Apr 30, 2023
zspencer
force-pushed
the
marketplace/wire-carts-and-delivery-areas-together
branch
from
1fe029a
to
07cbf20
Compare
zspencer
force-pushed
the
marketplace/delivery/migrate-delivery-info-off-marketplace
branch
from
83318a4
to
7daecf4
Compare
anaulin
approved these changes
May 4, 2023
Base automatically changed from
marketplace/wire-carts-and-delivery-areas-together
to
main
May 4, 2023 00:35
anaulin
reviewed
May 4, 2023
| def initialize(cart:, order_by: cart.marketplace.order_by, | ||
| delivery_window: cart.marketplace.delivery_window, **kwargs) | ||
| def initialize(cart:, order_by: cart.delivery_area&.order_by, | ||
| delivery_window: cart.delivery_area&.delivery_window, **kwargs) |
There was a problem hiding this comment.
Is this ever used with a different order_by and delivery_window passed in? It feels like this could be tightened to just accept cart.
Not part of your changes, just thinking out loud.
There was a problem hiding this comment.
That would be nice! I don't remember exactly why I had done it this way the first time; but I think it was because I was using component previews or something to test.
zspencer
force-pushed
the
marketplace/delivery/migrate-delivery-info-off-marketplace
branch
from
7daecf4
to
11f48ef
Compare
zspencer
deleted the
marketplace/delivery/migrate-delivery-info-off-marketplace
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Marketplace:Delivery#1325Marketplace:DeliveryArea#1136Marketplace:DeliveryConstraints #1185Allowing a
Marketplaceto specify it's delivery fees, constraints, etc. and aDeliveryAreato override them was putting us in a bit of a mess from "how does data even?!"This consolidates everything onto
DeliveryArea, yay!