Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[15.0][ADD] report_qweb_decimal_place #697

Closed

Conversation

AungKoKoLin1997
Copy link
Contributor

@AungKoKoLin1997 AungKoKoLin1997 commented Dec 13, 2022

This module intends to provide the base function for currencies to adjust the number of decimal places for the unit price in QWeb reports.
@qrtl

Comment on lines 1 to 3
To apply price unit format:
- Go to Invoicing --> Configuration --> Currencies
- Check apply_price_unit_format field
Copy link
Member

Choose a reason for hiding this comment

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

Please move this to CONFIGURE.rst.

"version": "15.0.1.0.0",
"author": "Quartile Limited, Odoo Community Association (OCA)",
"website": "https://github.com/OCA/reporting-engine",
"license": "AGPL-3",
Copy link
Member

Choose a reason for hiding this comment

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

Please make this LGPL.

or
not currency.rounding >= 1"
>
<span t-esc="'{:,.2f}'.format(price_unit)" />
Copy link
Member

Choose a reason for hiding this comment

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

Can we go a bit further than fixing it at the second decimal? Let's add price_decimal_place field in res.currency and take the value from there.

Comment on lines 11 to 15
<t
t-if="round(price_unit) != price_unit
or
not currency.rounding >= 1"
>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<t
t-if="round(price_unit) != price_unit
or
not currency.rounding >= 1"
>

Let's get rid of this condition.

Comment on lines 18 to 24
<t
t-if="round(price_unit) == price_unit
and
currency.rounding >= 1"
>
<span t-esc="'{:,.0f}'.format(price_unit)" />
</t>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<t
t-if="round(price_unit) == price_unit
and
currency.rounding >= 1"
>
<span t-esc="'{:,.0f}'.format(price_unit)" />
</t>

Let's get rid of this for simplicity.

class ResCurrency(models.Model):
_inherit = "res.currency"

apply_price_unit_format = fields.Boolean()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
apply_price_unit_format = fields.Boolean()
apply_price_decimal_place = fields.Boolean(help="Apply this decimal place to the unit price field of relevant PDF reports where appropriate customization is done.")

Comment on lines 1 to 3
This module does the following:

- Adds apply_price_unit_format in currency and the price_unit format of reports will depend on that field.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This module does the following:
- Adds apply_price_unit_format in currency and the price_unit format of reports will depend on that field.
This module intends to provide the base function for currencies to adjust the number of decimal places for the unit price in QWeb reports.
Installing this module alone does not affect the presentation of existing QWeb reports. Individual adjustments need to be done in separate modules in a manner similar to the following:
.. code-block:: xml
<?xml version="1.0" encoding="utf-8" ?>
<odoo>
<template
id="report_saleorder_document_inherit"
inherit_id="sale.report_saleorder_document"
>
<xpath expr="//tbody[@class='sale_tbody']//tr//td[3]" position="replace">
<td name="td_priceunit" class="text-right">
<t t-set="currency" t-value="doc.currency_id" />
<t t-set="price_unit" t-value="line.price_unit" />
<t t-call="report_qweb_decimal_place.price_unit_value_format" />
</td>
</xpath>
</template>
</odoo>
Background:
--------------
Odoo default reports display price unit with the decimal accuracy of product price configuration. However, globally applying the decimal accuracy setting is sometimes not appropriate under multi-currency settings where how unit prices should be presented differs depending on the currency.
For example, unit prices in JPY usually do not have decimals (with some exceptions depending on the industry), while those in USD may require up to 2-4 decimals. If we configure the decimal accuracy based on USD, the unit price presentation on PDF reports for JPY transactions may appear a bit unconventional.

@AungKoKoLin1997 AungKoKoLin1997 force-pushed the 15.0-add_report_qweb_decimal_place branch from 8e05667 to 5113448 Compare December 14, 2022 07:48
@AungKoKoLin1997 AungKoKoLin1997 changed the title [ADD] report_qweb_decimal_place [15.0][ADD] report_qweb_decimal_place Dec 14, 2022
</odoo>

Background:
--------------
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
--------------
-----------

-->
<t t-if="currency.apply_price_decimal_place">
<span
class="text-nowrap"
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this class assignment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even we add this class or not, there is no huge impact. This just change a little spacing and I added this class because price_unit of sale_order_document don't use this class but others use this class like invoice(https://github.com/odoo/odoo/blob/23ef75f9250e6920e592b14a0cec2d1d9545824e/addons/account/views/report_invoice.xml#L85)

Comment on lines 8 to 22
<?xml version="1.0" encoding="utf-8" ?>
<odoo>
<template
id="report_saleorder_document_inherit"
inherit_id="sale.report_saleorder_document"
>
<xpath expr="//tbody[@class='sale_tbody']//tr//td[3]" position="replace">
<td name="td_priceunit" class="text-right">
<t t-set="currency" t-value="doc.currency_id" />
<t t-set="price_unit" t-value="line.price_unit" />
<t t-call="report_qweb_decimal_place.price_unit_value_format" />
</td>
</xpath>
</template>
</odoo>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<?xml version="1.0" encoding="utf-8" ?>
<odoo>
<template
id="report_saleorder_document_inherit"
inherit_id="sale.report_saleorder_document"
>
<xpath expr="//tbody[@class='sale_tbody']//tr//td[3]" position="replace">
<td name="td_priceunit" class="text-right">
<t t-set="currency" t-value="doc.currency_id" />
<t t-set="price_unit" t-value="line.price_unit" />
<t t-call="report_qweb_decimal_place.price_unit_value_format" />
</td>
</xpath>
</template>
</odoo>
<template
id="report_saleorder_document_inherit"
inherit_id="sale.report_saleorder_document"
>
<xpath expr="//td[@name='td_priceunit']/span" position="replace">
<t t-set="currency" t-value="doc.currency_id" />
<t t-set="price_unit" t-value="line.price_unit" />
<t t-call="report_qweb_decimal_place.price_unit_value_format" />
</xpath>
</template>

Please double-check if this suggestion is valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. This suggestion is valid.

Comment on lines 4 to 9
<!--
It should define the price_unit and currency value.
e.g.
<t t-set="price_unit" t-value="line.price_unit" />
<t t-set="currency" t-value="o.currency_id" />
-->
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<!--
It should define the price_unit and currency value.
e.g.
<t t-set="price_unit" t-value="line.price_unit" />
<t t-set="currency" t-value="o.currency_id" />
-->

We can remove this as it's explained in readme.

</template>

Background:
-----------
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
-----------
~~~~~~~~~~~

Pay attention to the blank line as well.

ref. OCA/maintainer-tools#455 (comment)

Comment on lines 1 to 4
To apply price unit format:
- Go to Invoicing --> Configuration --> Currencies
- Check apply_price_decimal_place field
- Define decimal place in price_decimal_place field
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
To apply price unit format:
- Go to Invoicing --> Configuration --> Currencies
- Check apply_price_decimal_place field
- Define decimal place in price_decimal_place field
To apply price unit format:
#. Go to Invoicing --> Configuration --> Currencies
#. Check apply_price_decimal_place field
#. Define decimal place in price_decimal_place field

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. 👍

@github-actions
Copy link

github-actions bot commented May 7, 2023

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 7, 2023
@github-actions github-actions bot closed this Jun 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants