Skip to content
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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion shopfloor/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Shopfloor
!! This file is generated by oca-gen-addon-readme !!
!! changes will be overwritten. !!
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
!! source digest: sha256:f1d165021abc730bc8bf96092b9f76139e4b17c777975af4c9e496f86774bab6
!! source digest: sha256:b2c51fbfe37b326cc86b4856116825c4ccc2692b607ab630ac8e5a1e0baf5173
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

.. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png
Expand Down
2 changes: 1 addition & 1 deletion shopfloor/__manifest__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
{
"name": "Shopfloor",
"summary": "manage warehouse operations with barcode scanners",
"version": "14.0.4.3.1",
"version": "14.0.4.4.0",
"development_status": "Beta",
"category": "Inventory",
"website": "https://github.com/OCA/wms",
Expand Down
3 changes: 2 additions & 1 deletion shopfloor/data/shopfloor_scenario_data.xml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@
"no_prefill_qty": true,
"show_oneline_package_content": true,
"auto_post_line": true,
"scan_location_or_pack_first": true
"scan_location_or_pack_first": true,
"ask_for_leaf_destination_location" : true
}
</field>
</record>
Expand Down
41 changes: 41 additions & 0 deletions shopfloor/migrations/14.0.4.4.0/post-migration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Copyright 2023 Camptocamp SA (http://www.camptocamp.com)
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).

import json
import logging

from odoo import SUPERUSER_ID, api

_logger = logging.getLogger(__name__)


def migrate(cr, version):
if not version:
return
env = api.Environment(cr, SUPERUSER_ID, {})
checkout_scenario = env["shopfloor.scenario"].search([("key", "=", "checkout")])
_update_scenario_options(checkout_scenario)
checkout_menus = env["shopfloor.menu"].search(
[("scenario_id", "=", checkout_scenario.id)]
)
_enable_option_in_menus(checkout_menus)


def _update_scenario_options(scenario):
options = scenario.options
options["ask_for_leaf_destination_location"] = True
options_edit = json.dumps(options or {}, indent=4, sort_keys=True)
scenario.write({"options_edit": options_edit})
_logger.info(
"Option ask_for_leaf_destination_location added to the Checkout scenario"
)


