Skip to content

[18.0][IMP] product_secondary_unit: add configuration for report presentation#2211

Open
AungKoKoLin1997 wants to merge 2 commits intoOCA:18.0from
qrtl:18.0-imp-report_configuration_product_secondary_unit
Open

[18.0][IMP] product_secondary_unit: add configuration for report presentation#2211
AungKoKoLin1997 wants to merge 2 commits intoOCA:18.0from
qrtl:18.0-imp-report_configuration_product_secondary_unit

Conversation

@AungKoKoLin1997
Copy link
Contributor

Add hidden security groups (group_sale_secondary_unit, group_purchase_secondary_unit) and company-level configuration fields for secondary unit price display and column visibility.

These groups and settings are defined here in the base module so they can be shared across Sale, Purchase, and Invoicing without forcing account_move_secondary_unit to depend on sale_order_secondary_unit or purchase_order_secondary_unit.

Each child module activates the relevant group(s) via implied_ids on install, which controls the visibility of the corresponding settings in the UI.

@qrtl QT6210

@OCA-git-bot
Copy link
Contributor

Hi @sergio-teruel,
some modules you are maintaining are being modified, check this out!

@yostashiro yostashiro force-pushed the 18.0-imp-report_configuration_product_secondary_unit branch from b59ecd9 to b569a94 Compare February 16, 2026 12:29
Copy link
Member

@yostashiro yostashiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review. LGTM.

yostashiro added a commit to qrtl/purchase-workflow that referenced this pull request Feb 16, 2026
… unmerged dependency

Partially revert a1eea87 for purchase order and quotation report
templates. The reverted portions rely on methods
(hide_secondary_uom_column, get_secondary_uom_display_mode) introduced
by OCA/product-attribute#2211, which is not yet merged. Without that
dependency, the reports would crash when printing.

The secondary quantity column is kept but the conditional visibility
and display mode logic are removed until the dependency is available.
@pedrobaeza pedrobaeza added this to the 18.0 milestone Feb 16, 2026
class ResCompany(models.Model):
_inherit = "res.company"

secondary_uom_price_display_sale = fields.Selection(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This configurability level seems a bit insane. Are you going to display UoM price on sale, but not on purchase, but show the UoM? Can't this be fit with a single security group "Show seconday unit prices"?

Copy link
Contributor Author

@AungKoKoLin1997 AungKoKoLin1997 Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For showing the UoM column option, it is intended only to preserve the existing Sales/Purchase report behavior of sale_order_secondary_unit and purchase_order_secondary_unit when Primary Unit Price Only is selected. For the other price display modes, the secondary UoM is already shown in the quantity and price columns, so showing an additional UoM column would be redundant.

I don't think Sales and Purchase should always share the same display style for the secondary unit price and the secondary unit column. Therefore, I prefer separate settings for Sales and Purchase documents, with separate security groups to control each document type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the expected report when Primary Unit Price Only is selected without hiding the UoM column.
image

This is the expected report when Prioritize Secondary Unit Price is selected.
image

This is the expected report when Both Primary and Secondary Unit Prices is selected.
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But as said, this is too many configurations, while it's not a usual problem to show both. This only increases the maintenance burden to the main module being a residual feature that most people won't use, being enough with the default option. I think this has to be simplified.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pedrobaeza To me it makes sense to be able to configure sales and purchasing separately as they have different needs. But we can do the secondary UoM price display thing in a separate module if it's too much to be covered in the main module, which will mean four new additional modules (i.e., product_secondary_unit_price, purchase_order_secondary_unit_price, sale_order_secondary_unit_price and account_move_secondary_unit_price).

What we still want to do in this module, though, is to be able to hide the secondary qty column in reports. For this, we don't need to have separate config for sales and purchasing (as we will most likely disable this for both anyway).

Does this sound like a good approach?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also came across this need: to choose if the secondary unit should be visible on sales document or purchase documents.
It can be the case that the feature helps pick quantities on order lines, but we don't want/need them on the PDFs.
And the decision can be different for sale or purchase.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AungKoKoLin1997 That's a feature change and should go in a separate commit, so that it can be backported. Thanks.

Copy link
Contributor Author

@AungKoKoLin1997 AungKoKoLin1997 Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dreispt For easier backporting, I’ll split this into two commits:

  • Commit 1: Hide column configuration (hide_secondary_uom_column_* fields + related logic)
  • Commit 2: Price display configuration (secondary_uom_price_display_* fields + related logic)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dreispt I split into two commits.

Copy link
Contributor Author

@AungKoKoLin1997 AungKoKoLin1997 Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sergio-teruel is the main creator of this module, so I let him to decide at last if he considers OK to have all the configuration tweaks in the main module.

@sergio-teruel Can you please review the changes in this PR? We’re planning to use this module for our customer and would like to proceed. If you think the current changes aren’t suitable, I can follow up based on @yostashiro suggestion above.

Copy link

@kanda999 kanda999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that the report display changes depending on the option settings in my local

Copy link
Contributor

@nobuQuartile nobuQuartile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functional review LGTM:
Check the report behaviour with the related purchase_order_secondary_unit PR code.

JochenDeBie pushed a commit to DynAppsNV/purchase-workflow that referenced this pull request Feb 26, 2026
… unmerged dependency

Partially revert a1eea87 for purchase order and quotation report
templates. The reverted portions rely on methods
(hide_secondary_uom_column, get_secondary_uom_display_mode) introduced
by OCA/product-attribute#2211, which is not yet merged. Without that
dependency, the reports would crash when printing.

The secondary quantity column is kept but the conditional visibility
and display mode logic are removed until the dependency is available.
@AungKoKoLin1997 AungKoKoLin1997 force-pushed the 18.0-imp-report_configuration_product_secondary_unit branch 2 times, most recently from fa61d03 to eef5439 Compare March 3, 2026 02:35
Add hidden security groups (group_sale_secondary_unit,
group_purchase_secondary_unit) and company-level configuration fields
to control the visibility of the "Second Qty" column in reports.

These groups and settings are defined here in the base module so they
can be shared across Sale, Purchase, and Invoicing without forcing
account_move_secondary_unit to depend on sale_order_secondary_unit
or purchase_order_secondary_unit.

Each child module activates the relevant group(s) via implied_ids
on install, which controls the visibility of the corresponding
settings in the UI.

Co-authored-by: Yoshi Tashiro <tashiro@quartile.co>
@AungKoKoLin1997 AungKoKoLin1997 force-pushed the 18.0-imp-report_configuration_product_secondary_unit branch 3 times, most recently from fcaf7e9 to 23d9e96 Compare March 3, 2026 03:07
Add company-level configuration fields for secondary unit price
display. This allows configuring how unit prices and quantities
are shown in reports when secondary units are used.

The price display can be configured to show:
- Primary unit price only (default)
- Secondary unit price (prioritized)
- Both primary and secondary unit prices

This builds on the hide column configuration to provide flexible
report presentation options.

Co-authored-by: Yoshi Tashiro <tashiro@quartile.co>
@AungKoKoLin1997 AungKoKoLin1997 force-pushed the 18.0-imp-report_configuration_product_secondary_unit branch from 23d9e96 to 96fdd1a Compare March 3, 2026 03:10
@AungKoKoLin1997 AungKoKoLin1997 requested a review from dreispt March 3, 2026 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants