[MIG] sale_exception: Migration to 16.0 #2266

matiasperalta1 · 2022-11-17T14:19:19Z

No description provided.

Rule should only count available stock from selected warehouse from sale order. Without warehouse in context, it currently sums all stock available in all warehouses.

The previous parent menu has an action attached, and with these modules, it can't be clicked.

* [FIX] sale_exception confirm wizard * [FIX] add test of test_all_draft_orders & flake8 & readme.rst * [FIX] improve codecov test * [FIX] delete odl translation * [FIX] add Camptocamp to author * [FIX] readme.rst

Raise SingletonError when you have a recordset with multiple sale order. This restore the capability to write on multiple sale.order

Currently, it's located at the root menu because. It's a migration bug because the old menu was removed from 11.0.

… sale order line

The method called by 'sale_check_exception' has a side effect, it writes on 'exception.rule' + on the Many2many relation between it and sale.order(.line). When decorated by @api.constrains, any error during the method will be caught and re-raised as "ValidationError". This part of code is very prone to concurrent updates as 2 sales having the same exception will both write on the same 'exception.rule'. A concurrent update (OperationalError) is re-raised as ValidationError, and then is not retried properly. Calling the same method in create/write has the same effect than @api.constrains without shadowing the exception type. Full explanation: OCA/server-tools#1642

