[4947][ADD] purchase_stock_price_variance #107

AungKoKoLin1997 · 2025-02-12T10:17:55Z

yostashiro

Partial review.

inventory_price_check doesn't seem to follow the normal naming convention. How about purchase_stock_price_variance as the module name?

yostashiro · 2025-02-14T07:21:34Z

inventory_price_check/models/product_template.py

+    price_discrepancy_threshold_type = fields.Selection(
+        [("percentage", "Percentage"), ("fixed", "Fixed Value"), ("ignore", "Ignore")]
+    )
+    price_discrepancy_threshold_value = fields.Float(
+        help="Threshold value for price discrepancy warnings."
+    )


Can we make it like this:

Suggested change

price_discrepancy_threshold_type = fields.Selection(

[("percentage", "Percentage"), ("fixed", "Fixed Value"), ("ignore", "Ignore")]

)

price_discrepancy_threshold_value = fields.Float(

help="Threshold value for price discrepancy warnings."

)

price_variance_threshold_percent = fields.Float(help="Maximum variance (in percent) allowable between the product's standard price and purchase receipt unit price.")

price_variance_threshold_amount = fields.Float(help="Maximum allowable variance (in monetary amount, based on company currency) between the product's standard price and the purchase receipt unit price.")

The idea is to allow both types of thresholds. Hitting either one should trigger an error.

yostashiro · 2025-02-14T07:28:39Z

inventory_price_check/models/stock_picking.py

+class Stockpick(models.Model):
+    _inherit = "stock.picking"
+
+    allow_price_inconsistency = fields.Boolean(


Suggested change

allow_price_inconsistency = fields.Boolean(

bypass_price_variance_check = fields.Boolean(

yostashiro · 2025-02-14T07:29:35Z

inventory_price_check/models/stock_picking.py

+        for pick in self:
+            if pick.picking_type_id.code != "incoming":
+                continue
+            if pick.sudo().allow_price_inconsistency:


Why sudo() here?

This field is only allowed for some group and we will get access right error if the user is not in this group.

yostashiro · 2025-02-14T07:35:58Z

inventory_price_check/models/stock_picking.py

+                product = move.product_id
+                threshold_type = (
+                    product.price_discrepancy_threshold_type
+                    or pick.company_id.price_discrepancy_threshold_type


Values from the company should be kept in variables outside the loop.

yostashiro

Partial review.

yostashiro · 2025-02-17T03:38:25Z

purchase_stock_price_variance/models/stock_picking.py

+    bypass_price_variance_check = fields.Boolean(
+        copy=False,
+        tracking=True,
+        groups="purchase_stock_price_variance.group_bypass_price_variance_check",


Wouldn't it be better to extend write() to check if the user belongs to this group? I think this group assignment to the field is too rigid.

yostashiro · 2025-02-17T03:59:08Z

purchase_stock_price_variance/models/stock_picking.py

+                    product.price_variance_threshold_amount
+                    or global_price_variance_threshold_amount
+                )
+                received_price = move.price_unit


I think it should be safer this way to update the cost based on the current exchange rate.

Suggested change

received_price = move.price_unit

received_price = move._get_price_unit()

yostashiro · 2025-02-17T04:06:08Z

purchase_stock_price_variance/models/stock_picking.py

+                continue
+            if pick.sudo().bypass_price_variance_check:
+                continue
+            for move in pick.move_ids_without_package:


Any reason for not using pick.move_ids?

yostashiro · 2025-02-17T04:08:18Z

purchase_stock_price_variance/models/stock_picking.py

+            if pick.picking_type_id.code != "incoming":
+                continue


Can you please check if adding conditions using _is_in() and _is_dropshipped() for each stock move record is more appropriate instead of this?

When I checked the odoo source code, checking picking_type_id.code is used in stock._is_in() and _is_dropshipped() is introduced in stock_account and used them. In our module, stock_account is part of dependency because we use purchase_stock as a dependency. So, I think using _is_in() and _is_dropshipped() is more appropriate.

yostashiro · 2025-02-17T04:09:56Z

purchase_stock_price_variance/views/product_product_views.xml

+                    <field name="price_variance_threshold_percent" />
+                    <field name="price_variance_threshold_amount" />
+                    <field name="bypass_price_variance_check" />


Suggested change

<field name="price_variance_threshold_percent" />

<field name="price_variance_threshold_amount" />

<field name="bypass_price_variance_check" />

<field name="bypass_price_variance_check" />

<field name="price_variance_threshold_percent" />

<field name="price_variance_threshold_amount" />

and make the threshold fields invisible when bypass_price_variance_check is true.

AungKoKoLin1997 · 2025-02-17T08:10:27Z

I did bypass_price_variance_check to be True as a default for two reasons. First is to avoid the test cases failed of other module because the default threshold values are 0.0(it means there should be no variance) and reduce the work for the user to disable the check for all product and to set up the threshold value for the products that is needed to use for threshold check.

yostashiro · 2025-02-17T08:08:37Z

purchase_stock_price_variance/models/stock_picking.py

+                ):
+                    raise UserError(
+                        _(
+                            f"Price discrepancy detected for {product.name}: "


Suggested change

f"Price discrepancy detected for {product.name}: "

f"Price variance exceeding a threshold detected for {product.name}: "

yostashiro · 2025-02-17T08:18:48Z

purchase_stock_price_variance/readme/CONFIGURE.md

Please tidy the steps (with numbers, perhaps) and "correct" the line breaks.

yostashiro · 2025-02-17T08:24:51Z

purchase_stock_price_variance/readme/DESCRIPTION.md

+This module checks for variance between the receipt price and the product's
+standard price at the time of validation.


Suggested change

This module checks for variance between the receipt price and the product's

standard price at the time of validation.

This module checks the variance between the purchase price and the product's

standard price at the time of receipt picking validation and displays an error if the

variance exceeds a given threshold.

yostashiro · 2025-02-17T08:31:08Z

purchase_stock_price_variance/readme/USAGE.md

+A warning message will be displayed during validation if the variance
+between the receipt price and the standard price exceeds the threshold.
+
+Selecting 'Bypass Price Variance Check' allows users to bypass this error,
+and it is only visible to members of the 'Bypass Price Variance Check'
+group.


Suggested change

A warning message will be displayed during validation if the variance

between the receipt price and the standard price exceeds the threshold.

Selecting 'Bypass Price Variance Check' allows users to bypass this error,

and it is only visible to members of the 'Bypass Price Variance Check'

group.

An error message will be displayed during validation if the variance

between the purchase price and the standard price exceeds a threshold.

Selecting 'Bypass Price Variance Check' in the receipt picking allows users

to bypass this error. Only members of the 'Bypass Price Variance Check'

group can update this field.

yostashiro · 2025-02-17T08:34:00Z

purchase_stock_price_variance/models/stock_picking.py

+            self.env.company.price_variance_threshold_amount
+        )
+        for pick in self:
+            if pick.sudo().bypass_price_variance_check:


Suggested change

if pick.sudo().bypass_price_variance_check:

if pick.bypass_price_variance_check:

yostashiro · 2025-02-17T08:54:46Z

I did bypass_price_variance_check to be True as a default for two reasons. First is to avoid the test cases failed of other module because the default threshold values are 0.0(it means there should be no variance) and reduce the work for the user to disable the check for all product and to set up the threshold value for the products that is needed to use for threshold check.

@AungKoKoLin1997 Sorry, missed this comment. I think the default of the product should be false, and have another field in res.config.settings to enable the feature.

yostashiro

Code review.

nobuQuartile · 2025-02-18T03:02:43Z

purchase_stock_price_variance/models/stock_picking.py

+                if (
+                    percentage_difference > threshold_percent
+                    or amount_difference > threshold_amount
+                ):


@AungKoKoLin1997
I'm doing the functional test.
Currently, two problems arise.

The initial threshold value is 0, so if the variance check is set to true and other thresholds are not changed, all products will not be able to be received.

Both percentage and fixed values are used for variance, making it difficult to continue adjusting those values.

When I consulted Tashiro-san, he said that when it was 0, it would be best to avoid using that threshold.
I thought so too.

I think it's a good idea to separate the process by threshold to see if they're using that value, and if they're using them, check if they're above the threshold.
What do you think?

yostashiro

@nobuQuartile Please remove the pre-commit noise from other modules.

yostashiro · 2025-02-18T04:31:37Z

purchase_stock_price_variance/models/stock_picking.py

+                    threshold_percent != 0 and percentage_difference > threshold_percent
+                ) or (threshold_amount != 0 and amount_difference > threshold_amount):


Suggested change

threshold_percent != 0 and percentage_difference > threshold_percent

) or (threshold_amount != 0 and amount_difference > threshold_amount):

threshold_percent and percentage_difference > threshold_percent

) or (threshold_amount and amount_difference > threshold_amount):

yostashiro · 2025-02-18T04:36:39Z

purchase_stock_price_variance/models/res_company.py

+    price_variance_threshold_amount = fields.Float(
+        help="Maximum allowable variance (in monetary amount, based on company currency)"
+        " between the product's standard price and the purchase receipt unit price."
+    )


Let's make sure that the values are positive here.

Suggested change

)

)

