Skip to content

Scope location admin levels by hierarchy for multi-country installs - #386

Open
level09 wants to merge 18 commits into
mainfrom
feat/location-hierarchies
Open

Scope location admin levels by hierarchy for multi-country installs#386
level09 wants to merge 18 commits into
mainfrom
feat/location-hierarchies

Conversation

@level09

@level09 level09 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Administrative levels used to be one global list, named for Syria. This adds named hierarchies so a
partner geography can define its own ladder (Territory > Governorate > Locality) while the existing
one stays the default. Installations that never create a hierarchy behave exactly as before.

Nothing is stored on location: a location derives its hierarchy from admin_level.hierarchy_id,
so no existing row changes.

Two production bugs found while validating this

These are pre-existing and independent of the feature, but worth reviewing first because they are
the parts with real user impact.

4,938 of prod2's 4,942 Iraqi locations have no id_tree. That column powers "include
descendants" in the bulletin, actor and incident location filters, and NULL never matches, so
ancestor search under an Iraqi governorate returns almost nothing. Measured on a copy of prod2 data:
Al-Anbar returns 1 location where it should return 496. Nobody could repair it because Regenerate
only ever rebuilt full_location, and rebuild_id_trees() had no caller anywhere in the codebase.

Edits never cascaded to descendants. Renaming a location left every location under it holding
the old text, and moving one left its descendants pointing at the old ancestor chain, which drops
them out of ancestor search entirely. Location.rebuild_subtree() now rebuilds the subtree rather
than the single edited row, and Regenerate rebuilds both columns, so one click repairs the above.

flask doctor gained a check that counts rows whose stored path disagrees with the current titles,
so this stops being invisible.

Backward compatibility

Verified against copies of real data rather than argued:

  • 11,521 staging rows and 11,554 prod2 rows: id, parent_id, admin_level_id, country_id,
    full_location and id_tree checksums byte-identical before and after the migration
  • 27 of 27 functional probes identical to main, covering every location filter, all four
    serializers, descendant expansion and the dropdown payloads. Only difference anywhere is two
    additive keys on the admin level payload
  • Regenerating all 11,521 strings produces the same checksum on old and new code
  • A second hierarchy with deliberately colliding codes leaves every legacy row untouched
  • Migration is DDL only, 1.9 seconds on prod2-sized data, and a fresh install converges on the same
    schema as an upgraded one

Also fixed

  • Advanced search compared an incoming level code against admin_level_id; it now matches on id
  • The location editor assumed a parent sits at the child's code minus one, which breaks as soon as a
    level is deleted or reordered; the eligible parent level is now resolved server side
  • Admin levels seeded with a null display_order, which made full_location component order
    arbitrary until someone dragged an order in the interface
  • Nothing prevented a location becoming a child of its own descendant

Reviewing this

Commits are separated by concern:

  • daa2f7f07 the feature: model, migration, scoped CRUD, reorder, search
  • 8e21fec08 the cascade bug fix and the stale-path check
  • 72bb3ebea the cycle guard
  • 3b301ce2d the ladder shown under the admin level in the locations table

Frontend is five files: component-data.html, locations.html, location_dialog.html,
EditableTable.js, LocationSearchBox.js. Remaining frontend work is written up in
location-hierarchies-frontend-spec.md, of which BYNT-A matters most: the Regenerate panel does not
say that it rewrites every location in the installation, and on prod2 that is 5,924 rows.

Deliberately not included: generating the partner location data, which is a one-off script, and any
mapping between country and hierarchy.

@level09
level09 requested a review from apodacaduron as a code owner July 27, 2026 22:04
@level09 level09 self-assigned this Jul 27, 2026
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 863717b1-e51c-4857-9a21-54e05ce96f0b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/location-hierarchies

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

session.commit()
id = h.id

assert admin_client.delete(f"/admin/api/location-hierarchy/{id}").status_code == 200
Comment thread enferno/admin/models/LocationAdminLevel.py Fixed
apodacaduron
apodacaduron previously approved these changes Jul 29, 2026
Combine hierarchy scoping with the title_tr search from #370 and re-parent the
hierarchy migration onto b6e3d1a8f254.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants