Skip to content

T2860 - FIX invoice loop range and skip cancelled invoices on Odoo 18 - #277

Merged
ecino merged 1 commit into
18.0from
T2860-Annual-contracts-create-more-invoices-migrtion-to-18.0
Aug 3, 2026
Merged

T2860 - FIX invoice loop range and skip cancelled invoices on Odoo 18#277
ecino merged 1 commit into
18.0from
T2860-Annual-contracts-create-more-invoices-migrtion-to-18.0

Conversation

@danpa32

@danpa32 danpa32 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor
  • Fix off-by-one in _generate_invoices loop range: use advance_billing_months + month_interval - 1 as upper bound so the correct number of invoices is produced for all billing cycle combinations
  • Exclude cancelled invoices from _should_skip_invoice_generation so that a cancelled invoice no longer blocks regeneration for the same period
  • Expand readme (DESCRIPTION, USAGE, DEVELOP) with billing cycle docs

@danpa32
danpa32 requested review from NoeBerdoz and ecino June 24, 2026 15:08

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request updates the invoice generation loop upper bound to correctly handle monthly and annual billing cycles, excludes cancelled invoices from deduplication, and adds comprehensive documentation. However, a critical issue was identified where the implementation of _should_skip_invoice_generation still uses an exact match on invoice_date, contradicting the newly added documentation and design principles. It is recommended to update the query to use a date range covering the full billing interval.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread recurring_contract/models/contract_group.py
@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The changes are narrowly scoped to recurring invoice duplicate checks and regeneration behavior.

No correctness issues were identified in the reviewed change set, and the implementation aligns with the described invoice-regeneration behavior.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced a proof for a posted P1 finding and referenced the corresponding review comment for details.
  • T-Rex validated the invoice-range behavior; the before and after captures show the same result, confirming no change in this scenario, and the invoice-range-probe.py harness was used for the checks.
  • T-Rex validated the cancelled-invoice-regeneration behavior; the before-state lacked the state exclusion with matched_invoice_ids=[900] and should_skip_invoice_generation=True, while the after-state added the not-in state and produced no matches with should_skip_invoice_generation=False.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 Invoice generation loop still uses old upper bound, so advance billing with a starting offset skips the final cycle

    • Bug
      • The PR claims _generate_invoices fixes the off-by-one invoice generation range by using an upper bound equivalent to advance_billing_months + month_interval - 1, but head still contains range(starting_offset, group.advance_billing_months + 1, group.month_interval) at the invoice_offset loop. Executed before/after evidence shows no behavior change from base: with starting_offset=1, advance_billing_months=12, and month_interval=12, head generates only offset [1] / date 2026-02-01, while the claimed behavior would include [1, 13] / dates 2026-02-01 and 2027-02-01. The same mismatch appears for advance_billing_months=6, month_interval=3, where head produces [1, 4] instead of [1, 4, 7].
    • Cause
      • The intended range upper-bound change was not applied in recurring_contract/models/contract_group.py; line 308 still stops at group.advance_billing_months + 1, which excludes the next interval when starting_offset is nonzero.
    • Fix
      • Update the loop upper bound in _generate_invoices to the claimed interval-aware bound, e.g. range(starting_offset, group.advance_billing_months + group.month_interval, group.month_interval) or the exact equivalent required by the product spec, and add regression coverage for nonzero starting_offset with annual and quarterly intervals.

    T-Rex Ran code and verified through T-Rex

Reviews (5): Last reviewed commit: "[T2860] recurring_contract: exclude canc..." | Re-trigger Greptile

Comment thread recurring_contract/models/contract_group.py
@ecino

ecino commented Jul 16, 2026

Copy link
Copy Markdown
Member

@copilot When _calculate_start_date_and_offset() returns starting_offset = 1, the new upper bound can make the loop empty. With the default monthly setup (advance_billing_months = 1, month_interval = 1), this becomes range(1, 1, 1), so a waiting contract configured to skip the current month does not generate the next-month invoice at all. The previous loop still produced offset 1 for that path. Can you fix that PR?

Copilot AI commented Jul 16, 2026

Copy link
Copy Markdown

@copilot When _calculate_start_date_and_offset() returns starting_offset = 1, the new upper bound can make the loop empty. With the default monthly setup (advance_billing_months = 1, `month_inte...

Fixed in 3c8c592. I updated the loop upper bound to include starting_offset, so the starting_offset=1, monthly default path now generates offset 1 instead of producing an empty range.

Copilot finished work on behalf of ecino July 16, 2026 08:48
…for existing invoices

_should_skip_invoice_generation only matched on invoice_date/partner/contract/
product, so a cancelled invoice for a period still blocked regeneration for
that period. Add state != cancel to the search domain.
@danpa32
danpa32 force-pushed the T2860-Annual-contracts-create-more-invoices-migrtion-to-18.0 branch from 12f6b48 to 03283ea Compare July 16, 2026 14:01
@ecino
ecino merged commit 9e62632 into 18.0 Aug 3, 2026
2 checks passed
@ecino
ecino deleted the T2860-Annual-contracts-create-more-invoices-migrtion-to-18.0 branch August 3, 2026 10:12
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.

3 participants