Skip to content

[FIX] account_statement_import_file: avoid duplicate and slow statement imports - #979

Open
feg-adhoc wants to merge 1 commit into
OCA:19.0from
adhoc-dev:19.0-h-123605-feg
Open

[FIX] account_statement_import_file: avoid duplicate and slow statement imports#979
feg-adhoc wants to merge 1 commit into
OCA:19.0from
adhoc-dev:19.0-h-123605-feg

Conversation

@feg-adhoc

@feg-adhoc feg-adhoc commented Jul 23, 2026

Copy link
Copy Markdown

Problem

Importing a bank statement file can create duplicate bank statements (the same file imported several times). From the user's point of view the import seems to "loop" and never confirm — even though they clicked Import only once.

Root cause

This is a chain of events; the duplication is the important part:

  1. Creating the statement can be slow. Two contributors:
    • Odoo core assigns a sequence/name to every account.move created for the statement lines (per-move work that also grows with the journal history).
    • When account_accountant (Enterprise) is installed, account.bank.statement.create() synchronously renders a full PDF of the statement through wkhtmltopdf on every create, unless the context flag skip_pdf_attachment_generation is set. This is very slow and memory-heavy for large statements.
  2. If the request takes longer than the reverse proxy / ingress timeout (deployment-specific, outside Odoo's control), the proxy drops the connection.
  3. The Odoo web client treats the dropped connection as "connection lost" and automatically re-sends the same RPC when it reconnects.
  4. import_file_button is not idempotent: each re-sent request re-runs the whole import and creates another statement. Sheet-based mappings without a unique_import_id column have no per-line dedup, so nothing stops the duplicates.

Net effect: a single user click produces several identical statements and a "looping" UI.

Fix

Two changes, both scoped to the import:

  1. Skip the Enterprise synchronous PDF render during import — set skip_pdf_attachment_generation in the statement creation context. The imported source file is already attached to the statement, and the PDF can still be generated manually afterwards. No dependency on account_accountant is added (it is a no-op when that module is absent).

  2. Make import_file_button idempotent:

    • a transaction-scoped advisory lock keyed on the file content serializes concurrent imports of the same file — a re-sent request waits for the first one to finish;
    • it then returns the already-imported statement (matched by the attachment checksum) instead of creating a duplicate.

    As a result, a user who triggered a single import never sees an error and never ends up with duplicates, regardless of how the proxy behaves.

Why fix idempotency, and not "why does the import re-run / recompute in the first place?"

This is a fair review question, so to address it up front — we deliberately patch idempotency instead of chasing the underlying slowness first:

  • The duplication is a correctness bug that is independent of performance. The trigger is a non-idempotent write action sitting behind a web client that auto-retries on connection loss. Any import slow enough to exceed any proxy timeout will duplicate; making the import faster only changes how often it happens, not whether it can happen. The proxy timeout is deployment-specific and is not something this module controls.
  • The slowness does not originate in this module. The per-move sequence assignment / recomputation lives in Odoo core account.move; the synchronous PDF render is Enterprise account_accountant behaviour. Optimising core move creation is a large, risky, cross-module effort — and even a much faster import would still leave the correctness hole open (a short or misconfigured proxy timeout would still duplicate).
  • Therefore the correct fix for "a non-idempotent action re-run by an auto-retrying client" is to make the action idempotent and serialized, which is what this PR does. Reducing the underlying import cost is a legitimate but separate concern (Odoo core / a dedicated performance effort) and is orthogonal to the data-integrity fix here.

Note on behaviour

Re-importing the exact same file now returns the already-imported statement instead of creating a new one — consistent with the module's existing dedup intent (unique_import_id). To genuinely re-import a file, remove the previous statement first.

Copilot AI review requested due to automatic review settings July 23, 2026 19:56
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @alexis-via,
some modules you are maintaining are being modified, check this out!

Copilot AI 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.

Pull request overview

This PR improves the performance and reliability of bank statement file imports by preventing Odoo Enterprise (account_accountant) from synchronously rendering and attaching a statement PDF during account.bank.statement.create(), which can be slow and resource-intensive for large imports.

Changes:

  • Add skip_pdf_attachment_generation to the create context when creating imported bank statements, to avoid Enterprise’s heavy synchronous PDF rendering during import.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

st_vals["line_ids"] = [[0, False, line] for line in st_lines_to_create]
# Skip the Enterprise synchronous PDF render on create, too heavy
# for large imports (no-op if account_accountant is not installed).
context.setdefault("skip_pdf_attachment_generation", True)
@feg-adhoc
feg-adhoc force-pushed the 19.0-h-123605-feg branch 4 times, most recently from 34e81c1 to e0e45cd Compare July 24, 2026 17:55
@feg-adhoc feg-adhoc changed the title [FIX] account_statement_import_file: skip Enterprise auto-PDF attachment on import [FIX] account_statement_import_file: avoid duplicate and slow statement imports Jul 24, 2026
@feg-adhoc
feg-adhoc force-pushed the 19.0-h-123605-feg branch 2 times, most recently from 7efe914 to 7df490f Compare July 24, 2026 18:31
account_accountant (Enterprise) overrides account.bank.statement.create()
to synchronously render a full PDF of the statement via wkhtmltopdf, unless
the context flag skip_pdf_attachment_generation is set.

For large imported statements this render is prohibitively slow and heavy
(~17ms/line; several seconds for a few hundred lines), causing request
timeouts, dropped connections and serialization retries that re-run the
whole import, so nothing gets imported. In environments with a broken
wkhtmltopdf it fails outright.

Set skip_pdf_attachment_generation when creating statements during import.
The imported source file is already attached to the statement, and the PDF
can still be generated manually. No-op when account_accountant is absent.
@feg-adhoc
feg-adhoc force-pushed the 19.0-h-123605-feg branch from 7df490f to 63a1920 Compare July 24, 2026 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:account_statement_import_file Module account_statement_import_file series:19.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants