OBGM-564 items disappearing from pack and send page after editing revised qty#4143
Conversation
|
While investigating this issue there were many annoying eslint errors in the browser console so I decided to fix some of them. |
|
|
||
| void createMissingShipmentItems(StockMovement stockMovement) { | ||
| Requisition requisition = stockMovement.requisition | ||
| Requisition requisition = stockMovement.requisition.refresh() |
There was a problem hiding this comment.
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()
openboxes/grails-app/services/org/pih/warehouse/inventory/StockMovementService.groovy
Lines 2348 to 2362 in 1e77b28
There was a problem hiding this comment.
Please, check if the recall and revert recall refresh the product availability with this refresh() here?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@drodzewicz I'm fine with restoring the refresh(), but could you add also a comment in the code on why it is needed there?
There was a problem hiding this comment.
@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).
kchelstowski
left a comment
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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.
98c0e7b to
60ef384
Compare
No description provided.