[14.0][ADD] shopfloor_location_package_restriction #732

TDu · 2023-08-25T12:38:50Z

~~Needs : OCA/stock-logistics-warehouse#1797~~

This is not a good solution of the problem, needing 1 shopfloor module to nicely handle the errors/exception of another module.
A generic solution for this problem can be seen in #768

ref.: rau-200

OCA-git-bot · 2023-08-25T12:39:03Z

Hi @simahawk, @sebalix, @guewen,
some modules you are maintaining are being modified, check this out!

mmequignon · 2023-08-28T09:40:31Z

shopfloor/services/zone_picking.py

+        message = self._validate_destination(location, move_line.move_id)
+        if message:


looks like a handler to me 😉

Everything looks like a nail when you have a hammer.

jbaudoux

Instead of doing many glue modules for catching validation errors can't we catch any validation error, rollback the transaction and return the error ?
Here you solved the ValidationError from location_package_restriction, but there will be the same issue with location_product_restriction
cc @simahawk @lmignon

jbaudoux · 2023-09-06T07:44:01Z

shopfloor/services/zone_picking.py

-                error = self.msg_store.location_not_allowed()
-            if error:
-                return self._set_destination_all_response(buffer_lines, message=error)
+            if not message:


else: would be easier to read

You mean the next if clause ? But no because the first if could return a message ?

shopfloor/actions/message.py

jbaudoux · 2023-09-06T07:53:03Z

shopfloor/services/single_pack_transfer.py

+    def _validate_destination_location(self, package_level, location):
+        moves = package_level.move_line_ids.move_id
+        if not location:
+            return self._response_for_scan_location(
+                package_level, message=self.msg_store.no_location_found()
+            )
+        if not self.is_dest_location_valid(moves, location):
+            return self._response_for_scan_location(
+                package_level, message=self.msg_store.dest_location_not_allowed()
+            )
+        return None


Can't we move this to service level instead of repeating it in each service?

shopfloor/services/zone_picking.py

mmequignon · 2023-09-06T08:18:28Z

Instead of doing many glue modules for catching validation errors can't we catch any validation error, rollback the transaction and return the error ? Here you solved the ValidationError from location_package_restriction, but there will be the same issue with location_product_restriction cc @simahawk @lmignon

This is basically what I did here (except that I'm not looking for exceptions).
https://github.com/OCA/wms/blob/14.0/shopfloor_single_product_transfer/services/single_product_transfer.py#L18
Then you decorate your methods with this and you're good.
Not very flexible atm, but it can be improved for more generic needs.
For instance, we could pass a list of blocking message types and a list exceptions, which would trigger a rollback

@with_savepoint(blocking_types=["error"], blocking_exceptions=[UserError, ValidationError])
def my_endpoints(...)

jbaudoux · 2023-09-06T13:51:23Z

Instead of doing many glue modules for catching validation errors can't we catch any validation error, rollback the transaction and return the error ? Here you solved the ValidationError from location_package_restriction, but there will be the same issue with location_product_restriction cc @simahawk @lmignon

This is basically what I did here (except that I'm not looking for exceptions). https://github.com/OCA/wms/blob/14.0/shopfloor_single_product_transfer/services/single_product_transfer.py#L18 Then you decorate your methods with this and you're good. Not very flexible atm, but it can be improved for more generic needs. For instance, we could pass a list of blocking message types and a list exceptions, which would trigger a rollback
@with_savepoint(blocking_types=["error"], blocking_exceptions=[UserError, ValidationError])
def my_endpoints(...)

How do you build properly the response to pass all the right arguments ?

mmequignon · 2023-09-06T14:28:24Z

Instead of doing many glue modules for catching validation errors can't we catch any validation error, rollback the transaction and return the error ? Here you solved the ValidationError from location_package_restriction, but there will be the same issue with location_product_restriction cc @simahawk @lmignon

This is basically what I did here (except that I'm not looking for exceptions). https://github.com/OCA/wms/blob/14.0/shopfloor_single_product_transfer/services/single_product_transfer.py#L18 Then you decorate your methods with this and you're good. Not very flexible atm, but it can be improved for more generic needs. For instance, we could pass a list of blocking message types and a list exceptions, which would trigger a rollback
@with_savepoint(blocking_types=["error"], blocking_exceptions=[UserError, ValidationError])
def my_endpoints(...)
How do you build properly the response to pass all the right arguments ?

I'm not sure to understand your question.
Let's have a call about that.

TDu · 2023-10-17T11:18:20Z

Instead of doing many glue modules for catching validation errors can't we catch any validation error, rollback the transaction and return the error ? Here you solved the ValidationError from location_package_restriction, but there will be the same issue with location_product_restriction cc @simahawk @lmignon

I have opened this PR #768 that implements a way to handle any ValidationError when handling a response.

It could replace this one

mmequignon · 2023-10-27T08:46:25Z

Instead of doing many glue modules for catching validation errors can't we catch any validation error, rollback the transaction and return the error ? Here you solved the ValidationError from location_package_restriction, but there will be the same issue with location_product_restriction cc @simahawk @lmignon

I have opened this PR #768 that implements a way to handle any ValidationError when handling a response.

It could replace this one

We're still missing what was the requirement in this case:
Such as, rollback in case of error, or error management.

TDu changed the title ~~[14.0]~~ [14.0][ADD] shopfloor_location_package_restriction Aug 25, 2023

mmequignon reviewed Aug 28, 2023

View reviewed changes

TDu force-pushed the 14-sf-stock-location-pack-glue branch from c413c86 to 68614aa Compare August 29, 2023 06:54

jbaudoux requested changes Sep 6, 2023

View reviewed changes

TDu force-pushed the 14-sf-stock-location-pack-glue branch from 7453be2 to 282efe9 Compare September 8, 2023 07:22

TDu force-pushed the 14-sf-stock-location-pack-glue branch from 282efe9 to d9a3123 Compare October 13, 2023 09:56

TDu added 4 commits October 13, 2023 14:51

shopfloor: zp, spt refactor to help iheritance

f3340b1

Add shopfloor_location_package_restriction

95e9097

fixup! Add shopfloor_location_package_restriction

1b3f064

fixup! shopfloor: zp, spt refactor to help iheritance

65d4134

TDu force-pushed the 14-sf-stock-location-pack-glue branch from d9a3123 to 65d4134 Compare October 13, 2023 12:51

fixup! fixup! Add shopfloor_location_package_restriction

7dd4e47

TDu closed this Nov 10, 2023

TDu mentioned this pull request Nov 10, 2023

[14.0][IMP] shopfloor: add decorator to gracefully handle exceptions on response #768

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[14.0][ADD] shopfloor_location_package_restriction #732

[14.0][ADD] shopfloor_location_package_restriction #732

TDu commented Aug 25, 2023 •

edited

Loading

OCA-git-bot commented Aug 25, 2023

mmequignon Aug 28, 2023

TDu Aug 29, 2023

mmequignon Sep 6, 2023

jbaudoux left a comment

jbaudoux Sep 6, 2023

TDu Sep 7, 2023

jbaudoux Sep 6, 2023

TDu Oct 13, 2023

mmequignon commented Sep 6, 2023

jbaudoux commented Sep 6, 2023

mmequignon commented Sep 6, 2023

TDu commented Oct 17, 2023

mmequignon commented Oct 27, 2023

		message = self._validate_destination(location, move_line.move_id)
		if message:

[14.0][ADD] shopfloor_location_package_restriction #732

[14.0][ADD] shopfloor_location_package_restriction #732

Conversation

TDu commented Aug 25, 2023 • edited Loading

OCA-git-bot commented Aug 25, 2023

mmequignon Aug 28, 2023

Choose a reason for hiding this comment

TDu Aug 29, 2023

Choose a reason for hiding this comment

mmequignon Sep 6, 2023

Choose a reason for hiding this comment

jbaudoux left a comment

Choose a reason for hiding this comment

jbaudoux Sep 6, 2023

Choose a reason for hiding this comment

TDu Sep 7, 2023

Choose a reason for hiding this comment

jbaudoux Sep 6, 2023

Choose a reason for hiding this comment

TDu Oct 13, 2023

Choose a reason for hiding this comment

mmequignon commented Sep 6, 2023

jbaudoux commented Sep 6, 2023

mmequignon commented Sep 6, 2023

TDu commented Oct 17, 2023

mmequignon commented Oct 27, 2023

TDu commented Aug 25, 2023 •

edited

Loading