…ce (OCA#963)

* [IMP] sale_exception: show menu only to Exception Rule Managers * [IMP] Reindent lines to 4 spaces * [IMP] Improve information messages that display exceptions * [REM] Remove exception_ids fields from the sale order view, because it's no longer necessary and it allowed users to accidentaly deactivate a Sale Exception Rule using the active widget. * [IMP] Rename exception_ids_formatted to exceptions_summary

Disable for now as the solution is not obvious. Partially fixes OCA#972

Other ways it provokes linter fail

TestSaleOrder was useless and making sale order tests run twice.

ghost · 2022-11-21T07:46:51Z

sale_exception/models/sale.py

+        if check_exceptions:
+            self.sale_check_exception()
+
+    @api.model


Suggested change

@api.model

@api.model_create_multi

Keep in mind that you should also adapt the method _check_sale_check_exception. It will receive a list of dict from the create method and only an dict from the write method.

aliciagaarzo

After testing the migrated module, I’ve found some improvements:

Description field is noy properly displayed; user needs to scroll to the right to see it.
Allow to ignore exceptions even if the “is blocking” option is checked

sebastienbeau · 2022-12-30T13:34:28Z

sale_exception/models/sale.py

@@ -53,11 +53,12 @@ def _check_sale_check_exception(self, vals):
        if check_exceptions:
            self.sale_check_exception()

-    @api.model


Hi thanks for the migration.

It's not related to your change but after think twice
I think that the check on create is useless !

Indeed if you look at the method sale_check_exception, it explicitly skip all sale order that are not in the state "sale", when we create a sale order the state is always "draft".

So indeed of adding some extra complexity to the code, maybe it's a good opportunity to simplify it and just to remove the inherit on "def create".

I should have seen it before but I miss to review this PR : #916

@simahawk @rousseldenis @sbejaoui what do you think ?

@florian-dacosta

I agree the create override can be removed as it seems useless.

sebastienbeau · 2022-12-30T13:35:54Z

also, can you rebase as base_exception have been merged ?

sebastienbeau · 2022-12-30T13:45:44Z

Note: I also realize that the migration in V9 have drop the commit historic (it was correct in 8 see here: https://github.com/OCA/sale-workflow/commits/8.0?after=605d0ae2da6d37e9acd8466eb77d2cc023d2f289+69&branch=8.0&path%5B%5D=sale_exceptions&qualified_name=refs%2Fheads%2F8.0)

When this PR will be really, I will, before merging the PR, restore the commit history, so please ping me when it's ready, do not waist your time with this point, I will do it.

rousseldenis · 2023-01-02T09:50:53Z

Note: I also realize that the migration in V9 have drop the commit historic (it was correct in 8 see here: https://github.com/OCA/sale-workflow/commits/8.0?after=605d0ae2da6d37e9acd8466eb77d2cc023d2f289+69&branch=8.0&path%5B%5D=sale_exceptions&qualified_name=refs%2Fheads%2F8.0)

@sebastienbeau That's normal as module has been renamed at that time.

rousseldenis · 2023-01-02T09:54:30Z

/ocabot migration sale_exception

rousseldenis

LGTM. Minor change

rousseldenis · 2023-01-02T09:56:54Z

sale_exception/models/sale.py

+from odoo import api, fields, models
+
+
+class ExceptionRule(models.Model):


Please put models in separate files

rousseldenis · 2023-01-02T10:07:26Z

sale_exception/tests/test_multi_records.py

@@ -0,0 +1,125 @@
+# License AGPL-3 - See http://www.gnu.org/licenses/agpl-3.0.html
+
+import mock


Suggested change

import mock

from unittest import mock

lmignon

LGTM once the requested changes will be implemented

rousseldenis · 2023-01-11T08:45:14Z

@matiasperalta1

lmignon · 2023-01-17T13:50:26Z

@matiasperalta1 Do you plan to work on this PR soon or do you prefer we finalize your work? We need to move forward with this one....

sbejaoui · 2023-01-25T09:50:28Z

I pushed a new PR (#2344) to finalize this one because we need it for one of our projects.

rousseldenis · 2023-01-25T15:19:03Z

@matiasperalta1 Could you close this in favor of #2344 ?

atchuthan and others added 30 commits November 16, 2022 16:34

[ADD] sale_exception

54ae331

[FIX] sale_exception: README

dffe199

[FIX] sale_exception: configuration menu

9161a02

[IMP] Rename action_cancel to action_draft to reset the flag

faf2bf4

[FIX] sale_exception: count stock from selected warehouse

92b2e3d

Rule should only count available stock from selected warehouse from sale order. Without warehouse in context, it currently sums all stock available in all warehouses.

sale_*: Fix parent menu

050aff9

The previous parent menu has an action attached, and with these modules, it can't be clicked.

[FIX] add dependency to server-tools/base_exception

3d11acd

[FIX] sale_exception misc

ca2476c

* [FIX] sale_exception confirm wizard * [FIX] add test of test_all_draft_orders & flake8 & readme.rst * [FIX] improve codecov test * [FIX] delete odl translation * [FIX] add Camptocamp to author * [FIX] readme.rst

[FIX] sale_exception constraint

4b1c56d

Raise SingletonError when you have a recordset with multiple sale order. This restore the capability to write on multiple sale.order

[11.0][MIG] sale_exception

c0de66e

sale_exception: recover translations from v10

6d83b65

[FIX] sale_exception: Menu location (OCA#719)

784721a

Currently, it's located at the root menu because. It's a migration bug because the old menu was removed from 11.0.

Sale Exception to v12

3f63edd

[MIG] sale_exception: Migration to 12.0

87607dd

[IMP] sale_exception: format XML file 'views/sale_view.xml'

7f6698b

Adapt sale_exception to base_exception simplification

9c5420a

Add test and fix apple and orange mixing error between sale order and…

df1a1a8

… sale order line

[FIX] sale_exception: remove obsolete base_sale_multi_channel referen…

6a6e29b

…ce (OCA#963)

[FIX] sale_exception: Disable failing test

23663cb

Disable for now as the solution is not obvious. Partially fixes OCA#972

[FIX] sale_exception: Avoid W0101 pylint error

46d575d

[FIX] sale_exception: Disable test via __init__

ac7af3d

Other ways it provokes linter fail

[MIG] sale_exception: Migration to 13.0

5aa82a2

sale_exception: apply pre-commit

cea287d

sale_exception: fix test class

18f9d57

TestSaleOrder was useless and making sale order tests run twice.

sale_exception: drop backward compat code

637ff79

sale_exception: fix warning

a892cdc

[UPD] Update sale_exception.pot

47a524f

[UPD] README.rst

be850cb

OCA-git-bot and others added 2 commits November 16, 2022 16:34

[UPD] README.rst

ae5e27a

[IMP] sale_exception: pre-commit stuff

1df77fa

matiasperalta1 force-pushed the 16.0-mig-sale_exception branch from b98ab73 to 0c33215 Compare November 17, 2022 14:22

matiasperalta1 mentioned this pull request Nov 17, 2022

Migration to version 16.0 #2215

Open

100 tasks

ghost suggested changes Nov 21, 2022

View reviewed changes

[MIG] sale_exception: Migration to 16.0

4be9ab6

jjscarafia force-pushed the 16.0-mig-sale_exception branch from 0c33215 to 4be9ab6 Compare December 16, 2022 11:54

aliciagaarzo approved these changes Dec 21, 2022

View reviewed changes

aliciagaarzo reviewed Dec 23, 2022

View reviewed changes

sebastienbeau mentioned this pull request Dec 30, 2022

[16.0][MIG] - sale_exception #2319

Closed

sebastienbeau reviewed Dec 30, 2022

View reviewed changes

OCA-git-bot added this to the 16.0 milestone Jan 2, 2023

rousseldenis approved these changes Jan 2, 2023

View reviewed changes

rousseldenis reviewed Jan 2, 2023

View reviewed changes

lmignon suggested changes Jan 10, 2023

View reviewed changes

rousseldenis added needs review migration labels Jan 11, 2023

sbejaoui mentioned this pull request Jan 25, 2023

[MIG] sale_exception: Migration to 16.0 #2344

Merged

rousseldenis added duplicate stale PR/Issue without recent activity, it'll be soon closed automatically. and removed needs review labels Jan 25, 2023

OCA-git-bot merged commit 4be9ab6 into OCA:16.0 Feb 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MIG] sale_exception: Migration to 16.0 #2266

[MIG] sale_exception: Migration to 16.0 #2266

matiasperalta1 commented Nov 17, 2022

ghost Nov 21, 2022

aliciagaarzo left a comment

sebastienbeau Dec 30, 2022

sebastienbeau Dec 30, 2022

florian-dacosta Dec 30, 2022

sebastienbeau commented Dec 30, 2022

sebastienbeau commented Dec 30, 2022

rousseldenis commented Jan 2, 2023

rousseldenis commented Jan 2, 2023

rousseldenis left a comment

rousseldenis Jan 2, 2023

rousseldenis Jan 2, 2023

lmignon left a comment

rousseldenis commented Jan 11, 2023

lmignon commented Jan 17, 2023

sbejaoui commented Jan 25, 2023

rousseldenis commented Jan 25, 2023

		from odoo import api, fields, models


		class ExceptionRule(models.Model):

		@@ -0,0 +1,125 @@
		# License AGPL-3 - See http://www.gnu.org/licenses/agpl-3.0.html

		import mock

[MIG] sale_exception: Migration to 16.0 #2266

[MIG] sale_exception: Migration to 16.0 #2266

Conversation

matiasperalta1 commented Nov 17, 2022

ghost Nov 21, 2022

Choose a reason for hiding this comment

aliciagaarzo left a comment

Choose a reason for hiding this comment

sebastienbeau Dec 30, 2022

Choose a reason for hiding this comment

sebastienbeau Dec 30, 2022

Choose a reason for hiding this comment

florian-dacosta Dec 30, 2022

Choose a reason for hiding this comment

sebastienbeau commented Dec 30, 2022

sebastienbeau commented Dec 30, 2022

rousseldenis commented Jan 2, 2023

rousseldenis commented Jan 2, 2023

rousseldenis left a comment

Choose a reason for hiding this comment

rousseldenis Jan 2, 2023

Choose a reason for hiding this comment

rousseldenis Jan 2, 2023

Choose a reason for hiding this comment

lmignon left a comment

Choose a reason for hiding this comment

rousseldenis commented Jan 11, 2023

lmignon commented Jan 17, 2023

sbejaoui commented Jan 25, 2023

rousseldenis commented Jan 25, 2023