Your details: adding a telephone number or a postal address crashes, because the new row is given no owner #454

New issue

Closed

opened 2026-10-01 10:46:49 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:46:49 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, by reading how Your details saves its rows and then posting a new row to accounts:profile in a test.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at core/phone_numbers.py:302-383 (BasePhoneNumberFormSet); accounts/views.py:182-187, core/postal.py and core/mixins.py have not moved, and the same two posts raise the same two errors.

Your details cannot add a telephone number or a postal address. ProfileView.form_valid binds the two formsets to the profile and saves them without giving the new rows an owner. PhoneNumber and PostalAddress are OwnedModels, and owner is not nullable. Nothing else fills it in: not the forms (fields are kind, label and number, or the address parts), not the formsets, not the models' save(). The insert fails, the page answers 500, and the transaction takes the rest of the page's changes with it.

Where: accounts/views.py:182-187 (ProfileView.form_valid: numbers.instance = self.object; numbers.save() and addresses.instance = self.object; addresses.save()); core/phone_numbers.py:325-377 (BasePhoneNumberFormSet) and core/postal.py:245-285 (BasePostalAddressFormSet), neither of which sets owner in save_new. Compare core/mixins.py:107-114 (WebLinksMixin.save_web_links) and core/mixins.py:165-176 (PhoneNumbersMixin.save_phone_numbers), which both set form.instance.owner before saving; the profile's web links go through the first, but its numbers and addresses do not go through the second.
What happens: a signed-in person types a number into the empty telephone row (shown while Several telephone numbers is on, the default), or fills in the empty postal address row (always shown; addresses have no feature switch), and saves. The request raises IntegrityError: NOT NULL constraint failed: core_phonenumber.owner_id (or core_postaladdress.owner_id) and returns 500. Reproduced by posting phone_numbers-0-number_0=PT, phone_numbers-0-number_1=912345678 and, separately, one address row to accounts:profile: both raise, and no row is written. Editing rows that already exist works, because they carry their owner from when they were created.
Why it matters: nobody can record their own address, and nobody can add a number while the feature is on. The name, headline and links typed on the same page are lost with it.
Fix: give new rows their owner inside the formsets, so no caller can forget: override save_new in BasePhoneNumberFormSet and BasePostalAddressFormSet (as BaseWebLinkFormSet.save_new already does for kind) to set form.instance.owner from the holder, with the holder.owner if hasattr(holder, "owner") else holder.user rule that core/mixins.py repeats twice moved into one helper beside them. PhoneNumbersMixin.save_phone_numbers and WebLinksMixin.save_web_links can then drop their own loops.

Done when

A test in tests/test_phone_numbers.py posts a new number to accounts:profile and finds it saved with the account as owner; it fails today with the IntegrityError.
A test in tests/test_postal_addresses.py does the same for a new address.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, by reading how *Your details* saves its rows and then posting a new row to `accounts:profile` in a test. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `core/phone_numbers.py:302-383` (`BasePhoneNumberFormSet`); `accounts/views.py:182-187`, `core/postal.py` and `core/mixins.py` have not moved, and the same two posts raise the same two errors. *Your details* cannot add a telephone number or a postal address. `ProfileView.form_valid` binds the two formsets to the profile and saves them without giving the new rows an owner. `PhoneNumber` and `PostalAddress` are `OwnedModel`s, and `owner` is not nullable. Nothing else fills it in: not the forms (`fields` are `kind`, `label` and `number`, or the address parts), not the formsets, not the models' `save()`. The insert fails, the page answers 500, and the transaction takes the rest of the page's changes with it. - **Where:** `accounts/views.py:182-187` (`ProfileView.form_valid`: `numbers.instance = self.object; numbers.save()` and `addresses.instance = self.object; addresses.save()`); `core/phone_numbers.py:325-377` (`BasePhoneNumberFormSet`) and `core/postal.py:245-285` (`BasePostalAddressFormSet`), neither of which sets `owner` in `save_new`. Compare `core/mixins.py:107-114` (`WebLinksMixin.save_web_links`) and `core/mixins.py:165-176` (`PhoneNumbersMixin.save_phone_numbers`), which both set `form.instance.owner` before saving; the profile's web links go through the first, but its numbers and addresses do not go through the second. - **What happens:** a signed-in person types a number into the empty telephone row (shown while *Several telephone numbers* is on, the default), or fills in the empty postal address row (always shown; addresses have no feature switch), and saves. The request raises `IntegrityError: NOT NULL constraint failed: core_phonenumber.owner_id` (or `core_postaladdress.owner_id`) and returns 500. Reproduced by posting `phone_numbers-0-number_0=PT`, `phone_numbers-0-number_1=912345678` and, separately, one address row to `accounts:profile`: both raise, and no row is written. Editing rows that already exist works, because they carry their owner from when they were created. - **Why it matters:** nobody can record their own address, and nobody can add a number while the feature is on. The name, headline and links typed on the same page are lost with it. - **Fix:** give new rows their owner inside the formsets, so no caller can forget: override `save_new` in `BasePhoneNumberFormSet` and `BasePostalAddressFormSet` (as `BaseWebLinkFormSet.save_new` already does for `kind`) to set `form.instance.owner` from the holder, with the `holder.owner if hasattr(holder, "owner") else holder.user` rule that `core/mixins.py` repeats twice moved into one helper beside them. `PhoneNumbersMixin.save_phone_numbers` and `WebLinksMixin.save_web_links` can then drop their own loops. ## Done when - A test in `tests/test_phone_numbers.py` posts a new number to `accounts:profile` and finds it saved with the account as owner; it fails today with the `IntegrityError`. - A test in `tests/test_postal_addresses.py` does the same for a new address.

tiagoagueda added this to the 0.5.0 milestone

2026-10-01 10:46:49 +00:00

tiagoagueda added the

bug

tier

labels

2026-10-01 10:46:49 +00:00

tiagoagueda referenced this issue from a commit

2026-10-01 16:03:11 +00:00

Give a new number, link or address its owner where the row is made

tiagoagueda closed this issue

2026-10-01 16:03:11 +00:00