Skip to content

Conversation

@AungKoKoLin1997
Copy link
Contributor

@AungKoKoLin1997 AungKoKoLin1997 commented Jan 5, 2023

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.

@yostashiro
Copy link
Member

yostashiro commented Jan 5, 2023

This migration depend on this PR (#697)

I think we should treat this PR as an addition instead of migration.

@AungKoKoLin1997 AungKoKoLin1997 force-pushed the 16.0-mig-report_qweb_decimal_place branch from ea673b6 to 08e1ad2 Compare January 5, 2023 04:13
@AungKoKoLin1997 AungKoKoLin1997 changed the title [16.0][MIG] report_qweb_decimal_place: Migration to 16.0 [16.0][ADD] report_qweb_decimal_place: Migration to 16.0 Jan 5, 2023
@AungKoKoLin1997 AungKoKoLin1997 changed the title [16.0][ADD] report_qweb_decimal_place: Migration to 16.0 [16.0][ADD] report_qweb_decimal_place Jan 5, 2023
Comment on lines 4 to 8
<span
class="text-nowrap"
t-esc="price_unit"
t-options='{"widget": "float", "precision": currency.price_decimal_places}'
/>
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
<span
class="text-nowrap"
t-esc="price_unit"
t-options='{"widget": "float", "precision": currency.price_decimal_places}'
/>
<t t-if="currency.apply_price_decimal_place">
<span
class="text-nowrap"
t-esc="price_unit"
t-options='{"widget": "float", "precision": currency.price_decimal_places}'
/>
</t>

As we talked earlier, to minimize the code in the extension of individual reports.

Comment on lines 18 to 22
<t t-if="doc.currency_id.apply_price_decimal_place">
<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" />
</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="doc.currency_id.apply_price_decimal_place">
<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" />
</t>
<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" />

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.

👍

@AungKoKoLin1997 AungKoKoLin1997 force-pushed the 16.0-mig-report_qweb_decimal_place branch from 6bb70df to b15ee48 Compare May 19, 2023 02:04
@AungKoKoLin1997 AungKoKoLin1997 force-pushed the 16.0-mig-report_qweb_decimal_place branch from b15ee48 to 17e1522 Compare May 19, 2023 02:05
@OCA-git-bot
Copy link
Contributor

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). 🤖

@thomaspaulb
Copy link

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-709-by-thomaspaulb-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Jul 14, 2023
Signed-off-by thomaspaulb
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 16.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 16.0-ocabot-merge-pr-709-by-thomaspaulb-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 1c28b48 into OCA:16.0 Jul 14, 2023
@OCA-git-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants