OBGM-517 Fix duplicating items on create putaway workflow#4159
Conversation
| private Putaway bindPutawayData(Putaway putaway, User currentUser, Location currentLocation, JSONObject jsonObject) { | ||
| // Bind the putaway | ||
| bindData(putaway, jsonObject) | ||
| bindData(putaway, jsonObject, [exclude: ['putawayItems']]) |
There was a problem hiding this comment.
in general lgtm, but I'm wondering what the difference in Grails 1 is, that it was working there? Did you check that and have the answer?
There was a problem hiding this comment.
this is not the first time I came across this issue.
Before in grails 1 it didn't bind deeply nested items, which is why later in the code we manually iterate through jsonObject.putawayItems and bind them ourselves.
Grails 3 apparently does handle properly binding nested objects, hence why there appears a problem of duplicated items: first set is from implicit binding and the other is from manual binding.
So we have to choose one or the other.
I chose to exclude the implicit binding and stick to the manual to avoid unnecessary regression.
There was a problem hiding this comment.
Actually, I take back what I said.
In the end, I decided on using the grails data binding for the whole data object, because I didn't notice that the same problem was also on split items list of putaway item, and excluding the nested values and handling them manually is counterproductive.
I tested it to see if everything is bound properly and it seems to be working well.
…n updated putaway
|
|
||
| order.save(failOnError: true) | ||
| order.save(failOnError: true, flush: true) | ||
| return order |
There was a problem hiding this comment.
can you try just to return the line that saves? so to do:
return order.save(failOnError: true)calling save and then below returning the order (the not persisted one) feels weird to me
Try to do it without flush and let me know if it is working, as this flush should be unnecessary here
There was a problem hiding this comment.
Unfortunately, it does not solve the problem.
the newly added split item is still not saved at that point and so when we return the putaway after updating, it does not contain the newly added split item.
There was a problem hiding this comment.
What should help is removing @transactional from the controller. Opening the transaction at controller level makes the transaction being commited not at the service level (so your changes made in service are not passed to the controller), but it is commited after you call render (so you pass not yet flushed data to the frontend).
What should be also visible is that you should see your changes "with a delay" - what I mean by that, is that when you try to do adding the splitItems twice, at the second attempt, you would suddenly see the changes from the first attempt.
There was a problem hiding this comment.
Thanks, @kchelstowski for the explanation, that solution you proposed did help.
|
|
||
| putaway.putawayItems.add(putawayItem) | ||
| } | ||
|
|
There was a problem hiding this comment.
if you had made sure the binder handles that 2nd level nested (splitItemMap) cases, LGTM
No description provided.