Skip to content

[FIX] stock_account: avoid creating blank account.account records - #5885

Merged
MiquelRForgeFlow merged 1 commit into
OCA:19.0from
komit-consulting:19.0-fix-stock_account-empty-company-data
Aug 5, 2026
Merged

[FIX] stock_account: avoid creating blank account.account records#5885
MiquelRForgeFlow merged 1 commit into
OCA:19.0from
komit-consulting:19.0-fix-stock_account-empty-company-data

Conversation

@quoc-pn

@quoc-pn quoc-pn commented Aug 5, 2026

Copy link
Copy Markdown
Member

Bug

update_from_coa_generic() in stock_account/19.0.1.1/end-migration.py decided whether to include a record in company_data based on the raw chart template data (the outer any(record_data.get(key) for key in field_names) check), while the actual values to load were filtered separately based on whether the existing account already had the field set (not ref_or_id(record_id, model_name)[key]).

When an account already had account_stock_expense_id/account_stock_variation_id set on the existing record, the inner per-record dict ended up empty ({}) — but the record was still included in company_data because the outer check only looked at the raw template data.

AccountChartTemplate._load_data(company_data) then tried to load that empty vals dict. Since the xmlid didn't yet resolve to an existing record, it was treated as "to create", producing a new account.account row with only ORM defaults (no name, account_type, etc.), which crashes with:

null value in column "name" of relation "account_account" violates not-null constraint

Fix

Only include a record in company_data when the filtered per-record dict is actually non-empty, instead of relying on a separate check against the raw template data.

Test

Reproduced during a 19.0 migration where stock_account's end-migration.py crashed on AccountChartTemplate._load_data() with the above error; confirmed the crash disappears with this fix applied.

update_from_coa_generic() decided whether to include a record in
company_data based on the raw chart template data (outer `any()`
check), but the actual values loaded were filtered separately based
on whether the existing account already had the field set. When an
account already had account_stock_expense_id/account_stock_variation_id
set, the per-record dict ended up empty while the record was still
included in company_data.

_load_data() then created a brand new account.account from that empty
vals dict (since the xmlid did not resolve to an existing record yet),
producing a row with only ORM defaults and no name, which violates the
NOT NULL constraint on account_account.name.

Only include a record in company_data when it actually has filtered
values to load.
@OCA-git-bot OCA-git-bot added mod:openupgrade_scripts Module openupgrade_scripts series:19.0 labels Aug 5, 2026
@MiquelRForgeFlow MiquelRForgeFlow added this to the 19.0 milestone Aug 5, 2026

@MiquelRForgeFlow MiquelRForgeFlow 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.

Makes sense.

@MiquelRForgeFlow
MiquelRForgeFlow requested a review from hbrunn August 5, 2026 11:00
@MiquelRForgeFlow

Copy link
Copy Markdown
Contributor

Thanks @quoc-pn. BTW, why PR is in draft?

@quoc-pn
quoc-pn marked this pull request as ready for review August 5, 2026 11:02
@quoc-pn

quoc-pn commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Thanks @quoc-pn. BTW, why PR is in draft?

@MiquelRForgeFlow Thanks for your review. I marked the PR as ready.

@MiquelRForgeFlow
MiquelRForgeFlow merged commit 571ccbb into OCA:19.0 Aug 5, 2026
7 checks passed
if key in field_names and not ref_or_id(record_id, model_name)[key]
}
for record_id, record_data in template_data[model_name].items()
if any(record_data.get(key) for key in field_names)

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.

@quoc-pn @MiquelRForgeFlow

why is this check removed? It avoids crashes for COAs that don't define the field in question.

And obviously generated PRs need to conform https://github.com/OCA/.github/blob/master/AI_POLICY.md

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.

Good catch, fixed in #5888.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:openupgrade_scripts Module openupgrade_scripts series:19.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants