search

FoxComm / highlander

Because there can only be one
MIT License
9 stars 3 forks source link

Merge order_shipping_addresses with addresses #2067

Open kjanosz opened 7 years ago

kjanosz commented 7 years ago

After the discussion in this PR we should merge order_shipping_addresses with addresses and make addresses versioned, so that order would point to specific version of an address.

aafa commented 7 years ago

AFAIK order_shipping_addresses is needed to safely store a copy to an address. I'd say, the best approach would be to merge two tables and have only addresses with orders referencing a "version" (copy) of an address. With something like parent_id maybe? (comment picked from here https://github.com/FoxComm/highlander/pull/2058)

@anna-zzz I wonder what are use cases for having order_shipping_addresses edited separately from addresses? From my shopping experience I can surely provide different addresses for shipping and billing, but once I choose to edit one of them, I presume that corrected version will appear everywhere and be consistent (and not versioned - why?). So the situation where I'm changing the shipping address but my "profile" address stays the same looks like a bug to me. But that's exactly what current implementation of PATCH shipping-address is doing. In order for the front-end to keep them in sync (as we need for Apple Pay now) front-end need to do several calls. Having isDefaultShipping and isDefaultBilling and versioned addresses will also complicate things.

upd. rephrased.

aafa commented 7 years ago

gonna go with versioning, but that still looks questionable to me.

before merging tables POST CreateAddressPayload - creates both address and order_shipping_addresses POST addressId - creates order_shipping_addresses from address PATCH UpdateAddressPayload - updates order_shipping_addresses only DELETE - deletes order_shipping_addresses for current user's cart

after merging tables POST CreateAddressPayload - creates address + bound to a cart POST addressId - bound addressId to a cart PATCH UpdateAddressPayload - updates address (for current user's cart) PUT CreateAddressPayload - creates or updates address (for current user's cart) DELETE - unbound from a cart (todo: check with front-end)

Upd. After having a talk with @jmataya on this we've come to the conclusion that we'd better settle with not having addresses versioned. One table of addresses to be referenced from orders and accounts.

annappropriate commented 7 years ago

Well, if we decided against versioning, changing address will also affect historical orders. Then we need to have two tables