@api.constrains("price_variance_threshold_percent", "price_variance_threshold_amount")

def _check_price_variance_threshold(self):

for rec in self:

if rec.price_variance_threshold_percent < 0 or rec.price_variance_threshold_amount < 0:

raise ValidationError(_("The threshold values cannot be negative."))

yostashiro · 2025-02-18T04:44:05Z

purchase_stock_price_variance/views/res_config_setting_views.xml

+                    <div class="o_setting_right_pane">
+                        <label for="price_variance_threshold_percent" />
+                        <div class="text-muted">
+                            Maximum variance (in percent) allowable between the product's standard price and purchase receipt unit price.


Suggested change

Maximum variance (in percent) allowable between the product's standard price and purchase receipt unit price.

Maximum variance (in percent) allowable between the product's standard price and purchase receipt unit price.

Keeping it at zero means that this threshold will not be checked.

Please apply the same adjustments to all the relevant places.

yostashiro · 2025-02-18T04:52:12Z

purchase_stock_price_variance/views/res_config_setting_views.xml

+                        </div>
+                        <div class="content-group">
+                            <div class="mt16">
+                                <field name="price_variance_threshold_percent" />


Not sure this is the correct way to put it. Please check it yourself, and do the same for all the relevant fields.

Suggested change

<field name="price_variance_threshold_percent" />

<field name="price_variance_threshold_percent" /><span> %</span>

nobuQuartile · 2025-02-18T05:37:22Z

@AungKoKoLin1997 @yostashiro
Global values cannot be ignored for each threshold at the product level and cannot be allowed.

Current Product Variant Options
The initial value is set to 0

Set a value at the product level (+)
Disable the variant at the product level for a specific threshold (Not possible)
Use the global value (0)

Option1
When using numeric types, the only way to divide other than 0 and plus is negative.

Disable the variant at the product level for a specific threshold (-)

Option2

Set a value at the product level (+)
Disable the variant at the product level for a specific threshold (0)
Use the global value (-)

Option3
Create a new boolean for each threshold at the product level.

Set a value at the product level (+)
Disable the variant at the product level for a specific threshold (0)
Use the global value (New boolean)

I think Option3 is better.
What do you think?

nobuQuartile · 2025-02-18T07:42:54Z

We decided not to change the code.

Current Product Variant Options
The initial value is set to 0

Set a value at the product level (+)
Disable the variant at the product level for a specific threshold (Large value that is not typical)
Use the global value (0)

AungKoKoLin1997 added 2 commits February 12, 2025 10:16

[ADD] purchase_inventory_valuation_price_check

8cdbf6a

upd

745792e

AungKoKoLin1997 changed the title ~~[4947][ADD] purchase_inventory_valuation_price_check~~ [4947][ADD] purchase_inventory_price_check Feb 14, 2025

adj

7d567d6

AungKoKoLin1997 changed the title ~~[4947][ADD] purchase_inventory_price_check~~ [4947][ADD] inventory_price_check Feb 14, 2025

AungKoKoLin1997 marked this pull request as ready for review February 14, 2025 03:08

AungKoKoLin1997 requested review from yostashiro, kanda999 and nobuQuartile February 14, 2025 03:08

yostashiro requested changes Feb 14, 2025

View reviewed changes

adj

a20f54c

AungKoKoLin1997 changed the title ~~[4947][ADD] inventory_price_check~~ [4947][ADD] purchase_stock_price_variance Feb 17, 2025

adj

f54711e

AungKoKoLin1997 requested a review from yostashiro February 17, 2025 02:56

yostashiro reviewed Feb 17, 2025

View reviewed changes

AungKoKoLin1997 added 2 commits February 17, 2025 07:34

upd

61def00

adj

b5b9564

AungKoKoLin1997 requested a review from yostashiro February 17, 2025 07:44

yostashiro reviewed Feb 17, 2025

