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

shopfloor: ensure that confirmation email is sent when moves are validated #53

Merged
merged 3 commits into from
Aug 27, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
13 changes: 13 additions & 0 deletions shopfloor/models/stock_move.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,16 @@ def split_other_move_lines(self, move_lines, intersection=False):
backorder_move._action_assign()
return backorder_move
return False

def _action_done(self, cancel_backorder=False):
# Overloaded to send the email when the last move of a picking is validated.
# The method 'stock.picking._send_confirmation_email' is called only from
# the 'stock.picking.action_done()' method but never when moves are
# validated partially through the current method.
moves = super()._action_done(cancel_backorder)
if self.env.context.get("_sf_send_confirmation_email"):
pickings = moves.picking_id
for picking in pickings:
if picking.state == "done":
picking._send_confirmation_email()
return moves
8 changes: 6 additions & 2 deletions shopfloor/services/location_content_transfer.py
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,9 @@ def set_destination_package(
# split the move to process only the lines related to the package.
package_move.split_other_move_lines(package_move_lines)
self._write_destination_on_lines(package_level.move_line_ids, scanned_location)
package_moves.with_context(_sf_no_backorder=True)._action_done()
package_moves.with_context(
_sf_no_backorder=True, _sf_send_confirmation_email=True
)._action_done()
move_lines = self._find_transfer_move_lines(location)
message = self.msg_store.location_content_transfer_item_complete(
scanned_location
Expand Down Expand Up @@ -645,7 +647,9 @@ def set_destination_line(
remaining_move_line.qty_done = remaining_move_line.product_uom_qty
move_line.move_id.split_other_move_lines(move_line)
self._write_destination_on_lines(move_line, scanned_location)
move_line.move_id.with_context(_sf_no_backorder=True)._action_done()
move_line.move_id.with_context(
_sf_no_backorder=True, _sf_send_confirmation_email=True
)._action_done()
move_lines = self._find_transfer_move_lines(location)
message = self.msg_store.location_content_transfer_item_complete(
scanned_location
Expand Down
3 changes: 2 additions & 1 deletion shopfloor/services/single_pack_transfer.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ def _set_destination_and_done(self, move, scanned_location):
# when writing the destination on the package level, it writes
# on the move lines
move.move_line_ids.package_level_id.location_dest_id = scanned_location
move._action_done()
# FIXME: '_sf_no_backorder' was not passed in the context, is it expected?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps. Maybe. I don't know.
Originally, this scenario was meant to move a picking with a single package, the behavior for other uses cases are undefined (to my knowledge).

move.with_context(_sf_send_confirmation_email=True)._action_done()

def cancel(self, package_level_id):
package_level = self.env["stock.package_level"].browse(package_level_id)
Expand Down
12 changes: 9 additions & 3 deletions shopfloor/services/zone_picking.py
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,9 @@ def _set_destination_location(
# if the move has other move lines, it is split to have only this move line
move_line.move_id.split_other_move_lines(move_line)
# set to done (without backorder)
move_line.move_id.with_context(_sf_no_backorder=True)._action_done()
move_line.move_id.with_context(
_sf_no_backorder=True, _sf_send_confirmation_email=True
)._action_done()
# try to re-assign any split move (in case of partial qty)
if "confirmed" in move_line.picking_id.move_lines.mapped("state"):
move_line.picking_id.action_assign()
Expand Down Expand Up @@ -1063,7 +1065,9 @@ def set_destination_all(
self._write_destination_on_lines(buffer_lines, location)
# set lines to done + refresh buffer lines (should be empty)
moves = buffer_lines.mapped("move_id")
moves.with_context(_sf_no_backorder=True)._action_done()
moves.with_context(
_sf_no_backorder=True, _sf_send_confirmation_email=True
)._action_done()
message = self.msg_store.buffer_complete()
buffer_lines = self._find_buffer_move_lines(zone_location, picking_type)
else:
Expand Down Expand Up @@ -1257,7 +1261,9 @@ def unload_set_destination(
self._write_destination_on_lines(buffer_lines, location)
# set lines to done + refresh buffer lines (should be empty)
moves = buffer_lines.mapped("move_id")
moves.with_context(_sf_no_backorder=True)._action_done()
moves.with_context(
_sf_no_backorder=True, _sf_send_confirmation_email=True
)._action_done()
buffer_lines = self._find_buffer_move_lines(zone_location, picking_type)
if buffer_lines:
return self._response_for_unload_single(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest import mock

from .test_location_content_transfer_base import LocationContentTransferCommonCase


Expand Down Expand Up @@ -130,14 +132,18 @@ def test_set_destination_package_dest_location_to_confirm(self):
def test_set_destination_package_dest_location_ok(self):
"""Scanned destination location valid, moves set to done."""
package_level = self.picking1.package_level_ids[0]
response = self.service.dispatch(
"set_destination_package",
params={
"location_id": self.content_loc.id,
"package_level_id": package_level.id,
"barcode": self.dest_location.barcode,
},
)
with mock.patch.object(
type(self.picking1), "_send_confirmation_email"
) as send_confirmation_email:
response = self.service.dispatch(
"set_destination_package",
params={
"location_id": self.content_loc.id,
"package_level_id": package_level.id,
"barcode": self.dest_location.barcode,
},
)
send_confirmation_email.assert_called_once()
move_lines = self.service._find_transfer_move_lines(self.content_loc)
self.assert_response_start_single(
response,
Expand Down Expand Up @@ -301,15 +307,19 @@ def test_set_destination_line_partial_qty(self):
self.assertEqual(move_line_c.move_id.state, "done")
# Scan remaining qty (4/10)
remaining_move_line_c = move_product_c_splitted.move_line_ids
response = self.service.dispatch(
"set_destination_line",
params={
"location_id": self.content_loc.id,
"move_line_id": remaining_move_line_c.id,
"quantity": remaining_move_line_c.product_uom_qty,
"barcode": self.dest_location.barcode,
},
)
with mock.patch.object(
type(self.picking2), "_send_confirmation_email"
) as send_confirmation_email:
response = self.service.dispatch(
"set_destination_line",
params={
"location_id": self.content_loc.id,
"move_line_id": remaining_move_line_c.id,
"quantity": remaining_move_line_c.product_uom_qty,
"barcode": self.dest_location.barcode,
},
)
send_confirmation_email.assert_not_called()
# Check move line data
self.assertEqual(remaining_move_line_c.move_id.product_uom_qty, 4)
self.assertEqual(remaining_move_line_c.product_uom_qty, 0)
Expand All @@ -329,20 +339,24 @@ def test_set_destination_line_partial_qty(self):
move_line_d = self.picking2.move_line_ids.filtered(
lambda m: m.product_id == self.product_d
)
response = self.service.dispatch(
"set_destination_line",
params={
"location_id": self.content_loc.id,
"move_line_id": move_line_d.id,
"quantity": move_line_d.product_uom_qty,
"barcode": self.dest_location.barcode,
},
)
self.assertEqual(move_line_d.move_id.product_uom_qty, 10)
self.assertEqual(move_line_d.product_uom_qty, 0)
self.assertEqual(move_line_d.qty_done, 10)
self.assertEqual(move_line_d.state, "done")
self.assertEqual(self.picking2.state, "done")
with mock.patch.object(
type(self.picking2), "_send_confirmation_email"
) as send_confirmation_email:
response = self.service.dispatch(
"set_destination_line",
params={
"location_id": self.content_loc.id,
"move_line_id": move_line_d.id,
"quantity": move_line_d.product_uom_qty,
"barcode": self.dest_location.barcode,
},
)
self.assertEqual(move_line_d.move_id.product_uom_qty, 10)
self.assertEqual(move_line_d.product_uom_qty, 0)
self.assertEqual(move_line_d.qty_done, 10)
self.assertEqual(move_line_d.state, "done")
self.assertEqual(self.picking2.state, "done")
send_confirmation_email.assert_called_once()


class LocationContentTransferSetDestinationXSpecialCase(
Expand Down Expand Up @@ -506,24 +520,32 @@ def test_set_destination_line_split_move(self):
remaining_move_lines = self.picking.move_line_ids_without_package.filtered(
lambda ml: ml.state == "assigned"
)
for ml in remaining_move_lines:
with mock.patch.object(
type(self.picking), "_send_confirmation_email"
) as send_confirmation_email:
for ml in remaining_move_lines:
self.service.dispatch(
"set_destination_line",
params={
"location_id": self.content_loc.id,
"move_line_id": ml.id,
"quantity": ml.product_uom_qty,
"barcode": self.dest_location.barcode,
},
)
self.assertEqual(self.picking.state, "assigned")
send_confirmation_email.assert_not_called()
package_level = self.picking.package_level_ids[0]
with mock.patch.object(
type(self.picking), "_send_confirmation_email"
) as send_confirmation_email:
self.service.dispatch(
"set_destination_line",
"set_destination_package",
params={
"location_id": self.content_loc.id,
"move_line_id": ml.id,
"quantity": ml.product_uom_qty,
"package_level_id": package_level.id,
"barcode": self.dest_location.barcode,
},
)
self.assertEqual(self.picking.state, "assigned")
package_level = self.picking.package_level_ids[0]
self.service.dispatch(
"set_destination_package",
params={
"location_id": self.content_loc.id,
"package_level_id": package_level.id,
"barcode": self.dest_location.barcode,
},
)
self.assertEqual(self.picking.state, "done")
self.assertEqual(self.picking.state, "done")
send_confirmation_email.assert_called_once()
81 changes: 50 additions & 31 deletions shopfloor/tests/test_single_pack_transfer.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest import mock

from odoo.tests.common import Form

from .common import CommonCase
Expand Down Expand Up @@ -397,13 +399,17 @@ def test_validate(self):

# now, call the service to proceed with validation of the
# movement
response = self.service.dispatch(
"validate",
params={
"package_level_id": package_level.id,
"location_barcode": self.shelf2.barcode,
},
)
with mock.patch.object(
type(self.picking), "_send_confirmation_email"
) as send_confirmation_email:
response = self.service.dispatch(
"validate",
params={
"package_level_id": package_level.id,
"location_barcode": self.shelf2.barcode,
},
)
send_confirmation_email.assert_called_once()

self.assert_response(
response,
Expand Down Expand Up @@ -465,13 +471,17 @@ def test_validate_completion_info(self):

# now, call the service to proceed with validation of the
# movement
response = self.service.dispatch(
"validate",
params={
"package_level_id": package_level.id,
"location_barcode": self.shelf2.barcode,
},
)
with mock.patch.object(
type(self.picking), "_send_confirmation_email"
) as send_confirmation_email:
response = self.service.dispatch(
"validate",
params={
"package_level_id": package_level.id,
"location_barcode": self.shelf2.barcode,
},
)
send_confirmation_email.assert_called_once()

self.assert_response(
response,
Expand Down Expand Up @@ -595,13 +605,17 @@ def test_validate_location_to_confirm(self):

# expected destination is 'shelf2', we'll scan shelf1 which must
# ask a confirmation to the user (it's still in the same picking type)
response = self.service.dispatch(
"validate",
params={
"package_level_id": package_level.id,
"location_barcode": self.shelf1.barcode,
},
)
with mock.patch.object(
type(self.picking), "_send_confirmation_email"
) as send_confirmation_email:
response = self.service.dispatch(
"validate",
params={
"package_level_id": package_level.id,
"location_barcode": self.shelf1.barcode,
},
)
send_confirmation_email.assert_not_called()

message = self.service.actions_for("message").confirm_location_changed(
self.shelf2, self.shelf1
Expand Down Expand Up @@ -641,15 +655,19 @@ def test_validate_location_with_confirm(self):

# expected destination is 'shelf1', we'll scan shelf2 which must
# ask a confirmation to the user (it's still in the same picking type)
response = self.service.dispatch(
"validate",
params={
"package_level_id": package_level.id,
"location_barcode": self.shelf2.barcode,
# acknowledge the change of destination
"confirmation": True,
},
)
with mock.patch.object(
type(self.picking), "_send_confirmation_email"
) as send_confirmation_email:
response = self.service.dispatch(
"validate",
params={
"package_level_id": package_level.id,
"location_barcode": self.shelf2.barcode,
# acknowledge the change of destination
"confirmation": True,
},
)
send_confirmation_email.assert_called_once()

self.assert_response(
response,
Expand Down Expand Up @@ -769,7 +787,8 @@ def test_cancel_already_done(self):
picking = move.picking_id

# someone cancel the work started by our operator
move._action_done()
# FIXME: '_sf_no_backorder' was not passed in the context, is it expected?
move.with_context(_sf_send_confirmation_email=True)._action_done()

# now, call the service to cancel
response = self.service.dispatch(
Expand Down
4 changes: 3 additions & 1 deletion shopfloor/tests/test_stock_split.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,9 @@ def test_split_pickings_from_source_location(self):
move_line.qty_done = move_line.product_uom_qty
if i % 2:
move_line.location_dest_id = dest_location
self.pick_move.with_context(_sf_no_backorder=True)._action_done()
self.pick_move.with_context(
_sf_no_backorder=True, _sf_send_confirmation_email=True
)._action_done()
guewen marked this conversation as resolved.
Show resolved Hide resolved
self.assertEqual(self.pick_move.state, "done")
# Pack step, we want to split move lines from common source location
self.assertEqual(self.pack_move.state, "assigned")
Expand Down
Loading