def _enable_option_in_menus(menus):
for menu in menus:
menu.ask_for_leaf_destination_location = True
_logger.info(
"Option ask_for_leaf_destination_location enabled for menu {}".format(
menu.name
)
)
20 changes: 20 additions & 0 deletions shopfloor/models/shopfloor_menu.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@
to scan a destination package.
"""

ASK_FOR_LEAF_DESTINATION_LOCATION_HELP = """
When enabled, the destination location must be a leaf (location with no children)
location, if it is not, ask for scanning a child location of the destination.
"""


class ShopfloorMenu(models.Model):
_inherit = "shopfloor.menu"
Expand Down Expand Up @@ -226,6 +231,14 @@
allow_alternative_destination_package_is_possible = fields.Boolean(
compute="_compute_allow_alternative_destination_package_is_possible"
)
ask_for_leaf_destination_location = fields.Boolean(
string="Ask for leaf destination location",
default=False,
help=ASK_FOR_LEAF_DESTINATION_LOCATION_HELP,
)
ask_for_leaf_destination_location_is_possible = fields.Boolean(
compute="_compute_ask_for_leaf_destination_location_is_possible"
)

@api.onchange("unload_package_at_destination")
def _onchange_unload_package_at_destination(self):
Expand Down Expand Up @@ -455,3 +468,10 @@
menu.allow_alternative_destination_package_is_possible = (
menu.scenario_id.has_option("allow_alternative_destination_package")
)

@api.depends("scenario_id")
def _compute_ask_for_leaf_destination_location_is_possible(self):
for menu in self:
menu.ask_for_leaf_destination_location_is_possible = (

Check warning on line 475 in shopfloor/models/shopfloor_menu.py

View check run for this annotation

Codecov / codecov/patch

shopfloor/models/shopfloor_menu.py#L475

Added line #L475 was not covered by tests
menu.scenario_id.has_option("ask_for_leaf_destination_location")
)
13 changes: 7 additions & 6 deletions shopfloor/services/checkout.py
Original file line number Diff line number Diff line change
Expand Up @@ -1405,13 +1405,14 @@ def done(self, picking_id, confirmation=False):
)
lines_done = self._lines_checkout_done(picking)
dest_location = picking.location_dest_id
Copy link
Contributor

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

Copy link
Contributor

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:

  1. as said, we should take the destination location of the move
  2. 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)
  3. 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 🤔

Copy link
Contributor

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

child_locations = self.env["stock.location"].search(
[("id", "child_of", dest_location.id), ("usage", "!=", "view")]
)
if len(child_locations) > 0 and child_locations != dest_location:
return self._response_for_select_child_location(
picking,
if self.work.menu.ask_for_leaf_destination_location:
child_locations = self.env["stock.location"].search(
[("id", "child_of", dest_location.id), ("usage", "!=", "view")]
)
if len(child_locations) > 0 and child_locations != dest_location:
Copy link
Contributor

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?

Copy link
Contributor

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.

Copy link
Contributor

@jbaudoux jbaudoux Nov 14, 2023

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"

return self._response_for_select_child_location(
picking,
)
stock = self._actions_for("stock")
stock.validate_moves(lines_done.move_id)
return self._response_for_select_document(
Expand Down
2 changes: 1 addition & 1 deletion shopfloor/static/description/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ <h1 class="title">Shopfloor</h1>
!! This file is generated by oca-gen-addon-readme !!
!! changes will be overwritten. !!
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
!! source digest: sha256:f1d165021abc730bc8bf96092b9f76139e4b17c777975af4c9e496f86774bab6
!! source digest: sha256:b2c51fbfe37b326cc86b4856116825c4ccc2692b607ab630ac8e5a1e0baf5173
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->
<p><a class="reference external image-reference" href="https://odoo-community.org/page/development-status"><img alt="Beta" src="https://img.shields.io/badge/maturity-Beta-yellow.png" /></a> <a class="reference external image-reference" href="http://www.gnu.org/licenses/agpl-3.0-standalone.html"><img alt="License: AGPL-3" src="https://img.shields.io/badge/licence-AGPL--3-blue.png" /></a> <a class="reference external image-reference" href="https://github.com/OCA/wms/tree/14.0/shopfloor"><img alt="OCA/wms" src="https://img.shields.io/badge/github-OCA%2Fwms-lightgray.png?logo=github" /></a> <a class="reference external image-reference" href="https://translation.odoo-community.org/projects/wms-14-0/wms-14-0-shopfloor"><img alt="Translate me on Weblate" src="https://img.shields.io/badge/weblate-Translate%20me-F47D42.png" /></a> <a class="reference external image-reference" href="https://runboat.odoo-community.org/builds?repo=OCA/wms&amp;target_branch=14.0"><img alt="Try me on Runboat" src="https://img.shields.io/badge/runboat-Try%20me-875A7B.png" /></a></p>
<p>Shopfloor is a barcode scanner application for internal warehouse operations.</p>
Expand Down
17 changes: 17 additions & 0 deletions shopfloor/tests/test_checkout_done.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,28 @@ def test_done_partial(self):
)

def test_done_partial_confirm(self):
"""Check confirm partially done no check for leaf location."""
# lines are done
response = self.service.dispatch(
"done", params={"picking_id": self.picking.id, "confirmation": True}
)

self.assertRecordValues(self.picking, [{"state": "done"}])

self.assert_response(
response,
next_state="select_document",
message=self.service.msg_store.transfer_done_success(self.picking),
data={"restrict_scan_first": False},
)

def test_done_partial_confirm_ask_leaf_location(self):
"""Check confirm partially done with force leaf location option on."""
self.menu.sudo().ask_for_leaf_destination_location = True
response = self.service.dispatch(
"done", params={"picking_id": self.picking.id, "confirmation": True}
)

self.assertRecordValues(self.picking, [{"state": "assigned"}])

self.assert_response(
Expand Down
10 changes: 10 additions & 0 deletions shopfloor/views/shopfloor_menu.xml
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,16 @@
/>
<field name="allow_alternative_destination_package" />
</group>
<group
name="ask_for_leaf_destination_location"
attrs="{'invisible': [('ask_for_leaf_destination_location_is_possible', '=', False)]}"
>
<field
name="ask_for_leaf_destination_location_is_possible"
invisible="1"
/>
<field name="ask_for_leaf_destination_location" />
</group>
</group>
</field>
</record>
Expand Down
Loading