Companies: names that differ only in accented or non-Latin case become duplicates; renaming through the API skips every check #546

New issue

Closed

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

tiagoagueda commented

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

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading every place a company is matched by name and running the matching against the snapshot on SQLite.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at jobs/models.py:312-314 (the constraint), jobs/forms.py:408-422 (CompanyForm.clean_name) and jobs/forms.py:670-683 (ContactForm._save_department); the service, the importer and the API router have not moved.

The rule that Acme, acme and ACME are one employer lives only in Python-side name__iexact lookups; the database constraint unique_company_name_per_owner is case-sensitive. On SQLite, the default database, Django turns iexact into LIKE, which folds ASCII letters only, so an accented or non-Latin name typed in another case becomes a second company. The API's rename path has no check at all: it makes case duplicates, saves an empty name, and answers 500 on an exact clash.

Where:
- jobs/models.py:309-311: UniqueConstraint(fields=("owner", "name")), case-sensitive.
- applications/services.py:324, get_or_create_company: filter(name__iexact=name).first(), then create. Used by the intake form, the capture review, the listing form, the CSV import and the API's company, application and listing creation.
- jobs/forms.py:407-421, CompanyForm.clean_name: the same name__iexact check, whose docstring says the constraint "would happily accept "acme" alongside "Acme"".
- core/importer.py:575 and :756-760: the archive importer matches companies and departments with name__iexact.
- api/routers/companies.py:86-102, patch_company: setattr(company, field, value) for every field sent, then company.save(), with no name check.
- jobs/forms.py:673-686, ContactForm._save_department: departments matched exactly (get_or_create(name=...)), while the importer matches them case-insensitively.
What happens (run against the snapshot on SQLite):
- With Émile Frères recorded, get_or_create_company(user, "émile frères") creates a second company; so do Οτε and ΟΤΕ. CompanyForm accepts ÉMILE FRÈRES as new. Acme/ACME is caught, as intended.
- Beside Acme, PATCH /api/v1/companies/{id} with {"name": "ACME"} answers 200 and leaves ACME and Acme; {"name": ""} answers 200 and saves a company with no name; {"name": "Acme"} answers 500 with IntegrityError: UNIQUE constraint failed: jobs_company.owner_id, jobs_company.name.
- On PostgreSQL iexact uses UPPER(), which folds Unicode, but two concurrent creations of the same name both pass the check and the second fails on the constraint with a 500.
Why it matters: duplicate employers split postings, contacts and applications between two records, and the funnel and Insights count them apart; the person has to find and merge them by hand. The API is the path the browser extensions and the MCP server use, so the unchecked rename is reachable by any token with the write scope. jobs/duplicates.py will later flag some of these as possible duplicates, but only after the fact.
Fix:
1. Store a comparison key on Company, casefold() with whitespace collapsed, written in save() as PostalAddress.comparable is, and replace the constraint with UniqueConstraint(owner, key) (a data migration fills the key; existing clashes need a report rather than a silent merge).
2. Match on that key in get_or_create_company, CompanyForm.clean_name, the importer and the CSV import, instead of name__iexact. (jobs/duplicates.py:fold also strips accents and punctuation, which is right for a hint and too loose for a constraint.)
3. In get_or_create_company, catch IntegrityError inside a savepoint and read the existing row again.
4. Give patch_company the form's rules: refuse an empty name with 422 and a name whose key another company holds with 409, as problem documents.
5. Give departments the same key, so the contact form and the importer agree.

Done when