View reviewed changes

upd

c0da992

AungKoKoLin1997 requested a review from yostashiro February 17, 2025 09:41

yostashiro approved these changes Feb 17, 2025

View reviewed changes

nobuQuartile reviewed Feb 18, 2025

View reviewed changes

nobuQuartile added 2 commits February 18, 2025 03:57

upd when a threshold is 0

6bc4c91

[4947][IMP] pre-commit

1db6583

yostashiro requested changes Feb 18, 2025

View reviewed changes

upd

f3ff93a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4947][ADD] purchase_stock_price_variance #107

[4947][ADD] purchase_stock_price_variance #107

AungKoKoLin1997 commented Feb 12, 2025

yostashiro left a comment

yostashiro Feb 14, 2025

yostashiro Feb 14, 2025

yostashiro Feb 14, 2025

AungKoKoLin1997 Feb 17, 2025

yostashiro Feb 14, 2025

yostashiro left a comment

yostashiro Feb 17, 2025

yostashiro Feb 17, 2025

yostashiro Feb 17, 2025

yostashiro Feb 17, 2025

AungKoKoLin1997 Feb 17, 2025 •

edited

Loading

yostashiro Feb 17, 2025

AungKoKoLin1997 commented Feb 17, 2025

yostashiro Feb 17, 2025

yostashiro Feb 17, 2025

yostashiro Feb 17, 2025

yostashiro Feb 17, 2025

yostashiro Feb 17, 2025

yostashiro commented Feb 17, 2025

yostashiro left a comment

nobuQuartile Feb 18, 2025

yostashiro left a comment

yostashiro Feb 18, 2025

yostashiro Feb 18, 2025

yostashiro Feb 18, 2025

yostashiro Feb 18, 2025

nobuQuartile commented Feb 18, 2025

nobuQuartile commented Feb 18, 2025

	allow_price_inconsistency = fields.Boolean(
	bypass_price_variance_check = fields.Boolean(

	received_price = move.price_unit
	received_price = move._get_price_unit()

	f"Price discrepancy detected for {product.name}: "
	f"Price variance exceeding a threshold detected for {product.name}: "

		This module checks for variance between the receipt price and the product's
		standard price at the time of validation.

-This module checks for variance between the receipt price and the product's
-standard price at the time of validation.
+This module checks the variance between the purchase price and the product's
+standard price at the time of receipt picking validation and displays an error if the
+variance exceeds a given threshold.

-A warning message will be displayed during validation if the variance
-between the receipt price and the standard price exceeds the threshold.
-Selecting 'Bypass Price Variance Check' allows users to bypass this error,
-and it is only visible to members of the 'Bypass Price Variance Check'
-group.
+An error message will be displayed during validation if the variance
+between the purchase price and the standard price exceeds a threshold.
+Selecting 'Bypass Price Variance Check' in the receipt picking allows users
+to bypass this error. Only members of the 'Bypass Price Variance Check'
+group can update this field.

	if pick.sudo().bypass_price_variance_check:
	if pick.bypass_price_variance_check:

		threshold_percent != 0 and percentage_difference > threshold_percent
		) or (threshold_amount != 0 and amount_difference > threshold_amount):

-    )
+    )
+    @api.constrains("price_variance_threshold_percent", "price_variance_threshold_amount")
+    def _check_price_variance_threshold(self):
+        for rec in self:
+            if rec.price_variance_threshold_percent < 0 or rec.price_variance_threshold_amount < 0:
+                raise ValidationError(_("The threshold values cannot be negative."))

	Maximum variance (in percent) allowable between the product's standard price and purchase receipt unit price.
	Maximum variance (in percent) allowable between the product's standard price and purchase receipt unit price.
	Keeping it at zero means that this threshold will not be checked.

	<field name="price_variance_threshold_percent" />
	<field name="price_variance_threshold_percent" /><span> %</span>

[4947][ADD] purchase_stock_price_variance #107

Are you sure you want to change the base?

[4947][ADD] purchase_stock_price_variance #107

Conversation

AungKoKoLin1997 commented Feb 12, 2025

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AungKoKoLin1997 Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AungKoKoLin1997 commented Feb 17, 2025

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

yostashiro commented Feb 17, 2025

yostashiro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nobuQuartile commented Feb 18, 2025

nobuQuartile commented Feb 18, 2025

AungKoKoLin1997 Feb 17, 2025 •

edited

Loading