Contacts: moving a contact who has a department to another company, or to none, answers 500 #527

New issue

Closed

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

tiagoagueda commented

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

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the contact form against Contact.clean and posting it in a test.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at jobs/models.py:649-655 (Contact.clean) and jobs/forms.py:593 (ContactForm.Meta.fields), jobs/forms.py:639-646 (ContactForm.scope_querysets) and jobs/forms.py:670-687 (ContactForm._save_department); jobs/views.py:724 has not moved.

A contact who is in a department cannot be given another company, or no company, through the contact form: the save crashes with a 500. The model refuses a department that belongs to a different company, keyed on a field the form does not have, and the form only reassigns the department after it has saved.

Where: jobs/models.py:646-652 (Contact.clean); jobs/forms.py:596 (ContactForm.Meta.fields, no department); jobs/forms.py:642-649 (ContactForm.scope_querysets); jobs/forms.py:673-690 (ContactForm._save_department, which runs only after save); reached from ContactUpdateView (jobs/views.py:724).
What happens: a contact at company A is in A's Engineering department. The person edits the contact and picks company B (or empties the company box). ModelForm._post_clean sets the new company on the instance and runs Contact.clean, which still sees A's department and raises ValidationError({"department": ...}). ContactForm has no department field, so Django's add_error raises ValueError: 'ContactForm' has no field named 'department'. and the request answers 500. It happens whatever is typed in the Department box, with Employers with a structure on or off: all eight combinations of {on, off} × {department typed, left blank} × {company B, no company} raise in the snapshot.
Why it matters: a recruiter changing employer is an ordinary edit, and it cannot be recorded. With the feature on, the only way round is two saves (clear the department first, then change the company), which nothing tells the person. With the feature off, the contact keeps its department by design (#138) while the Department box is removed, so the contact can never be moved to another company from the interface at all.
Fix: in ContactForm, before the model is validated (an override of _post_clean, or clean), drop the old department from the instance when the company has changed or been cleared, so _save_department attaches or creates the right one at the new company afterwards. Independently, a model error about a field the form does not render should land on a field it does: raise the mismatch on company (or as a non-field error) rather than on department.

Done when

A test in tests/test_departments.py edits a contact that has a department, posts it to jobs:contact_update with another company and with no company, with the structure feature on and off, and gets a redirect, with the contact at the new company and in the right department (or none).

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the contact form against `Contact.clean` and posting it in a test. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `jobs/models.py:649-655` (`Contact.clean`) and `jobs/forms.py:593` (`ContactForm.Meta.fields`), `jobs/forms.py:639-646` (`ContactForm.scope_querysets`) and `jobs/forms.py:670-687` (`ContactForm._save_department`); `jobs/views.py:724` has not moved. A contact who is in a department cannot be given another company, or no company, through the contact form: the save crashes with a 500. The model refuses a department that belongs to a different company, keyed on a field the form does not have, and the form only reassigns the department after it has saved. - **Where:** `jobs/models.py:646-652` (`Contact.clean`); `jobs/forms.py:596` (`ContactForm.Meta.fields`, no `department`); `jobs/forms.py:642-649` (`ContactForm.scope_querysets`); `jobs/forms.py:673-690` (`ContactForm._save_department`, which runs only after `save`); reached from `ContactUpdateView` (`jobs/views.py:724`). - **What happens:** a contact at company A is in A's *Engineering* department. The person edits the contact and picks company B (or empties the company box). `ModelForm._post_clean` sets the new company on the instance and runs `Contact.clean`, which still sees A's department and raises `ValidationError({"department": ...})`. `ContactForm` has no `department` field, so Django's `add_error` raises `ValueError: 'ContactForm' has no field named 'department'.` and the request answers 500. It happens whatever is typed in the *Department* box, with *Employers with a structure* on or off: all eight combinations of {on, off} × {department typed, left blank} × {company B, no company} raise in the snapshot. - **Why it matters:** a recruiter changing employer is an ordinary edit, and it cannot be recorded. With the feature on, the only way round is two saves (clear the department first, then change the company), which nothing tells the person. With the feature off, the contact keeps its department by design (#138) while the *Department* box is removed, so the contact can never be moved to another company from the interface at all. - **Fix:** in `ContactForm`, before the model is validated (an override of `_post_clean`, or `clean`), drop the old department from the instance when the company has changed or been cleared, so `_save_department` attaches or creates the right one at the new company afterwards. Independently, a model error about a field the form does not render should land on a field it does: raise the mismatch on `company` (or as a non-field error) rather than on `department`. ## Done when - A test in `tests/test_departments.py` edits a contact that has a department, posts it to `jobs:contact_update` with another company and with no company, with the structure feature on and off, and gets a redirect, with the contact at the new company and in the right department (or none).

tiagoagueda added this to the 0.5.0 milestone

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

tiagoagueda added the

tier

bug

labels

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

tiagoagueda referenced this issue from a commit

2026-10-03 18:09:38 +00:00

Let a contact change company without crashing on its old department

tiagoagueda closed this issue

2026-10-03 18:09:38 +00:00