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

[16.0][ADD] report_qweb_decimal_precision #842

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

yostashiro
Copy link
Sponsor Member

@yostashiro yostashiro commented Jan 13, 2024

This module allows administrators to define the decimal precision of float fields for QWeb report presentation.

Functionality wise, this basically replaces https://github.com/OCA/reporting-engine/tree/16.0/report_qweb_decimal_place.

Tests need to be added.

Configuration:
image

Result:
image

@qrtl

Copy link

@len-foss len-foss left a comment

Choose a reason for hiding this comment

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

Code LGTM

Feature is a very nice improvement :-)

Comment on lines 16 to 21
score = 0
precision_rec = False
for dp_qweb_rec in dp_qweb_recs:
score_rec = dp_qweb_rec._get_score(record)
if score_rec > score:
precision_rec = dp_qweb_rec

Choose a reason for hiding this comment

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

Suggested change
score = 0
precision_rec = False
for dp_qweb_rec in dp_qweb_recs:
score_rec = dp_qweb_rec._get_score(record)
if score_rec > score:
precision_rec = dp_qweb_rec
precision_rec = max(dp_qweb_recs, default=None, key=lambda r: r._get_score(record))

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

@len-foss Thanks for the suggestion! Updated the code.

@yostashiro yostashiro force-pushed the 16.0-add-report_qweb_decimal_precision branch 2 times, most recently from c6a99f7 to 6c57cfe Compare January 22, 2024 01:43
Copy link
Contributor

@AungKoKoLin1997 AungKoKoLin1997 left a comment

Choose a reason for hiding this comment

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

LGTM.

@yostashiro yostashiro force-pushed the 16.0-add-report_qweb_decimal_precision branch from 6c57cfe to 0da5173 Compare February 8, 2024 02:31
@yostashiro yostashiro force-pushed the 16.0-add-report_qweb_decimal_precision branch from 0da5173 to f51b1bc Compare February 17, 2024 06:20
@liuhehe1995
Copy link
Contributor

It worked as expected.
Functional tests were performed.(purchase.order.line/sale.order.line/account.move.line)

@yostashiro yostashiro force-pushed the 16.0-add-report_qweb_decimal_precision branch from f51b1bc to 502ab25 Compare February 19, 2024 13:34
Copy link
Contributor

@liuhehe1995 liuhehe1995 left a comment

Choose a reason for hiding this comment

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

LGTM.

@HviorForgeFlow
Copy link
Member

Hello @yostashiro and @AungKoKoLin1997 I can see that both module that you are proposing share similar functionalities. Should I merge this or should we go for #857?

I can see that report_qwebt_field_converter is kind of more generic feature.

Feedback welcome

CC @OCA/reporting-engine-maintainers

@yostashiro
Copy link
Sponsor Member Author

@HviorForgeFlow Indeed, #857 intends to supersede this PR if it works out nicely. We just haven't gotten around to do a review on that PR, which we will follow up in the coming days.

@HviorForgeFlow
Copy link
Member

Ok thanks for your quick feedback, maybe this PR can be closed then.

Copy link
Member

@HviorForgeFlow HviorForgeFlow left a comment

Choose a reason for hiding this comment

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

Pending superseed #857

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants