OBPIH-6267 Do not select edited lines to receive automatically in par…#4900
OBPIH-6267 Do not select edited lines to receive automatically in par…#4900awalkowiak merged 2 commits intodevelopfrom
Conversation
…tial receiving workflow
kchelstowski
requested review from
alannadolny,
awalkowiak,
drodzewicz and
ewaterman
github-actions
bot
added
the
domain: frontend
kchelstowski
added
the
warn: do not merge
kchelstowski
removed
the
warn: do not merge
alannadolny
left a comment
alannadolny
left a comment
There was a problem hiding this comment.
Looking at those changes, I am getting more and more confident that this should be refactored asap, because each of our changes is rather fixing using duct tape, not in the way it should be done.
| const shipmentItemsGrouped = shipmentItems.reduce((acc, item, idx) => { | ||
| if (idx < rowIndex) { | ||
| return { | ||
| ...acc, | ||
| itemsBeforeCurrentRow: [...acc.itemsBeforeCurrentRow, item], | ||
| }; | ||
| } | ||
| if (idx === rowIndex) { | ||
| return { | ||
| ...acc, | ||
| originalItem: item, | ||
| }; | ||
| } | ||
| return { | ||
| ...acc, | ||
| itemsAfterCurrentRow: [...acc.itemsAfterCurrentRow, item], | ||
| }; | ||
| }, { itemsBeforeCurrentRow: [], originalItem: null, itemsAfterCurrentRow: [] }); |
There was a problem hiding this comment.
I think you can simplify that code using _.groupBy from lodash
There was a problem hiding this comment.
I don't know how I would be able to use the _.groupBy from lodash here - from what I can see in the docs, I don't find it useful in that specific case, where I base the "key" on whether the index is lower, equal or higher than x.
I believe that this .reduce is quite readable, isn't it?
| static mapContainers(containers, parentIndex, rowIndex, items) { | ||
| return containers.map((container, idx) => { | ||
| if (idx === parentIndex) { | ||
| const { shipmentItems } = container; | ||
| return { | ||
| ...container, | ||
| shipmentItems: PartialReceivingPage.mapShipmentItems(shipmentItems, rowIndex, items), | ||
| }; | ||
| } | ||
| return container; | ||
| }); | ||
| } |
There was a problem hiding this comment.
I think it would be better to move this function to some utils instead of creating a static function ... static functions in React feels odd to me
There was a problem hiding this comment.
The reason why it is static is that the eslint was shouting that I don't use this keyword in a class method, so the options were either to make it static or to move it out of the component. I decided to go with the static, as there are already a few of them in this component.
I think a util is unnecessary here, because those methods are specifically targeting this component, I don't find it reusable anywhere else.
What do you think?
3 participants
…tial receiving workflow
✨ Description of Change
Link to GitHub issue or Jira ticket:
Description:
📷 Screenshots & Recordings (optional)