-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add/amazon order route #1492
Add/amazon order route #1492
Conversation
e611ee3
to
1428b1a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, except couple tiny notes
ashes/src/modules/orders/details.js
Outdated
@@ -18,6 +18,11 @@ const _getOrder = createAsyncActions( | |||
(refNum: string) => Api.get(`/orders/${refNum}`) | |||
); | |||
|
|||
const _getAmazonOrder = createAsyncActions( | |||
'getAmazonOrder', | |||
(refNum: string) => Api.get(`/hyperion/orders/111-5296499-9653858/full`) // ${refNum} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hardcoded value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
ashes/src/modules/orders/details.js
Outdated
export const updateOrder = _updateOrder.perform; | ||
export const updateShipments = _updateShipments.perform; | ||
export const clearFetchErrors =_getOrder.clearErrors; | ||
|
||
function orderSucceeded(state: State, payload: Object): State { | ||
const order: OrderParagon = new OrderParagon(payload.result || payload); | ||
const res = payload.result || payload; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this condition can be deleted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
and i think we should add a link to |
@@ -77,19 +80,29 @@ const setCellContents = (order, field) => { | |||
} | |||
}; | |||
|
|||
type Props = { | |||
order: OrderParagon; | |||
columns: Array<*>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have Columns
in interface/table.js
👍 |
Design
What was done
[Invalid]
text with value as a fallback in State component.What is not done here