tests/test_duplicates.py (or a new tests/test_company_names.py): get_or_create_company returns the same company for Émile Frères and émile frères on SQLite, and CompanyForm refuses ÉMILE FRÈRES beside Émile Frères.
tests/test_api.py: PATCH /api/v1/companies/{id} with a case variant of another company's name answers 409, with an empty name 422, and never 500.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading every place a company is matched by name and running the matching against the snapshot on SQLite. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `jobs/models.py:312-314` (the constraint), `jobs/forms.py:408-422` (`CompanyForm.clean_name`) and `jobs/forms.py:670-683` (`ContactForm._save_department`); the service, the importer and the API router have not moved. The rule that *Acme*, *acme* and *ACME* are one employer lives only in Python-side `name__iexact` lookups; the database constraint `unique_company_name_per_owner` is case-sensitive. On SQLite, the default database, Django turns `iexact` into `LIKE`, which folds ASCII letters only, so an accented or non-Latin name typed in another case becomes a second company. The API's rename path has no check at all: it makes case duplicates, saves an empty name, and answers 500 on an exact clash. - **Where:** - `jobs/models.py:309-311`: `UniqueConstraint(fields=("owner", "name"))`, case-sensitive. - `applications/services.py:324`, `get_or_create_company`: `filter(name__iexact=name).first()`, then `create`. Used by the intake form, the capture review, the listing form, the CSV import and the API's company, application and listing creation. - `jobs/forms.py:407-421`, `CompanyForm.clean_name`: the same `name__iexact` check, whose docstring says the constraint "would happily accept \"acme\" alongside \"Acme\"". - `core/importer.py:575` and `:756-760`: the archive importer matches companies and departments with `name__iexact`. - `api/routers/companies.py:86-102`, `patch_company`: `setattr(company, field, value)` for every field sent, then `company.save()`, with no name check. - `jobs/forms.py:673-686`, `ContactForm._save_department`: departments matched exactly (`get_or_create(name=...)`), while the importer matches them case-insensitively. - **What happens** (run against the snapshot on SQLite): - With *Émile Frères* recorded, `get_or_create_company(user, "émile frères")` creates a second company; so do *Οτε* and *ΟΤΕ*. `CompanyForm` accepts *ÉMILE FRÈRES* as new. *Acme*/*ACME* is caught, as intended. - Beside *Acme*, `PATCH /api/v1/companies/{id}` with `{"name": "ACME"}` answers 200 and leaves *ACME* and *Acme*; `{"name": ""}` answers 200 and saves a company with no name; `{"name": "Acme"}` answers 500 with `IntegrityError: UNIQUE constraint failed: jobs_company.owner_id, jobs_company.name`. - On PostgreSQL `iexact` uses `UPPER()`, which folds Unicode, but two concurrent creations of the same name both pass the check and the second fails on the constraint with a 500. - **Why it matters:** duplicate employers split postings, contacts and applications between two records, and the funnel and Insights count them apart; the person has to find and merge them by hand. The API is the path the browser extensions and the MCP server use, so the unchecked rename is reachable by any token with the `write` scope. `jobs/duplicates.py` will later flag some of these as possible duplicates, but only after the fact. - **Fix:** 1. Store a comparison key on `Company`, `casefold()` with whitespace collapsed, written in `save()` as `PostalAddress.comparable` is, and replace the constraint with `UniqueConstraint(owner, key)` (a data migration fills the key; existing clashes need a report rather than a silent merge). 2. Match on that key in `get_or_create_company`, `CompanyForm.clean_name`, the importer and the CSV import, instead of `name__iexact`. (`jobs/duplicates.py:fold` also strips accents and punctuation, which is right for a hint and too loose for a constraint.) 3. In `get_or_create_company`, catch `IntegrityError` inside a savepoint and read the existing row again. 4. Give `patch_company` the form's rules: refuse an empty name with 422 and a name whose key another company holds with 409, as problem documents. 5. Give departments the same key, so the contact form and the importer agree. ## Done when - `tests/test_duplicates.py` (or a new `tests/test_company_names.py`): `get_or_create_company` returns the same company for *Émile Frères* and *émile frères* on SQLite, and `CompanyForm` refuses *ÉMILE FRÈRES* beside *Émile Frères*. - `tests/test_api.py`: `PATCH /api/v1/companies/{id}` with a case variant of another company's name answers 409, with an empty name 422, and never 500.

tiagoagueda added this to the 0.5.0 milestone

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

tiagoagueda added the

bug

label

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

tiagoagueda referenced this issue from a commit

2026-10-04 10:04:58 +00:00

Give the API's company rename the form's rules and survive a race

tiagoagueda referenced this issue from a commit

2026-10-04 10:04:58 +00:00

Bring the changelog and two older tests in line with the bunch