seed_demo: a refused run still marks the account's address verified and primary, never clearing the old primary #496

New issue

Closed

opened 2026-10-01 10:47:06 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:47:06 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading seed_demo's handle.

Checked again on 2026-10-01 against main at 0a5cad587: still present.

seed_demo refuses to add to an account that already holds a search, but only after it has already written to that account's addresses. It sets the address matching user.email to verified and primary before the check, outside the transaction that guards the seeding. It also never clears any other primary address. A run that reports an error has therefore still changed the account.

Where: Command.handle in core/management/commands/seed_demo.py:293-318. The EmailAddress.objects.update_or_create(..., defaults={"email": user.email, "verified": True, "primary": True}) is at :304-308. The has_data check and raise CommandError(...) come after it, at :310-318, and transaction.atomic() starts at :320.
What happens: an operator runs seed_demo with the address of an existing account that holds a search, perhaps their own, as Getting started and the README suggest with seed_demo you@example.org. The command answers "That account already holds a job search" and exits with an error. That account's EmailAddress row for user.email is now verified=True, primary=True anyway. For an account whose people have signed in, this is normally the row that was already primary and verified, so nothing visible changes. It does change something when user.email no longer matches the primary EmailAddress, which happens when an account's address is edited through the Django admin (accounts/admin.py:21, mounted when POSTULO_ADMIN_URL is set): a second row is created, or updated, as primary, and the account then has two primary addresses.
Why it matters: a command that refuses should leave the database as it found it. With two primary rows, allauth and Postulo's own code (plugins/email/__init__.py:45 orders by -primary) disagree about which address is the account's. That affects where mail is sent and which address shows as the account's own. The operator vouching for an address at the console is deliberate, as in create_superuser (accounts/models.py:85-95), and is not the defect here.
Fix: move the has_data check and its CommandError above every write. Vouch for the address inside the transaction.atomic() block, after the check, and do it with allauth's EmailAddress.set_as_primary() (or clear primary on the user's other rows first), so the account keeps exactly one primary address. create_superuser has the same update_or_create without clearing other primaries, which is harmless there because the account is new. A shared helper in accounts would keep the two from drifting apart.

Done when

tests/test_seed_demo.py: running seed_demo against an account that holds a search, and whose only EmailAddress is unverified, raises CommandError and leaves that row unverified. Running it on an account whose user.email differs from its primary EmailAddress leaves exactly one primary row. Both fail today.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading `seed_demo`'s `handle`. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. `seed_demo` refuses to add to an account that already holds a search, but only after it has already written to that account's addresses. It sets the address matching `user.email` to verified and primary before the check, outside the transaction that guards the seeding. It also never clears any other primary address. A run that reports an error has therefore still changed the account. - **Where:** `Command.handle` in `core/management/commands/seed_demo.py:293-318`. The `EmailAddress.objects.update_or_create(..., defaults={"email": user.email, "verified": True, "primary": True})` is at `:304-308`. The `has_data` check and `raise CommandError(...)` come after it, at `:310-318`, and `transaction.atomic()` starts at `:320`. - **What happens:** an operator runs `seed_demo` with the address of an existing account that holds a search, perhaps their own, as *Getting started* and the README suggest with `seed_demo you@example.org`. The command answers "That account already holds a job search" and exits with an error. That account's `EmailAddress` row for `user.email` is now `verified=True, primary=True` anyway. For an account whose people have signed in, this is normally the row that was already primary and verified, so nothing visible changes. It does change something when `user.email` no longer matches the primary `EmailAddress`, which happens when an account's address is edited through the Django admin (`accounts/admin.py:21`, mounted when `POSTULO_ADMIN_URL` is set): a second row is created, or updated, as primary, and the account then has two primary addresses. - **Why it matters:** a command that refuses should leave the database as it found it. With two primary rows, `allauth` and Postulo's own code (`plugins/email/__init__.py:45` orders by `-primary`) disagree about which address is the account's. That affects where mail is sent and which address shows as the account's own. The operator vouching for an address at the console is deliberate, as in `create_superuser` (`accounts/models.py:85-95`), and is not the defect here. - **Fix:** move the `has_data` check and its `CommandError` above every write. Vouch for the address inside the `transaction.atomic()` block, after the check, and do it with allauth's `EmailAddress.set_as_primary()` (or clear `primary` on the user's other rows first), so the account keeps exactly one primary address. `create_superuser` has the same `update_or_create` without clearing other primaries, which is harmless there because the account is new. A shared helper in `accounts` would keep the two from drifting apart. ## Done when - `tests/test_seed_demo.py`: running `seed_demo` against an account that holds a search, and whose only `EmailAddress` is unverified, raises `CommandError` and leaves that row unverified. Running it on an account whose `user.email` differs from its primary `EmailAddress` leaves exactly one primary row. Both fail today.

tiagoagueda added this to the 0.6.0 milestone

2026-10-01 10:47:06 +00:00

tiagoagueda added the

tier

bug

labels

2026-10-01 10:47:06 +00:00

tiagoagueda referenced this issue from a commit

2026-10-04 04:15:10 +00:00

Make a refused seed_demo run leave the account untouched

tiagoagueda closed this issue

2026-10-04 04:15:10 +00:00