Contacts: a department typed in different case makes a second team, and the test named for reuse asserts the duplicate #528

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

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the contact form and its tests and trying the form against a test database.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at jobs/forms.py:579-582 (the comment on ContactForm.new_department) and jobs/forms.py:682-684 (the get_or_create in ContactForm._save_department); tests/test_departments.py:129-148 and core/importer.py:756-759 have not moved.

The contact form's department box promises to match a typed name against the company's existing departments "which is how Industry.named already works". Industry.named matches by slug, ignoring case and accents. _save_department matches the exact string, so "engineering" typed beside an existing "Engineering" makes a second department at the same company. The test that should guard the reuse is named for it and asserts the opposite.

  • Where: jobs/forms.py:582-585 (the comment on ContactForm.new_department), jobs/forms.py:685-687 (ContactForm._save_department: Department.objects.get_or_create(owner=self.user, company_id=contact.company_id, name=name)), tests/test_departments.py:129-148 (test_a_name_the_company_already_has_is_reused ends with assert Department.objects.for_user(user).count() == 2, "case differs, so it is a new one")
  • What happens: a company has the department "Engineering". The person adds a contact there and types "engineering". The company now lists both teams, contacts are split between them, and the department datalist offers both. Reproduced against the snapshot: the departments are ['Engineering', 'engineering']. The importer already matches departments case-insensitively when it attaches one to an application (core/importer.py:756-759), so the two paths disagree about whether these are one team.
  • Why it matters: the duplicate is made silently, and no test anywhere checks that an identical name is reused, so the promise in the comment is not held by the code or by the tests.
  • Fix: in _save_department, look the name up with name__iexact among the company's departments first and create only when none matches, keeping the spelling first entered (as Industry.named does). Rename or rewrite test_a_name_the_company_already_has_is_reused so it asserts one department after typing a different case, and add a test that an identical name reuses the department.

Done when

  • test_a_name_the_company_already_has_is_reused in tests/test_departments.py asserts Department.objects.for_user(user).count() == 1 and contact.department.name == "Engineering" after typing "engineering"; it fails today.
Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the contact form and its tests and trying the form against a test database. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `jobs/forms.py:579-582` (the comment on `ContactForm.new_department`) and `jobs/forms.py:682-684` (the `get_or_create` in `ContactForm._save_department`); `tests/test_departments.py:129-148` and `core/importer.py:756-759` have not moved. The contact form's department box promises to match a typed name against the company's existing departments "which is how `Industry.named` already works". `Industry.named` matches by slug, ignoring case and accents. `_save_department` matches the exact string, so "engineering" typed beside an existing "Engineering" makes a second department at the same company. The test that should guard the reuse is named for it and asserts the opposite. - **Where:** `jobs/forms.py:582-585` (the comment on `ContactForm.new_department`), `jobs/forms.py:685-687` (`ContactForm._save_department`: `Department.objects.get_or_create(owner=self.user, company_id=contact.company_id, name=name)`), `tests/test_departments.py:129-148` (`test_a_name_the_company_already_has_is_reused` ends with `assert Department.objects.for_user(user).count() == 2, "case differs, so it is a new one"`) - **What happens:** a company has the department "Engineering". The person adds a contact there and types "engineering". The company now lists both teams, contacts are split between them, and the department datalist offers both. Reproduced against the snapshot: the departments are `['Engineering', 'engineering']`. The importer already matches departments case-insensitively when it attaches one to an application (`core/importer.py:756-759`), so the two paths disagree about whether these are one team. - **Why it matters:** the duplicate is made silently, and no test anywhere checks that an identical name is reused, so the promise in the comment is not held by the code or by the tests. - **Fix:** in `_save_department`, look the name up with `name__iexact` among the company's departments first and create only when none matches, keeping the spelling first entered (as `Industry.named` does). Rename or rewrite `test_a_name_the_company_already_has_is_reused` so it asserts one department after typing a different case, and add a test that an identical name reuses the department. ## Done when - `test_a_name_the_company_already_has_is_reused` in `tests/test_departments.py` asserts `Department.objects.for_user(user).count() == 1` and `contact.department.name == "Engineering"` after typing "engineering"; it fails today.
tiagoagueda added this to the 0.6.0 milestone 2026-10-01 10:47:24 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
Postulo/postulo#528
No description provided.