[18.0][IMP] purchase_order_secondary_unit: add vendor pricelist support by yostashiro · Pull Request #2932 · OCA/purchase-workflow

yostashiro · 2026-01-11T12:19:04Z

Extend product.supplierinfo and purchase.order.line to support secondary unit pricing
~~Add company setting to control price display in purchase reports (primary only, prioritize secondary, or both)~~ -> Moved to product_secondary_unit
Update purchase order/quotation reports to show secondary unit prices according to the configuration

yostashiro · 2026-01-11T13:46:54Z

Secondary Price fields added to purchase order line and vendor pricelist.

Added company-level report settings. The default settings generate reports in the same layout but with a small enhancement of showing the UoM for the unit price (as it could otherwise be a bit confusing).

If you prioritize secondary unit price and hide secondary UoM column, the report will only show secondary qty and secondary price if the purchase order line has a secondary UoM set, otherwise fall back to primary qty and primary price.

If you choose to show both primary and secondary, the report will do so in the same columns.

AungKoKoLin1997 · 2026-01-14T03:51:55Z

I am reviewing the functional and found some strange behaviors. I am not sure this may be not part of the scope of this PR.

Test Case 1

I create two Secondary Unit of Measure for a product.
Then, I create a purchase order with secondary uom. The secondary uom and secondary uom price are correctly assigned in vendor info.
After that, I create another purchase order with with the same vendor of first PO and different price unit and different secondary uom.
When I check the vendor info, I found the price unit of existing vendor line is updated with new price unit. But the secondary uom and secondary uom price are not updated with new values.

Proudct:

Purchase Order 1:

Seller Info:

Purchase Order 2:

Seller Info:

Test Case 2

I create two Secondary Unit of Measure for a product.
Then, I create a purchase order with secondary uom. The secondary uom and secondary uom price are correctly assigned in vendor info.
After that, I create another purchase order with with the same vendor of first PO and same price unit and different secondary uom.
In the vendor info, I found the secondary uom and secondary uom price are not updated.

Purchase Order 1:

Seller Info:

Purchase Order 2:

Seller Info:

AungKoKoLin1997 · 2026-01-14T04:15:46Z

Regarding the two test cases I mentioned in my previous comment, they’re out of scope for this PR. I’m also not confident they’re worth addressing right now, and we don’t expect to use those use cases at the moment—so I’ll ignore them for now.

AungKoKoLin1997

LGTM

AungKoKoLin1997 · 2026-01-15T08:24:50Z

In my last commit, I fixed the layout issue when we use Prioritize Secondary Unit Price without hiding Secondary UoM Column.

AungKoKoLin1997 · 2026-01-15T10:05:39Z

@yostashiro Could you please review my last two commits? I will squash commits after you review.

yostashiro · 2026-01-15T13:19:30Z

purchase_order_secondary_unit/reports/purchase_order_templates.xml

+        <!-- Unit price: add UoM and secondary price ('secondary' and 'both' modes) -->
+        <xpath expr="//span[@t-field='line.price_unit']" position="after">
+            <t
+                t-if="line.report_show_price_uom() and line.get_secondary_uom_display_mode() != 'secondary'"


Suggested change

t-if="line.report_show_price_uom() and line.get_secondary_uom_display_mode() != 'secondary'"

t-if="line.report_show_price_uom()"

@AungKoKoLin1997 We should not change the code here. Please update line.report_show_price_uom() instead.

yostashiro · 2026-01-15T13:25:22Z

purchase_order_secondary_unit/views/purchase_order_portal_templates.xml

+            <t
+                t-if="line.report_show_price_uom() and line.get_secondary_uom_display_mode() != 'secondary'"
+            >
+                / <span t-field="line.product_uom.name" />
+            </t>
+            <t t-if="line.get_secondary_uom_display_mode() == 'secondary'">
+                <span t-field="line.secondary_uom_price" />
+                <t t-if="line.report_show_price_uom()">
+                    / <span
+                    t-field="line.secondary_uom_id.name"
+                    groups="uom.group_uom"
+                />
+                </t>
+            </t>
+            <t t-if="line.get_secondary_uom_display_mode() == 'both'">
+                <br />
+                <span class="text-muted">
+                    (<span t-field="line.secondary_uom_price" />
+                    / <span
+                    t-field="line.secondary_uom_id.name"
+                    groups="uom.group_uom"
+                />)
+                </span>
+            </t>


I guess we could create a common template and call it from both report and portal view.

yostashiro · 2026-01-16T07:41:10Z

purchase_order_secondary_unit/views/purchase_order_portal_templates.xml

@@ -0,0 +1,89 @@
+<odoo>
+    <!-- Unit price: add UoM and secondary price ('secondary' and 'both' modes) -->
+    <template id="purchase_secondary_uom_price">


Suggested change

<template id="purchase_secondary_uom_price">

<template id="purchase_uom_price">

I changed the template name.

yostashiro · 2026-01-16T09:35:05Z

@AungKoKoLin1997 Looks good. 👍 Please go ahead and squash commits.

AungKoKoLin1997 · 2026-01-20T05:44:07Z

Depends on OCA/product-attribute#2182.

AungKoKoLin1997 · 2026-01-20T07:55:00Z

I added new commit to propagate secondary_uom_id from purchase order lines to account move lines when the field exists on account.move.line. This is useful when account_move_secondary_unit is installed, and avoids the need for a glue module.

nobuQuartile · 2026-01-21T03:03:56Z

purchase_order_secondary_unit/models/product_supplierinfo.py

+            else:
+                rec.secondary_uom_price = 0.0
+
+    @api.onchange("secondary_uom_price")


Suggested change

@api.onchange("secondary_uom_price")

I think we don't have to use @api.onchane for _inverse function.

I removed the onchange myself and tested the behavior, and found that the value isn’t updated dynamically until the record is saved.
With onchange in place, it seems the value updates dynamically as soon as it’s written, even before saving.
So, we should have api.onchane for this _inverse function.

Maybe you can directly update the price in _onchange_product_id_secondary_uom and skip adding the onchange in the inverse.

We could add a new onchange method that replicates the logic of this inverse (I guess _onchange_product_id_secondary_uom is not the one to work on) if we should be strict about not using onchange with an inverse.

I see a lot of these patterns, though, in the standard codebase, so I thought it was a widely accepted practice.

Why not reusing the onchange_product_id_secondary_uom? You update secondary_uom_id and price which is related with it in a single transaction

We could make it work but I don't feel there is a good reason to do so as the intents are different.

Ok, not blocking the decision, let me know when we can move it forward, appreciate that the module has been functional tested deeply :)

Thanks! You can go ahead and merge this if you are okay with the current code, or should we add new onchange methods to remove the onchange from inverses?

nobuQuartile · 2026-01-21T03:04:38Z

purchase_order_secondary_unit/models/purchase_order.py

+            else:
+                rec.secondary_uom_price = 0.0
+
+    @api.onchange("secondary_uom_price")


Suggested change

@api.onchange("secondary_uom_price")

I think we don't have to use @api.onchane for _inverse function.

I removed the onchange myself and tested the behavior, and found that the value isn’t updated dynamically until the record is saved.
With onchange in place, it seems the value updates dynamically as soon as it’s written, even before saving.
So, we should have api.onchane for this _inverse function.

nobuQuartile

LGTM
Code review and functional review

OCA-git-bot · 2026-01-21T04:34:08Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

OCA-git-bot · 2026-01-21T04:34:08Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

HviorForgeFlow

Code review, as it looks like adding a api.onchange in the inverse can work, because in the user interface the compute is not done until you save, you could add that price recalculation directly on the secondary uom on change, you can try, if this does not work we can go with your option

HviorForgeFlow · 2026-01-26T14:22:26Z

purchase_order_secondary_unit/models/product_supplierinfo.py

+            else:
+                rec.secondary_uom_price = 0.0
+
+    @api.onchange("secondary_uom_price")


Maybe you can directly update the price in _onchange_product_id_secondary_uom and skip adding the onchange in the inverse.

HviorForgeFlow · 2026-01-26T14:23:03Z

purchase_order_secondary_unit/models/purchase_order.py

+            else:
+                rec.secondary_uom_price = 0.0
+
+    @api.onchange("secondary_uom_price")


yostashiro · 2026-01-26T15:09:52Z

@HviorForgeFlow Thank you very much for your review. Commented here: #2932 (comment)

HviorForgeFlow

Code is not beautiful as it could be but it works.

My second concern is about adding that compute store fields.

@pedrobaeza what do you think about the onchange in the inverse and the compute store? Thanks in advice for taking your time to respond

HviorForgeFlow · 2026-01-26T16:12:35Z

purchase_order_secondary_unit/models/purchase_order.py

+    secondary_uom_price = fields.Float(
+        string="Secondary Price",
+        digits="Product Price",
+        aggregator="avg",
+        compute="_compute_secondary_uom_price",
+        inverse="_inverse_secondary_uom_price",
+        store=True,
+    )


My next concern is that this compute store can slowdown the update of the module for big databases.

Maybe we need a migration script where we create the column and fill it manually with a sql query.

Yes, the Héctor's concern is legit. About being computed or use onchanges, I usually prefer computed fields for not depending on UI, but sometimes the things get messy due to dependencies, and a simple onchange resolves the usability question.

Hi @pedrobaeza , I think Héctor is concerned about adding @api.onchange to inverse methods. Do you have any thoughts on this?

Well, I meant about making it computed stored. About reusing inverse methods as onchange may be legit for reflecting things in UI, but it depends on the logic of inverse method.

yostashiro · 2026-01-26T23:53:50Z

Maybe we need a migration script where we create the column and fill it manually with a sql query.

Thanks for catching this!

@AungKoKoLin1997 Can you please help add a migration script for the purchase order line field?

AungKoKoLin1997 · 2026-01-27T02:00:25Z

Can you please help add a migration script for the purchase order line field?

@yostashiro Done!
I already bump module version from 18.0.1.0.1 → 18.0.1.0.2.

yostashiro · 2026-01-27T02:17:26Z

purchase_order_secondary_unit/migrations/18.0.1.0.2/pre-migrate.py

+            UPDATE purchase_order_line pol
+                SET secondary_uom_price =
+                    COALESCE(
+                        pol.price_unit * (
+                            SELECT su.factor
+                              FROM product_secondary_unit su
+                            WHERE su.id = pol.secondary_uom_id
+                        ),
+                        0.0
+                    );


I suppose this would be more performant?

Suggested change

UPDATE purchase_order_line pol

SET secondary_uom_price =

COALESCE(

pol.price_unit * (

SELECT su.factor

FROM product_secondary_unit su

WHERE su.id = pol.secondary_uom_id

),

0.0

);

UPDATE purchase_order_line pol

SET secondary_uom_price = pol.price_unit * su.factor

FROM product_secondary_unit su

WHERE su.id = pol.secondary_uom.id;

Yes. It will be.

yostashiro · 2026-01-27T02:20:52Z

purchase_order_secondary_unit/migrations/18.0.1.1.0/pre-migrate.py

The bump should be "minor". i.e. 18.0.1.0.1 -> 18.0.1.1.0

… report config - Extend product.supplierinfo and purchase.order.line to support secondary unit pricing - Update purchase order/quotation reports to show secondary unit prices according to the configuration

… move line This commit propagates secondary_uom_id from purchase order lines to account move lines when the field exists on account.move.line. This is useful when account_move_secondary_unit is installed, and avoids the need for a glue module.

yostashiro · 2026-01-27T04:05:35Z

Looks good now.

HviorForgeFlow · 2026-01-27T16:27:26Z

/ocabot merge minor

OCA-git-bot · 2026-01-27T16:27:36Z

What a great day to merge this nice PR. Let's do it!
Prepared branch 18.0-ocabot-merge-pr-2932-by-HviorForgeFlow-bump-minor, awaiting test results.

OCA-git-bot · 2026-01-27T16:40:09Z

Congratulations, your PR was merged at 695e72e. Thanks a lot for contributing to OCA. ❤️

mav-adhoc · 2026-02-03T19:29:38Z

Hello! I don't get the "hide_secondary_uom_column(o)" in the view. There is not such attribute in res.company.

yostashiro · 2026-02-03T23:49:31Z

@mav-adhoc Oops, OCA/product-attribute#2182 needs to be merged, or we need to revert the parts that use hide_secondary_uom_column().

@sergio-teruel Would you be able to take a look at OCA/product-attribute#2182?

gdgellatly · 2026-02-09T18:46:38Z

purchase_order_secondary_unit/reports/purchase_order_templates.xml

+        <xpath expr="//span[@t-field='line.product_qty']" position="attributes">
+            <attribute
+                name="t-if"
+            >line.get_secondary_uom_display_mode() != 'secondary'</attribute>


@yostashiro Where is this function? We updated and getting errors as not existing, can't find anywhere.

This PR depends on OCA/product-attribute#2182, and the function is introduced in that PR , as I mentioned in #2932 (comment) . Unfortunately, this PR was merged before the dependency PR was merged. @yostashiro also explained the situation in the comment above: #2932 (comment).

OK, we have pinned to old version anyway after review as we really don't want these changes. In my opinion this should have been a seperate extension module, not because of the breakage, but it is for a feature that not everyone will want, its not really fixing an obvious oversight or error in functionality. In my experience this module is mostly used for straight conversions of qty's and adding price features just makes harder.

@gdgellatly That's a fair point and we did consider a split. The tradeoff is four additional modules across the core apps (xxx_secondary_unit_price for product, purchase, sale, and account), and we thought the maintenance overhead would outweigh the benefit. Our clients who need the secondary unit conversion also tend to need pricing per secondary unit, and all the added features here are effectively opt-in. That said, we are happy to split the module if the PSC opts for it. You can also leave a comment on OCA/product-attribute#2182 as it gets harder to change direction after merge.

@yostashiro Honestly it is no big deal, we have pinned now and that will get us through the v18 lifecycle which is only a few more months. For us a secondary unit is just a conversion to a primary unit, so price_subtotal is always identical and secondary_price_unit if we had would always just be price_subtotal/secondary_qty

And then look again during migration to decide what to do.

celm1990

I think this PR must be reverted until the dependency OCA/product-attribute#2182
is merged; otherwise, this module is broken when you try to print the report.

yostashiro · 2026-02-16T04:07:24Z

@AungKoKoLin1997 Can you please help create a PR to revert the template adjustments to avoid printing errors?

@celm1990 The dependency is now OCA/product-attribute#2211 by the way. We could also have it merged quickly if it gets support from the community.

yostashiro · 2026-02-16T13:47:29Z

Here is a follow-up: #2953

yostashiro force-pushed the 18.0-imp-purchase_secondary_unit branch 6 times, most recently from ae63904 to fc3ea0f Compare January 11, 2026 13:28

AungKoKoLin1997 force-pushed the 18.0-imp-purchase_secondary_unit branch from fc3ea0f to 1021636 Compare January 14, 2026 04:16

AungKoKoLin1997 approved these changes Jan 14, 2026

View reviewed changes

AungKoKoLin1997 force-pushed the 18.0-imp-purchase_secondary_unit branch 2 times, most recently from 736f15b to 8e4da27 Compare January 15, 2026 04:16

yostashiro commented Jan 15, 2026

View reviewed changes

yostashiro commented Jan 16, 2026

View reviewed changes

