Company tree: the ten-deep cap only measures upwards, so a chain built bottom-up grows past it and its group loses the top #532

New issue

Closed

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

tiagoagueda commented

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

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading Company.clean and Company.group and building a chain against a test database.

Related to #55.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at jobs/models.py:367-403 (Company.clean), jobs/models.py:405-418 (Company.group) and jobs/forms.py:221-230 (the parent choices in CompanyForm.scope_querysets).

Company.clean refuses a parent when the chain above the new parent is more than MAX_DEPTH (10) companies deep, but it ignores whatever already hangs under the company being edited. Giving the current top of a chain a new parent is always checked as a chain of two, so a chain built from the bottom up, or two chains joined, can grow without limit. Company.group then stops after ten steps and reports a company in the middle of the chain as the top.

Where: jobs/models.py:364-400 (Company.clean: seen, node, depth = set(), self.parent, 1 counts only upwards), jobs/models.py:402-415 (Company.group, bounded by MAX_DEPTH), reached through CompanyForm (jobs/forms.py:220-229 offers any company outside the edited one's subtree as parent, and the model form runs Company.clean)
What happens: a person records a group subsidiary first: C0, then C1 with C0 set under it, then C2 with C1 set under it, and so on. Every save passes full_clean(). With fifteen companies (C0 at the bottom, C14 at the top), C0.group is C10, not C14. The same happens when an existing six-deep chain is attached under another six-deep chain.
Why it matters: group_members(), the companies list's ?group= filter, the "across the group" figures and the rule deciding which departments an application may name all work on a partial group, silently. The cap's own comment says it exists so that a mistake cannot make a very deep chain that every page walks; it does not hold.
Fix: in Company.clean, when self.pk is set, add the height of the company's own subtree (the longest path through descendants, with the same visited set) to the depth above the new parent, and refuse when the sum exceeds MAX_DEPTH.

Done when

A test in tests/test_company_tree.py builds a chain of MAX_DEPTH companies, then gives its top a new parent and asserts that full_clean() raises the "more than 10 companies deep" error. It fails today.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading `Company.clean` and `Company.group` and building a chain against a test database. Related to #55. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `jobs/models.py:367-403` (`Company.clean`), `jobs/models.py:405-418` (`Company.group`) and `jobs/forms.py:221-230` (the parent choices in `CompanyForm.scope_querysets`). `Company.clean` refuses a parent when the chain above the new parent is more than `MAX_DEPTH` (10) companies deep, but it ignores whatever already hangs under the company being edited. Giving the current top of a chain a new parent is always checked as a chain of two, so a chain built from the bottom up, or two chains joined, can grow without limit. `Company.group` then stops after ten steps and reports a company in the middle of the chain as the top. - **Where:** `jobs/models.py:364-400` (`Company.clean`: `seen, node, depth = set(), self.parent, 1` counts only upwards), `jobs/models.py:402-415` (`Company.group`, bounded by `MAX_DEPTH`), reached through `CompanyForm` (`jobs/forms.py:220-229` offers any company outside the edited one's subtree as parent, and the model form runs `Company.clean`) - **What happens:** a person records a group subsidiary first: C0, then C1 with C0 set under it, then C2 with C1 set under it, and so on. Every save passes `full_clean()`. With fifteen companies (C0 at the bottom, C14 at the top), `C0.group` is C10, not C14. The same happens when an existing six-deep chain is attached under another six-deep chain. - **Why it matters:** `group_members()`, the companies list's `?group=` filter, the "across the group" figures and the rule deciding which departments an application may name all work on a partial group, silently. The cap's own comment says it exists so that a mistake cannot make a very deep chain that every page walks; it does not hold. - **Fix:** in `Company.clean`, when `self.pk` is set, add the height of the company's own subtree (the longest path through `descendants`, with the same visited set) to the depth above the new parent, and refuse when the sum exceeds `MAX_DEPTH`. ## Done when - A test in `tests/test_company_tree.py` builds a chain of `MAX_DEPTH` companies, then gives its top a new parent and asserts that `full_clean()` raises the "more than 10 companies deep" error. It fails today.

tiagoagueda added this to the 0.6.0 milestone

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

tiagoagueda added the

tier

bug

labels

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

tiagoagueda referenced this issue from a commit

2026-10-04 05:28:45 +00:00

Count what hangs under a company when its parent is changed

tiagoagueda closed this issue

2026-10-04 05:28:45 +00:00