-
-
Notifications
You must be signed in to change notification settings - Fork 193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[14.0] shopfloor: add checkout option ask leaf location dest #775
[14.0] shopfloor: add checkout option ask leaf location dest #775
Conversation
155a506
to
f254bc4
Compare
f254bc4
to
e521bd2
Compare
Following this change on the checkout scenario OCA#618 A check has been added to force the user to scan a child location if the destination location is not a leaf in the locations tree. To allow for flexibility this change adds the options to disable this feature.
e521bd2
to
e08ad60
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need an option for this?
@@ -1405,13 +1405,14 @@ def done(self, picking_id, confirmation=False): | |||
) | |||
lines_done = self._lines_checkout_done(picking) | |||
dest_location = picking.location_dest_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be the move destination location
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is something fishy there:
- as said, we should take the destination location of the move
- but what move? we get all lines done of
picking
, so we could get several moves as well, each having a different destination location (e.g. a sub-location of the picking destination location thanks to a put-away) - so we should ask the user to scan a relevant leaf location for each move that targets a non-leaf location?
Right now this code is asking the user to scan a leaf location even if it's probably not necessary IMO 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should always scan a destination if the moves target different destinations
) | ||
if len(child_locations) > 0 and child_locations != dest_location: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we simply test if location_dest_id.usage == "view"
then we always need to choose a destination location?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Odoo std allows to put internal locations inside internal ones, so checking only usage == view
is not enough.
In such case we would need such option to decide what behavior we want for each Shopfloor menu.
Now I'm not sure about the current implementation of this feature, see my comment above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following sebalix remark that we are processing multiple moves, use this condition:
len(lines_done.location_dest_id) != 1 or lines_done.location_dest_id.usage == "view"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed with @sebalix . Looks like proposed changes should solve the issue without the need of a new option.
If Odoo allows to store products in stock locations that are not a leaf of the location structure, the shopfloor app should do the same. |
It's just that the option is currently providing a feature that nobody needs. So, let's not introduce more complexity. Adapting the |
Following on the comments here, I opened another PR #780 as the fix changes completely. |
Following this change on the checkout scenario:
A check has been added to force the user to scan a child location if the destination location is not a leaf in the locations tree.
To allow for flexibility this change adds the options to disable this feature.