AungKoKoLin1997 force-pushed the 18.0-imp-purchase_secondary_unit branch 2 times, most recently from 91f742d to 3a74084 Compare January 20, 2026 05:32

nobuQuartile reviewed Jan 21, 2026

View reviewed changes

nobuQuartile approved these changes Jan 21, 2026

View reviewed changes

AungKoKoLin1997 force-pushed the 18.0-imp-purchase_secondary_unit branch from f3c57f1 to 3ad0c96 Compare January 21, 2026 04:25

OCA-git-bot added approved ready to merge labels Jan 21, 2026

yostashiro changed the title ~~[18.0][IMP] purchase_order_secondary_unit: add vendor pricelist support and report config~~ [18.0][IMP] purchase_order_secondary_unit: add vendor pricelist support Jan 21, 2026

HviorForgeFlow reviewed Jan 26, 2026

View reviewed changes

HviorForgeFlow requested changes Jan 26, 2026

View reviewed changes

OCA-git-bot removed approved ready to merge labels Jan 26, 2026

AungKoKoLin1997 force-pushed the 18.0-imp-purchase_secondary_unit branch from 3ad0c96 to 2f98124 Compare January 27, 2026 01:59

yostashiro commented Jan 27, 2026

View reviewed changes

yostashiro and others added 2 commits January 27, 2026 03:52

[IMP] purchase_order_secondary_unit: add vendor pricelist support and…

a1eea87

… report config - Extend product.supplierinfo and purchase.order.line to support secondary unit pricing - Update purchase order/quotation reports to show secondary unit prices according to the configuration

AungKoKoLin1997 force-pushed the 18.0-imp-purchase_secondary_unit branch from 2f98124 to 9970e65 Compare January 27, 2026 03:52

HviorForgeFlow approved these changes Jan 27, 2026

View reviewed changes

OCA-git-bot added the approved label Jan 27, 2026

OCA-git-bot added the bot is merging ⏳ label Jan 27, 2026

OCA-git-bot merged commit beee471 into OCA:18.0 Jan 27, 2026
6 of 7 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Jan 27, 2026

gdgellatly reviewed Feb 9, 2026

View reviewed changes

HviorForgeFlow mentioned this pull request Feb 10, 2026

[18.0][FIX] product_secondary_unit: UoM conversion in secondary unit mixin OCA/product-attribute#2182

Merged

celm1990 reviewed Feb 16, 2026

View reviewed changes

	t-if="line.report_show_price_uom() and line.get_secondary_uom_display_mode() != 'secondary'"
	t-if="line.report_show_price_uom()"

	<template id="purchase_secondary_uom_price">
	<template id="purchase_uom_price">

Uh oh!

Comments

Conversation

yostashiro commented Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yostashiro commented Jan 11, 2026

Uh oh!

AungKoKoLin1997 commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AungKoKoLin1997 commented Jan 14, 2026

Uh oh!

AungKoKoLin1997 left a comment

Choose a reason for hiding this comment

Uh oh!

AungKoKoLin1997 commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AungKoKoLin1997 commented Jan 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yostashiro commented Jan 16, 2026

Uh oh!

AungKoKoLin1997 commented Jan 20, 2026

Uh oh!

AungKoKoLin1997 commented Jan 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nobuQuartile Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nobuQuartile Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nobuQuartile left a comment

Choose a reason for hiding this comment

Uh oh!

OCA-git-bot commented Jan 21, 2026

Uh oh!

OCA-git-bot commented Jan 21, 2026

Uh oh!

HviorForgeFlow left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yostashiro commented Jan 26, 2026

yostashiro commented Jan 11, 2026 •

edited

Loading

AungKoKoLin1997 commented Jan 14, 2026 •

edited

Loading

AungKoKoLin1997 commented Jan 15, 2026 •

edited

Loading

nobuQuartile Jan 21, 2026 •

edited

Loading

nobuQuartile Jan 21, 2026 •

edited

Loading