[16.0][IMP] stock_move_line_lock_qty_done: prevent test cases from other modules from failing #1760

AungKoKoLin1997 · 2024-11-08T10:33:22Z

Before this PR, if we had other test cases that updated the done quantity after the transfer was done in our module, they would be blocked by stock_move_line_lock_qty_done. This PR addresses that issue.

@qrtl QT4650

AungKoKoLin1997 · 2024-11-08T10:44:35Z

I assume the test cases failed from stock_move_line_auto_fill in this PR is not related with the changes because I see the other PR is facing the same issue.

yostashiro · 2024-11-09T10:14:59Z

stock_move_line_lock_qty_done/models/stock_move_line.py

+        check_qty_done_change_allowed = (
+            config["test_enable"]
+            and self.env.context.get("test_stock_move_line_lock_qty_done")
+        ) or not config["test_enable"]
+        if not check_qty_done_change_allowed:
+            return True


Shouldn't this suffice?

Suggested change

check_qty_done_change_allowed = (

config["test_enable"]

and self.env.context.get("test_stock_move_line_lock_qty_done")

) or not config["test_enable"]

if not check_qty_done_change_allowed:

return True

if config["test_enable"] and not self.env.context.get("test_stock_move_line_lock_qty_done"):

return True

yostashiro

LGTM.

rousseldenis · 2024-11-15T10:17:19Z

stock_move_line_lock_qty_done/models/stock_move_line.py



 class StockMoveLine(models.Model):

    _inherit = "stock.move.line"

    def _check_qty_done_change_allowed(self, vals):
+        if config["test_enable"] and not self.env.context.get(


I know this is an easy workaround that flourishes everywhere but it's not a good practice.

Introduce a configuration setting that allows to enable or disable the feature instead.

Thanks !

@rousseldenis I thought you might say so. ;)

rousseldenis

See comment

rousseldenis · 2024-11-15T10:17:38Z

@sbejaoui

AungKoKoLin1997 · 2024-11-15T11:32:49Z

@sbejaoui Can you please review?

yostashiro

LGTM. 👍

yostashiro · 2024-11-16T04:43:58Z

stock_move_line_lock_qty_done/data/ir_config_parameter.xml

+    <record id="param_move_line_lock_qty_done_enable" model="ir.config_parameter">
+        <field
+            name="key"
+        >stock_move_line_lock_qty_done.move_line_lock_qty_done_enable</field>


Non-blocking. Just thought the key could be shortened.

Suggested change

>stock_move_line_lock_qty_done.move_line_lock_qty_done_enable</field>

>stock_move_line_lock_qty_done.lock_qty_done</field>

rousseldenis · 2024-11-18T08:12:18Z

stock_move_line_lock_qty_done/models/res_config_settings.py

