[16.0][FIX] shopfloor: Fetch qty_available only when needed #1055
Conversation
Several services use `_data_move_line` to prepare stock move line data for the Shopfloor frontend. However, qty_available is not displayed on all pages where stock move lines are handled. Since qty_available is computed from quants, accessing it unnecessarily can lead to useless database queries. In high-concurrency environments, this can even trigger deadlocks when reads and writes occur simultaneously across many Shopfloor users. This fix limits the use of qty_available to cases where it’s strictly needed.
sbejaoui
changed the title
lmignon
left a comment
lmignon
left a comment
There was a problem hiding this comment.
Do you've analyzed the domain used in such a case. The qty available is computed for a specific location. I find it hard to believe that there are several people at the same time asking for the same quantity of the same product for the same location... In any case, if you don't need the field because you don't display it, it's time saved.
The quantity is not relevant; the issue is related to the same product from the same location, and it occurs consistently based on the data I analyzed in our use case. |
|
This PR has the |
| data["product"]["qty_available"] = product.with_context( | ||
| location=line.location_id.id | ||
| ).qty_available | ||
| if "no_qty_available" not in kw or not kw.get("no_qty_available"): |
There was a problem hiding this comment.
why not simply this?
| if "no_qty_available" not in kw or not kw.get("no_qty_available"): | |
| if not kw.get("no_qty_available"): |
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
github-actions
bot
added
the
stale
| if "no_qty_available" not in kw or not kw.get("no_qty_available"): | ||
| data["product"]["qty_available"] = product.with_context( | ||
| location=line.location_id.id | ||
| ).qty_available |
There was a problem hiding this comment.
Is it possible to avoid double negations ? not no_qty_available is not really intuitive to read.
For instance, explicitely add a named argument with_qty_available which defaults to True.
When you don't need this information in the data, we can just set with_qty_available=False.
Then you can do this, which IMO is easier to read. and less bug prone.
| if "no_qty_available" not in kw or not kw.get("no_qty_available"): | |
| data["product"]["qty_available"] = product.with_context( | |
| location=line.location_id.id | |
| ).qty_available | |
| if with_qty_available: | |
| data["product"]["qty_available"] = product.with_context( | |
| location=line.location_id.id | |
| ).qty_available |
What do you think ?
github-actions
bot
removed
the
stale
7 participants
Several services use
_data_move_lineto prepare stock move line data for the Shopfloor frontend. However, qty_available is not displayed on all pages where stock move lines are handled.Since qty_available is computed from quants, accessing it unnecessarily can lead to useless database queries. In high-concurrency environments, this can even trigger deadlocks when reads and writes occur simultaneously across many Shopfloor users.
This fix limits the use of qty_available to cases where it’s strictly needed.
@lmignon , @rousseldenis