Scope location admin levels by hierarchy for multi-country installs - #386
Open
level09 wants to merge 18 commits into
Open
Scope location admin levels by hierarchy for multi-country installs#386level09 wants to merge 18 commits into
level09 wants to merge 18 commits into
Conversation
…ion field and single-use helper
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
| session.commit() | ||
| id = h.id | ||
|
|
||
| assert admin_client.delete(f"/admin/api/location-hierarchy/{id}").status_code == 200 |
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 fromadmin_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 "includedescendants" 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, andrebuild_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 ratherthan the single edited row, and Regenerate rebuilds both columns, so one click repairs the above.
flask doctorgained 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:
id,parent_id,admin_level_id,country_id,full_locationandid_treechecksums byte-identical before and after the migrationmain, covering every location filter, all fourserializers, descendant expansion and the dropdown payloads. Only difference anywhere is two
additive keys on the admin level payload
schema as an upgraded one
Also fixed
codeagainstadmin_level_id; it now matches on idlevel is deleted or reordered; the eligible parent level is now resolved server side
display_order, which madefull_locationcomponent orderarbitrary until someone dragged an order in the interface
Reviewing this
Commits are separated by concern:
daa2f7f07the feature: model, migration, scoped CRUD, reorder, search8e21fec08the cascade bug fix and the stale-path check72bb3ebeathe cycle guard3b301ce2dthe ladder shown under the admin level in the locations tableFrontend is five files:
component-data.html,locations.html,location_dialog.html,EditableTable.js,LocationSearchBox.js. Remaining frontend work is written up inlocation-hierarchies-frontend-spec.md, of which BYNT-A matters most: the Regenerate panel does notsay 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.