feat: implemented user order details #360
Conversation
defudef
commented
Apr 8, 2020
•
defudef
commented
- I'm confirming that if i made any changes to public APIs or exposed any public APIs they are doccumented.
defudef
changed the title
packages/core/theme-module/theme/pages/MyAccount/OrderDetails.vue
Outdated
Show resolved
Hide resolved
|
also coverage decreased |
|
|
||
| export const getOrderDate = (order: Order): string => order?.createdAt || ''; | ||
|
|
||
| export const getOrderId = (order: Order): string => order?.id || ''; | ||
|
|
||
| export const getOrderNumber = (order: Order): string => order?.orderNumber || getOrderId(order); |
There was a problem hiding this comment.
how that is different from ID? Did you made sure that this field is not specific to commercetools?
There was a problem hiding this comment.
A lot of ecommerce platforms have the ID and Order Number separately.
There was a problem hiding this comment.
export const getOrderId = (order: Order): string => order?.id || order?.orderNumber;
There was a problem hiding this comment.
does a lot mean all of them? keep in mind that helpers are for the UI, are both needed in the UI? Imho @andrzejewsky proposed right solution
|
|
||
| export const getOrderPrice = (order: Order): AgnosticPrice => getPrice(order?.totalPrice); | ||
|
|
||
| const transformAddressToString = (address: Address): string => ( |
There was a problem hiding this comment.
shouldn't that be a set of helpers OR just a non-agnostic object?
@andrzejewsky we've just discussed that
There was a problem hiding this comment.
It's temporary actually (just to keep DRY)
There was a problem hiding this comment.
if something is temporarly add an issue and todo here with issue linked
| }); | ||
|
|
||
| return { | ||
| cartGetters, |
There was a problem hiding this comment.
I'm not entirely sure about that so its up for discussion but maybe we should just have item getters in orderGetters are we sure that in every platform item from order is the same one as cart item? Also from the DX point of view It doesn't seem intuitive, for every other comparable you have a single corresponding getter
@defudef @andrzejewsky whats your opinion
There was a problem hiding this comment.
I think we need in this case orderItemGetters with OrderItem component. Unfortunately, there is no component in SFUI for that case
There was a problem hiding this comment.
I'm not sure I get it, why you want to pair component and getters?
There was a problem hiding this comment.
The problem is with a component that renders bought items:
SfCollectedProduct
v-for="product in orderGetters.getItems(order)"
:key="cartGetters.getItemSku(product)"
:image="cartGetters.getItemImage(product)"
:title="cartGetters.getItemName(product)"
:regular-price="'$' + cartGetters.getItemPrice(product).regular"
:qty="cartGetters.getItemQty(product)"
class="ordered-product"
/>
the orderGetters doesn't have methods that returns: SKU, image, or quantity
There was a problem hiding this comment.
RFC proposed here: #363
| getBillingAddress: (address: ORDER) => string; | ||
| getShippingAddress: (address: ORDER) => string; |
There was a problem hiding this comment.
would change that for implementation to decide about address structure
| getBillingAddress: (address: ORDER) => string; | |
| getShippingAddress: (address: ORDER) => string; | |
| getBillingAddress: (address: ORDER) => ADDRESS; | |
| getShippingAddress: (address: ORDER) => ADDRESS; |
There was a problem hiding this comment.
remove and create RFC, ideally before 4 m so we can discuss it on decision making window @defudef