+class ResConfigSettings(models.TransientModel):
+    _inherit = "res.config.settings"
+
+    lock_qty_done = fields.Boolean(


I'd prefer a related on a company field.

That will allow you to avoid get/set_values calls.

If we are related on a company field instead of system parameter and enable this feature by default, do you want to set lock_qty_done to false at the end of the test cases of the module to prevent this feature from other test cases? May be I don't get your intention.

No.

From a functional point of view, enabling or disabling the feature per company is a good practice.
From a technical point of view (that's a matter of taste), I don't like such writings. Moreover, in v16, you have the field attribute config_parameterthat allows to avoid such get/set.

yostashiro · 2024-11-25T08:05:57Z

stock_move_line_lock_qty_done/demo/demo_company.xml

+    <record id="base.main_company" model="res.company">
+        <field name="lock_qty_done">False</field>
+    </record>


To disable the feature for all companies.

Suggested change

<record id="base.main_company" model="res.company">

<field name="lock_qty_done">False</field>

</record>

<function model="res.company" name="write">

<value model="res.company" search="[]" />

<value eval="{'lock_qty_done': False}" />

</function>

I updated the code.

yostashiro · 2024-11-25T08:07:29Z

stock_move_line_lock_qty_done/models/stock_move_line.py

+        if not self.company_id.lock_qty_done:
+            return True


Suggested change

if not self.company_id.lock_qty_done:

return True

self is a recordset. Apply the check inside the for loop.

Updated the code.

rousseldenis · 2024-11-25T08:35:23Z

stock_move_line_lock_qty_done/models/res_company.py

+    _inherit = "res.company"
+
+    lock_qty_done = fields.Boolean(
+        default=True,


But last question, why enabling it by default?

IMHO, that should be done on your project side.

My thought is that users install this module because they want to use its features. So, requiring them to change the settings after installing the module feels a bit strange to me.

That has been always a debate. But that changes Odoo standard behavior. Which is not always wanted. That's why enabling the feature on deployment is better.

And you avoid disabling it in demo data

I updated the code.

AungKoKoLin1997 · 2024-11-25T09:40:33Z

@yostashiro Please review the PR with the latest changes.

yostashiro · 2024-11-25T09:50:18Z

stock_move_line_lock_qty_done/readme/CONFIGURE.rst

 To configure this module, you need to add the users allowed to edit the done quntity
 for done moves to the group "Can edit done quantity for done stock moves"
+
+Enable the 'Limit Updates to Done Quantity After Validation' option under 'Inventory > Settings' to enable the restriction of modifying the done quantity for validated stock moves.


Suggested change

To configure this module, you need to add the users allowed to edit the done quntity

for done moves to the group "Can edit done quantity for done stock moves"

Enable the 'Limit Updates to Done Quantity After Validation' option under 'Inventory > Settings' to enable the restriction of modifying the done quantity for validated stock moves.

Update the following settings:

* Go to *Inventory > Settings* and select 'Limit Updates to Done Quantity After Validation' option to enable the restriction. This should be done for each company.

* Add users allowed to edit the done quantity for done moves to the group 'Can edit done quantity for done stock moves'.

…dules from failing

yostashiro

LGTM.

AungKoKoLin1997 marked this pull request as draft November 8, 2024 10:39

AungKoKoLin1997 marked this pull request as ready for review November 8, 2024 10:43

yostashiro reviewed Nov 9, 2024

View reviewed changes

AungKoKoLin1997 force-pushed the 16.0-imp-stock_move_line_lock_qty_done branch from a356ecc to f863bd7 Compare November 12, 2024 02:28

yostashiro approved these changes Nov 12, 2024

View reviewed changes

AungKoKoLin1997 mentioned this pull request Nov 15, 2024

[16.0][MIG] product_cost_price_avco_sync: Migration to 16.0 #1465

Open

rousseldenis reviewed Nov 15, 2024

View reviewed changes

rousseldenis requested changes Nov 15, 2024

View reviewed changes

AungKoKoLin1997 force-pushed the 16.0-imp-stock_move_line_lock_qty_done branch from f863bd7 to e857eb4 Compare November 15, 2024 11:30

AungKoKoLin1997 requested review from yostashiro and rousseldenis November 15, 2024 11:32

AungKoKoLin1997 force-pushed the 16.0-imp-stock_move_line_lock_qty_done branch 2 times, most recently from ae8134b to d399ae0 Compare November 16, 2024 03:39

yostashiro approved these changes Nov 16, 2024

View reviewed changes

AungKoKoLin1997 force-pushed the 16.0-imp-stock_move_line_lock_qty_done branch from d399ae0 to 321184f Compare November 18, 2024 02:46

AungKoKoLin1997 requested a review from yostashiro November 18, 2024 02:54

rousseldenis added enhancement needs review labels Nov 18, 2024

rousseldenis added this to the 16.0 milestone Nov 18, 2024

rousseldenis reviewed Nov 18, 2024

View reviewed changes

AungKoKoLin1997 force-pushed the 16.0-imp-stock_move_line_lock_qty_done branch from 321184f to 285bcfa Compare November 25, 2024 03:54

AungKoKoLin1997 requested a review from rousseldenis November 25, 2024 03:56

AungKoKoLin1997 force-pushed the 16.0-imp-stock_move_line_lock_qty_done branch from 285bcfa to d4f4cd6 Compare November 25, 2024 04:06

yostashiro suggested changes Nov 25, 2024

View reviewed changes

rousseldenis reviewed Nov 25, 2024

View reviewed changes

AungKoKoLin1997 force-pushed the 16.0-imp-stock_move_line_lock_qty_done branch from d4f4cd6 to ba4bd44 Compare November 25, 2024 08:53

AungKoKoLin1997 requested a review from yostashiro November 25, 2024 08:55

AungKoKoLin1997 force-pushed the 16.0-imp-stock_move_line_lock_qty_done branch from ba4bd44 to 101a2bf Compare November 25, 2024 09:37

AungKoKoLin1997 requested a review from rousseldenis November 25, 2024 09:38

AungKoKoLin1997 force-pushed the 16.0-imp-stock_move_line_lock_qty_done branch from 101a2bf to dff0632 Compare November 25, 2024 09:43

rousseldenis approved these changes Nov 25, 2024

View reviewed changes

yostashiro reviewed Nov 25, 2024

View reviewed changes

AungKoKoLin1997 force-pushed the 16.0-imp-stock_move_line_lock_qty_done branch from dff0632 to f47aa80 Compare November 25, 2024 09:58

AungKoKoLin1997 requested a review from yostashiro November 25, 2024 09:59

[IMP] stock_move_line_lock_qty_done: prevent test cases from other mo…

3f0e04b

…dules from failing

AungKoKoLin1997 force-pushed the 16.0-imp-stock_move_line_lock_qty_done branch from f47aa80 to 3f0e04b Compare November 25, 2024 10:10

yostashiro approved these changes Nov 25, 2024

View reviewed changes

OCA-git-bot added the approved label Nov 25, 2024

AungKoKoLin1997 mentioned this pull request Nov 26, 2024

[16.0][IMP] stock_inventory_discrepancy: add configuration for enabling discrepancy check OCA/stock-logistics-warehouse#1966

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[16.0][IMP] stock_move_line_lock_qty_done: prevent test cases from other modules from failing #1760

[16.0][IMP] stock_move_line_lock_qty_done: prevent test cases from other modules from failing #1760

AungKoKoLin1997 commented Nov 8, 2024 •

edited

Loading

AungKoKoLin1997 commented Nov 8, 2024

yostashiro Nov 9, 2024

yostashiro left a comment

rousseldenis Nov 15, 2024 •

edited

Loading

yostashiro Nov 15, 2024

rousseldenis left a comment

rousseldenis commented Nov 15, 2024

AungKoKoLin1997 commented Nov 15, 2024

yostashiro left a comment

yostashiro Nov 16, 2024

rousseldenis Nov 18, 2024

AungKoKoLin1997 Nov 18, 2024 •

edited

Loading

rousseldenis Nov 18, 2024

yostashiro Nov 25, 2024

AungKoKoLin1997 Nov 25, 2024

yostashiro Nov 25, 2024

AungKoKoLin1997 Nov 25, 2024

rousseldenis Nov 25, 2024

AungKoKoLin1997 Nov 25, 2024

rousseldenis Nov 25, 2024 •

edited

Loading

AungKoKoLin1997 Nov 25, 2024

AungKoKoLin1997 commented Nov 25, 2024

yostashiro Nov 25, 2024

yostashiro left a comment

	>stock_move_line_lock_qty_done.move_line_lock_qty_done_enable</field>
	>stock_move_line_lock_qty_done.lock_qty_done</field>

-    <record id="base.main_company" model="res.company">
-        <field name="lock_qty_done">False</field>
-    </record>
+    <function model="res.company" name="write">
+        <value model="res.company" search="[]" />
+        <value eval="{'lock_qty_done': False}" />
+    </function>

[16.0][IMP] stock_move_line_lock_qty_done: prevent test cases from other modules from failing #1760

Are you sure you want to change the base?

[16.0][IMP] stock_move_line_lock_qty_done: prevent test cases from other modules from failing #1760

Conversation

AungKoKoLin1997 commented Nov 8, 2024 • edited Loading

AungKoKoLin1997 commented Nov 8, 2024

Choose a reason for hiding this comment

yostashiro left a comment

Choose a reason for hiding this comment

rousseldenis Nov 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rousseldenis left a comment

Choose a reason for hiding this comment

rousseldenis commented Nov 15, 2024

AungKoKoLin1997 commented Nov 15, 2024

yostashiro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AungKoKoLin1997 Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rousseldenis Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AungKoKoLin1997 commented Nov 25, 2024

Choose a reason for hiding this comment

yostashiro left a comment

Choose a reason for hiding this comment

AungKoKoLin1997 commented Nov 8, 2024 •

edited

Loading

rousseldenis Nov 15, 2024 •

edited

Loading

AungKoKoLin1997 Nov 18, 2024 •

edited

Loading

rousseldenis Nov 25, 2024 •

edited

Loading