Skip to content

[16.0][ADD] sale_invoice_plan_report: add new module - #364

Open
kaynnan wants to merge 1 commit into
OCA:16.0from
Escodoo:16.0-add-sale_invoice_plan_report
Open

[16.0][ADD] sale_invoice_plan_report: add new module#364
kaynnan wants to merge 1 commit into
OCA:16.0from
Escodoo:16.0-add-sale_invoice_plan_report

Conversation

@kaynnan

@kaynnan kaynnan commented Apr 6, 2026

Copy link
Copy Markdown

@kaynnan
kaynnan force-pushed the 16.0-add-sale_invoice_plan_report branch 3 times, most recently from 77a477a to d18e54a Compare April 6, 2026 19:04

@CristianoMafraJunior CristianoMafraJunior left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@kaynnan
kaynnan force-pushed the 16.0-add-sale_invoice_plan_report branch 2 times, most recently from f4cbf69 to dcafbae Compare April 6, 2026 19:27
@rousseldenis rousseldenis added this to the 16.0 milestone Apr 7, 2026
<!-- Total Planned -->
<div class="col-sm-4 mb-2">
<div
style="background: #F8F0F5; border: 1px solid #E5D5DE; border-radius: 6px; padding: 10px 14px;"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@kaynnan Could you maybe add class and css instead of style ? Thanks

@OCA-git-bot

Copy link
Copy Markdown
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). 🤖

@alexey-pelykh alexey-pelykh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice module, @kaynnan — it renders cleanly and the smoke tests in tests/test_report.py cover the main paths (section shown/hidden, advance vs installment, currency marker). One non-blocking observation on views/report_saleorder.xml:

The two summary cards format their totals as plain numbers, e.g.

<t t-esc="'{:,.2f}'.format(sum(line.amount_invoiced if line.invoiced else 0 for line in doc.invoice_plan_ids))"/>

so "Invoiced" and "Pending" render with no currency symbol and hard-coded ,/. separators, whereas the table below correctly uses the monetary widget with the order currency:

<span t-field="plan_line.amount" t-options='{"widget": "monetary", "display_currency": doc.currency_id}'/>

On a non-USD order (or a locale with different separators) the card totals and the table amounts will disagree in formatting. Worth routing the card totals through the monetary widget / doc.currency_id too for consistency.

(+1 to @rousseldenis's point about moving the inline style= to CSS classes — same spirit.)

Co-Reviewed-By: Claude Opus 4.8 noreply@anthropic.com

@kaynnan
kaynnan force-pushed the 16.0-add-sale_invoice_plan_report branch from dcafbae to 807977b Compare July 8, 2026 10:48
@OCA-git-bot OCA-git-bot added series:16.0 mod:sale_invoice_plan_report Module sale_invoice_plan_report labels Jul 8, 2026
@kaynnan
kaynnan force-pushed the 16.0-add-sale_invoice_plan_report branch from 807977b to 08f9f8b Compare July 8, 2026 11:02

@rousseldenis rousseldenis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code and functional review.

Nice.

@@ -0,0 +1,11 @@
This module extends the sales order/quotation PDF report to include invoice plan information when the order is configured to use invoice plans.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@kaynnan Don't hesitate to include screenshots as this will help module adoption and make them better on app store

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.

6 participants