Skip to content

OBGM-564 items disappearing from pack and send page after editing revised qty#4143

Merged
awalkowiak merged 3 commits intofeature/upgrade-to-grails-3.3.10from
OBGM-564-items-disappearing-from-pack-and-send-page-after-editing-revised-qty
Jul 10, 2023
Merged

OBGM-564 items disappearing from pack and send page after editing revised qty#4143
awalkowiak merged 3 commits intofeature/upgrade-to-grails-3.3.10from
OBGM-564-items-disappearing-from-pack-and-send-page-after-editing-revised-qty

Conversation

@drodzewicz
Copy link
Collaborator

No description provided.

@drodzewicz drodzewicz self-assigned this Jul 6, 2023
@drodzewicz
Copy link
Collaborator Author

While investigating this issue there were many annoying eslint errors in the browser console so I decided to fix some of them.
Most of them were related to the defined proptypes that didn;t match the passed prop data


void createMissingShipmentItems(StockMovement stockMovement) {
Requisition requisition = stockMovement.requisition
Requisition requisition = stockMovement.requisition.refresh()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it took me a while to debug this problem until out of desperation I started to compare both OBGM and develop branch functions line by line, then I noticed that OBGM branch was missing the requisition.refresh()

void createMissingShipmentItems(StockMovement stockMovement) {
Requisition requisition = stockMovement.requisition.refresh()
if (requisition) {
Shipment shipment = Shipment.findByRequisition(requisition)
if (shipment && requisition.status >= RequisitionStatus.PICKED) {
createMissingShipmentItems(requisition, shipment)
if (shipment.hasErrors() || !shipment.save(flush: true)) {
throw new ValidationException("Invalid shipment", shipment.errors)
}
}
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, check if the recall and revert recall refresh the product availability with this refresh() here?

Copy link
Member

@jmiranda jmiranda Jul 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's missing, I think it was explicitly removed. Or maybe we used a Requisition.get() before. I think that would be fine.

Edit: Re-reading Artur's commit comment for OBGM-386 it seems like he explicitly removed the get() to avoid an issue with the session. I don't know what he encountered but we should probably discuss this in a tech huddle.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately reverting it back to Requisition.get(stockMovement.id) doesn't fix the issue with reviseItems.
I tested recall and product availability refresh with the refresh() and it seems to be working fine, everything seems to be updated immediately.
We could wait until tech huddle to discuss this case in more detail, but at this moment I don't have any other solution other than the one currently in the PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drodzewicz I'm fine with restoring the refresh(), but could you add also a comment in the code on why it is needed there?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drodzewicz Oh, now I know why it was changed there... The initial ticket/pr it was related to, was this one: #4117 where I got a suggestion from @jmiranda that having only stockMovement.requisition instead doing get might work, which I checked, and for simple item revising it was in the end working correctly, hence I decided to get rid of it in my then current ticket (which was not related to item revising).

Copy link
Collaborator

@kchelstowski kchelstowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as for those ESLint issues, we need to adjust settings to what was in Grails 1, because I agree it's annoying


void createMissingShipmentItems(StockMovement stockMovement) {
Requisition requisition = stockMovement.requisition
Requisition requisition = stockMovement.requisition.refresh()
Copy link
Member

@jmiranda jmiranda Jul 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's missing, I think it was explicitly removed. Or maybe we used a Requisition.get() before. I think that would be fine.

Edit: Re-reading Artur's commit comment for OBGM-386 it seems like he explicitly removed the get() to avoid an issue with the session. I don't know what he encountered but we should probably discuss this in a tech huddle.

@drodzewicz drodzewicz force-pushed the OBGM-564-items-disappearing-from-pack-and-send-page-after-editing-revised-qty branch 2 times, most recently from 98c0e7b to 60ef384 Compare July 10, 2023 07:35
@awalkowiak awalkowiak merged commit f4420d8 into feature/upgrade-to-grails-3.3.10 Jul 10, 2023
@awalkowiak awalkowiak deleted the OBGM-564-items-disappearing-from-pack-and-send-page-after-editing-revised-qty branch July 10, 2023 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants