Skip to content

[19.0] [FIX] partner_stage: rename res_partner.state -> stage_state#2367

Open
bosd wants to merge 1 commit into
OCA:19.0from
bosd:19.0-fix-partner_stage-state-column-conflict
Open

[19.0] [FIX] partner_stage: rename res_partner.state -> stage_state#2367
bosd wants to merge 1 commit into
OCA:19.0from
bosd:19.0-fix-partner_stage-state-column-conflict

Conversation

@bosd
Copy link
Copy Markdown
Contributor

@bosd bosd commented May 28, 2026

Summary

The stored related field res.partner.state (added in partner_stage/models/res_partner.py) collides with account_move.state in Odoo Enterprise's account_reports VAT report SQL.

account_reports.account_return._check_suite_common_vat_report issues:

SELECT ARRAY_AGG(move.id)
FROM account_move move
JOIN account_fiscal_position fpos ON fpos.id = move.fiscal_position_id
JOIN res_partner partner ON partner.id = move.commercial_partner_id
WHERE
    state = 'posted'
    AND move.company_id IN %(company_ids)s
    ...

The unqualified state = 'posted' matches both move.state and partner.state once partner_stage is installed, and Postgres raises:

psycopg2.errors.AmbiguousColumn: column reference "state" is ambiguous

This breaks the VAT closing checks on any deployment that combines partner_stage with Odoo Enterprise's account_reports.

Fix

Rename the field to stage_state — same store / related / readonly semantics, more descriptive name, no collision with downstream queries.

  • models/res_partner.py: field renamed.
  • migrations/19.0.1.2.0/pre-migrate.py: renames the Postgres column from state to stage_state for existing installations (idempotent guard against installations that already have stage_state from a fresh install or a re-run).
  • init_hook.py: the post-install SQL writes to the new column name.
  • Manifest bumped 19.0.1.1.0 → 19.0.1.2.0.

Test plan

  • Fresh install: column is created as stage_state, init_hook populates default stage cleanly.
  • Upgrade from 19.0.1.1.0: pre-migrate renames the existing state column to stage_state; data preserved.
  • Re-run upgrade: pre-migrate is a no-op because stage_state already exists.
  • Combined with Odoo Enterprise's account_reports: VAT report closing checks now run without an AmbiguousColumn error. (Verified by the maintainer on an FLC production database; PR-side CI does not have enterprise.)

Breaking change

The field is renamed. Any third-party that reads res.partner.state directly (rather than the canonical res.partner.stage_id.state) needs to follow the rename. A pre-migration could not be more clever than this without making assumptions about external consumers.

@OCA-git-bot
Copy link
Copy Markdown
Contributor

Hi @dreispt,
some modules you are maintaining are being modified, check this out!

Copy link
Copy Markdown
Contributor

@BhaveshHeliconia BhaveshHeliconia left a comment

Choose a reason for hiding this comment

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

Functional LGTM,
But last digit should be increased in manifest version for fix PRs

Copy link
Copy Markdown
Member

@dreispt dreispt left a comment

Choose a reason for hiding this comment

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

to bump MAJOR

@bosd bosd force-pushed the 19.0-fix-partner_stage-state-column-conflict branch from f37c4f5 to bc17147 Compare June 2, 2026 09:36
@bosd
Copy link
Copy Markdown
Contributor Author

bosd commented Jun 2, 2026

Addressed the review: bumped the manifest to a major version 19.0.2.0.0 (per @dreispt — a field rename res_partner.statestage_state is a breaking change) and moved the migration to migrations/19.0.2.0.0/ to match. @dreispt @BhaveshHeliconia ready when you are 🙏

@bosd bosd force-pushed the 19.0-fix-partner_stage-state-column-conflict branch from bc17147 to 1a42cc2 Compare June 2, 2026 09:39
@bosd
Copy link
Copy Markdown
Contributor Author

bosd commented Jun 2, 2026

Heads-up on a small but important change in the latest push: I made the migration idempotent / self-healing instead of a plain rename. The simple RENAME state -> stage_state only heals a DB where the legacy column is the only one — if a database already has stage_state (reinstall, or the field rename landed first), a stray state column can linger alongside it and keep the VAT report ambiguous. The new migration covers all four cases (state-only → rename, both → backfill+drop legacy, stage_state-only → noop, neither → ORM creates it). Verified in production where exactly the both-columns case recurred. ruff/pre-commit green.

Copy link
Copy Markdown
Contributor

@BhaveshHeliconia BhaveshHeliconia left a comment

Choose a reason for hiding this comment

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

LGTM!

@dreispt
Copy link
Copy Markdown
Member

dreispt commented Jun 2, 2026

Good for me, please check the test build.

The stored related res.partner.state field collided with
account_move.state in Odoo Enterprise's account_reports VAT report
SQL (_check_suite_common_vat_report), which issues an unqualified
WHERE state = 'posted' against a JOIN of account_move and
res_partner. Postgres bailed with::

    column reference "state" is ambiguous

Renamed the field (and its Postgres column) to stage_state. The
field still exists with the same store / related / readonly
semantics, just under a more specific name that does not collide
with downstream queries. Pre-migrate handles the column rename for
existing installations.

The companion init_hook write was updated to use the new column
name. Internal view references on res.partner.stage.state are
unaffected (different model).
@bosd bosd force-pushed the 19.0-fix-partner_stage-state-column-conflict branch from 1a42cc2 to 57ae13a Compare June 2, 2026 17:01
@bosd
Copy link
Copy Markdown
Contributor Author

bosd commented Jun 2, 2026

Pushed a test for the migration (tests/test_migration.py) covering all three reconciliation branches (state-only → rename, both → drop legacy, stage_state-only → noop) — the schema changes run inside the test transaction and roll back. This restores coverage (codecov/project) and re-triggers CI; the earlier red 'test with Odoo' was a flaked 'Initialize containers' (Docker/Postgres) step, not a test failure.

bosd pushed a commit to bosd/partner-contact that referenced this pull request Jun 2, 2026
partner_stage now exposes the partner stage value as `stage_state`
(res_partner.`state` was renamed to avoid the AmbiguousColumn collision with
account_move.state in the enterprise VAT report — see OCA#2367). Update the
injected Many2one domain and the tests to filter on `stage_state`.

Depends on OCA#2367
bosd pushed a commit to bosd/partner-contact that referenced this pull request Jun 2, 2026
partner_stage now exposes the partner stage value as `stage_state`
(res_partner.`state` was renamed to avoid the AmbiguousColumn collision with
account_move.state in the enterprise VAT report — see OCA#2367). Update the
injected Many2one domain and the tests to filter on `stage_state`.

Depends on OCA#2367
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.

4 participants