Skip to content

T3284 - remove recurring invoicer object - #279

Open
danpa32 wants to merge 4 commits into
18.0from
T3284-Remove-recurring-invoicer-object
Open

T3284 - remove recurring invoicer object#279
danpa32 wants to merge 4 commits into
18.0from
T3284-Remove-recurring-invoicer-object

Conversation

@danpa32

@danpa32 danpa32 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

The invoice generation code is complex. We historically use a recurring.invoicer model that keeps the history of generated invoices but this has no real benefit to our users and can be safely removed. It will also simplify the code and the database inprint.

See other PR related to it

Module 1 — recurring_contract (compassion-accounting)

The invoicer wizard and its menu were replaced by a server action. Verification:

  • Accounting → Contracts → Launch invoices generation (menu item): opens and runs without error — this now calls generate_from_cron() directly instead of the old wizard
  • Contract Group form → "Generate invoices" stat button: triggers job and generates invoices correctly
  • Confirm there is no "Generated invoices" menu anymore (the old invoicer list view was removed)

Module 2 — sponsorship_compassion (compassion-modules)

The invoicer was removed from _generate_invoices and _generate_gifts. Test from a Sponsorships → Sponsorships (S, SC, or SWP contract):

Contract Group form → "Generate invoices" button: invoices generate correctly AND birthday/Christmas gift invoices are still generated alongside regular invoices

Module 3 — sponsorship_switzerland (compassion-switzerland)

  • _build_invoice_gen_data override no longer receives the invoicer param. This is called during invoice line construction for Swiss-specific data. Test on a Swiss sponsorship contract:
  • Contract Group form → "Generate invoices" button: verify the generated invoices have correct Swiss-specific data (payment reference, bank details, etc.)

danpa32 and others added 4 commits July 1, 2026 13:47
The recurring.invoicer model stored batches of generated invoices but
provided no real user value — it was just a log of cron runs. Remove
the model, wizard, views, cron, and all related code. Invoice generation
now runs directly without creating an invoicer record.

Migration script drops the recurring_invoicer_id column from account_move
and drops the recurring_invoicer table.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add generate_from_cron() directly to ContractGroup to replace the
deleted InvoicerWizard, create a new daily cron targeting that method,
and replace the wizard-based menu action with an ir.actions.server action.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The XML ID action_invoice_automatic_generation existed in the DB as
ir.actions.act_window from the deleted wizard view. Pre-migration deletes
it so Odoo can recreate it as ir.actions.server during the upgrade.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@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 removes the recurring.invoicer model and its associated wizard, refactoring the invoice generation process to run directly on recurring.contract.group via a new cron method. It also introduces pre- and post-migration scripts to clean up the database. Feedback on the changes suggests deleting from ir_actions instead of ir_act_window in the pre-migration script to avoid leaving orphaned rows due to Odoo's inheritance structure, and adding self.ensure_one() to button_generate_invoices to prevent potential errors when executed on multi-record or empty recordsets.

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 on lines +12 to +20
cr.execute("""
DELETE FROM ir_act_window
WHERE id IN (
SELECT res_id FROM ir_model_data
WHERE module = 'recurring_contract'
AND name = 'action_invoice_automatic_generation'
AND model = 'ir.actions.act_window'
)
""")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

In Odoo, ir.actions.act_window inherits from ir.actions.actions via _inherits. The parent table is ir_actions and the child table is ir_act_window. Deleting directly from ir_act_window will leave a stale orphaned row in ir_actions because the foreign key cascade only works from parent to child. To clean up both tables properly, you should delete from ir_actions instead.

Suggested change
cr.execute("""
DELETE FROM ir_act_window
WHERE id IN (
SELECT res_id FROM ir_model_data
WHERE module = 'recurring_contract'
AND name = 'action_invoice_automatic_generation'
AND model = 'ir.actions.act_window'
)
""")
cr.execute("""
DELETE FROM ir_actions
WHERE id IN (
SELECT res_id FROM ir_model_data
WHERE module = 'recurring_contract'
AND name = 'action_invoice_automatic_generation'
AND model = 'ir.actions.act_window'
)
""")

Comment on lines 243 to +247
def button_generate_invoices(self):
"""Immediately generate invoices for the contract group."""
invoicer = (
self.with_context(queue_job__no_delay=True)
.with_company(self.company_id)
.generate_invoices()
before_invoice_ids = set(
self.mapped("active_contract_ids.invoice_line_ids.move_id").ids
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The button_generate_invoices method accesses self.company_id directly. If this method is ever called on an empty recordset or a recordset with multiple records (e.g., from a list view or server action), it will raise an error or behave unexpectedly. Adding self.ensure_one() at the beginning of the method ensures safe execution and adheres to defensive programming practices.

    def button_generate_invoices(self):
        """Immediately generate invoices for the contract group."""
        self.ensure_one()
        before_invoice_ids = set(
            self.mapped("active_contract_ids.invoice_line_ids.move_id").ids
        )

@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

This PR needs a migration fix before it is safe for upgraded databases.

The main refactor is localized and consistent, but upgraded databases can keep a scheduled action that points to the removed wizard model.

recurring_contract/migrations/18.0.1.1.0/pre-migration.py; recurring_contract/data/daily_invoice_generation_cron.xml

T-Rex T-Rex Logs

What T-Rex did

  • Reproduced the stale cron removal by running the focused migration harness against representative data and verified the migration deleted the old cron entries while the new cron remained.
  • Inspected the invoice menu action through before/after artifacts, confirming old Generated invoices XML IDs were present before and absent after, and that model.generate_from_cron() executed in the after state.
  • Validated the contract-group generate flow by comparing before and after states, noting the after state has the head invoice generated and has_recurring_invoicer_id changed to False with exit code 0.
  • Documented the Python-level repro path for the downstream override that produced a TypeError, indicating an argument-mismatch in the changed path.
  • Recorded the Swiss override path regression, where the signature change caused swiss_override to raise a missing-argument TypeError, reflecting a mismatch in expected parameters.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
recurring_contract/migrations/18.0.1.1.0/pre-migration.py Removes the stale act-window XML id before recreating it as a server action, but misses cleanup of the old recurring invoicer cron record.
recurring_contract/data/daily_invoice_generation_cron.xml Renames the daily cron to call recurring.contract.group.generate_from_cron; migration must also handle existing old cron XML records.
recurring_contract/models/contract_group.py Moves cron and menu invoice generation onto contract groups and removes invoicer tracking from the generation flow.
recurring_contract/views/recurring_contract_view.xml Adds the replacement server action and menu item that invoke contract group invoice generation directly.
recurring_contract/migrations/18.0.1.1.0/post-migration.py Drops the obsolete recurring_invoicer_id column and recurring_invoicer table after model removal.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User as Accounting User / Cron
participant Action as Server Action / ir.cron
participant Group as recurring.contract.group
participant Queue as queue_job
participant Move as account.move

User->>Action: Launch invoices generation
Action->>Group: generate_from_cron()
Group->>Group: search active, non-suspended groups
Group->>Queue: with_delay_sh(_generate_invoices)
Queue->>Group: _generate_invoices()
Group->>Group: _build_invoice_gen_data(invoicing_date)
Group->>Move: create(inv_data)
Move-->>Group: generated invoice
Group->>Move: action_post()
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User as Accounting User / Cron
participant Action as Server Action / ir.cron
participant Group as recurring.contract.group
participant Queue as queue_job
participant Move as account.move

User->>Action: Launch invoices generation
Action->>Group: generate_from_cron()
Group->>Group: search active, non-suspended groups
Group->>Queue: with_delay_sh(_generate_invoices)
Queue->>Group: _generate_invoices()
Group->>Group: _build_invoice_gen_data(invoicing_date)
Group->>Move: create(inv_data)
Move-->>Group: generated invoice
Group->>Move: action_post()
Loading

Comments Outside Diff (2)

  1. General comment

    P1 Removed _generate_invoices parameter breaks sponsorship_compassion gift generation override

    • Bug
      • sponsorship_compassion overrides recurring.contract.group._generate_invoices(self, invoicer) and calls super(...)._generate_invoices(invoicer) before generating birthday and Christmas gifts. In head, recurring_contract/models/contract_group.py changes the base method to def _generate_invoices(self) with no invoicer parameter. The downstream override therefore raises TypeError when it calls super with the invoicer argument, preventing the Contract Group Generate invoices path from reaching _generate_gifts for birthday/Christmas gifts.
    • Cause
      • The accounting method contract in recurring_contract/models/contract_group.py was changed by removing the invoicer/invoicer_id parameter from _generate_invoices without preserving backwards-compatible acceptance for downstream overrides that still pass it.
    • Fix
      • Keep _generate_invoices backwards-compatible by accepting an optional ignored parameter (for example def _generate_invoices(self, invoicer_id=False): or *args/**kwargs) during the migration, and update downstream sponsorship_compassion to stop passing the removed invoicer once gift generation no longer depends on recurring.invoicer.

    T-Rex Ran code and verified through T-Rex

  2. General comment

    P1 Swiss sponsorship invoice generation override breaks after _build_invoice_gen_data invoicer parameter removal

    • Bug
      • The related sponsorship_switzerland module overrides recurring.contract.group._build_invoice_gen_data with signature (self, invoicing_date, invoicer, gift_wizard=False). On base, the Contract Group Generate invoices path calls the method with both invoicing_date and invoicer, allowing the Swiss override to add ref, mandate_id, partner_bank_id, and payment_reference. On head, recurring_contract/models/contract_group.py now calls self._build_invoice_gen_data(invoicing_date), so the unchanged Swiss override raises TypeError before invoice data is created.
    • Cause
      • The PR removed the invoicer parameter from the base method call/signature in recurring_contract/models/contract_group.py without preserving backward compatibility for downstream overrides that still require the positional parameter.
    • Fix
      • Either preserve compatibility by continuing to pass an optional invoicer argument or adapt the contract with a compatibility shim, and update downstream sponsorship_switzerland.models.contract_group._build_invoice_gen_data to accept the new signature, e.g. (self, invoicing_date, gift_wizard=False) or a tolerant *args/**kwargs transition. Ensure the super() call in the Swiss override matches the new base signature.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "Merge branch '18.0' into T3284-Remove-re..." | Re-trigger Greptile

Comment on lines +12 to +26
cr.execute("""
DELETE FROM ir_act_window
WHERE id IN (
SELECT res_id FROM ir_model_data
WHERE module = 'recurring_contract'
AND name = 'action_invoice_automatic_generation'
AND model = 'ir.actions.act_window'
)
""")
cr.execute("""
DELETE FROM ir_model_data
WHERE module = 'recurring_contract'
AND name = 'action_invoice_automatic_generation'
AND model = 'ir.actions.act_window'
""")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Remove stale cron
The migration deletes the old action_invoice_automatic_generation act-window XML record before recreating it, but it leaves the old recurring_invoicer_cron record from the removed daily_invoicer_cron.xml. Existing databases will keep that noupdate cron pointing at model_recurring_invoicer_wizard, while this PR removes the wizard model and adds a new invoice_generation_cron, so upgrades retain a broken and duplicate scheduled action. Delete or retarget the old cron and its XML id during migration.

Artifacts

Repro: focused migration harness that seeds old and new cron XML IDs and runs pre-migration.py

  • Contains supporting evidence from the run (text/x-python; charset=utf-8).

Repro: command transcript and before-after SQL rows showing recurring_invoicer_cron survives after migration

  • Keeps the command output available without making the summary code-heavy.

View artifacts

T-Rex Ran code and verified through T-Rex

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.

1 participant