[13.0][OU-FIX] hr_contract: fill contract_id for every employee, not just one

[muhammadali124]

What's wrong

fill_employee_contract_id() in addons/hr_contract/migrations/13.0.1.0/post-migration.py is meant to backfill the new (in 13.0) hr.employee.contract_id field for every employee holding a contract in state = 'open', since it's a plain Many2one and has no way to be populated by the schema migration itself.

The current query:

UPDATE hr_employee he
SET contract_id = sub.contract_id
FROM (
    SELECT he.id AS employee_id, hc.id as contract_id
    FROM hr_contract hc, hr_employee he
    WHERE he.id = hc.employee_id AND hc.state = 'open'
    LIMIT 1
) sub
WHERE sub.employee_id = he.id AND he.contract_id IS NULL

The inner SELECT is a non-correlated subquery — its own he/hc aliases are scoped entirely to itself and never reference the outer UPDATE hr_employee he row. In PostgreSQL, UPDATE ... FROM (subquery) ... evaluates the subquery once, as an independent derived table (this would require a LATERAL join to be evaluated per outer row, which it isn't).

Combined with LIMIT 1 and no ORDER BY, the subquery returns at most one row, globally, across the entire database — one arbitrary (employee_id, contract_id) pair, picked by whatever order the query planner happens to produce.

The outer UPDATE's join (sub.employee_id = he.id) can then only ever match the single hr_employee row whose id equals that one arbitrary employee_id. Every other employee with a genuinely open contract is left with contract_id = NULL.

Impact

In any database with more than one employee holding an open contract at migration time — the normal case for any live company — this script links exactly one employee, chosen arbitrarily, and silently leaves every other employee's contract_id unset.

Downstream effect: hr.employee.contract_warning is a stored computed field (@api.depends('contract_id', 'contract_id.state', 'contract_id.kanban_state')) that evaluates not employee.contract_id as True for every employee this script failed to link, surfacing a persistent, incorrect "there is something wrong with the contract" warning on the employee's card even though the contract itself is genuinely open and correct.

Why this went unnoticed in small-scale testing

If a test database happens to have exactly one employee with an open contract, the non-correlated LIMIT 1 subquery has no competing row to arbitrarily drop — it deterministically returns that one row, and the bug produces a correct-looking result by coincidence. The failure only becomes visible once more than one employee genuinely qualifies.

The fix

Replace the non-correlated LIMIT 1 with DISTINCT ON (employee_id) ... ORDER BY employee_id, id, which correlates one contract per employee deterministically and updates every qualifying employee, not just one:

UPDATE hr_employee he
SET contract_id = sub.contract_id
FROM (
    SELECT DISTINCT ON (hc.employee_id)
        hc.employee_id, hc.id AS contract_id
    FROM hr_contract hc
    WHERE hc.state = 'open'
    ORDER BY hc.employee_id, hc.id
) sub
WHERE sub.employee_id = he.id AND he.contract_id IS NULL

How this was found

Found while manually verifying a 12.0 → 13.0 upgrade rehearsal. A single test contract was deliberately set to open pre-migration; post-migration, the employee's card showed a contract warning despite the contract being genuinely open. Tracing the cause led to this migration script, and SQL semantic analysis of the non-correlated LIMIT 1 subquery confirmed the defect described above.