Rebuild sort order when display-name format changes#2436
Open
eduralph wants to merge 1 commit into
Open
Conversation
A flat list view caches the (sortkey, handle) map across rebuilds for efficiency — search/filter changes only hide rows, never reorder them. When the display-name format is reconfigured, the sort key itself changes (e.g. surname→given), but the view's rebuild path didn't invalidate the cache, leaving the rows in the previous sort order until the database is reopened. Define rebuild_sort() as a no-op default on BaseModel (the shared base of flat and hierarchical models) so a display-format change can uniformly invalidate the sort cache. Override it on FlatBaseModel to set a _sort_dirty flag; the two cache-reuse guards in _rebuild_search and _rebuild_filter check this flag before reusing the cached map, and clear it after recomputing sort_keys(). Wire a new _format_changed handler in BasePersonView to call rebuild_sort() before build_tree() on both nameformat-changed and placeformat-changed signals. This addresses iteration 1's sign-off rejection: BasePersonView is shared by both the flat People view and the Person Tree view; PersonTreeModel inherits TreeBaseModel, not FlatBaseModel, so iteration 1's direct addition to FlatBaseModel alone would raise AttributeError at runtime on a tree format change. The base-class no-op makes the shared callback safe on any person model. The flat model's override keeps the cache optimization for search/filter (only recomputing when the sort key itself changes). Verified against: - gramps/gui/views/treemodels/basemodel.py:31-56 — cache infrastructure - gramps/gui/views/treemodels/flatbasemodel.py:491-624 — cache reuse guards - gramps/plugins/lib/libpersonview.py:181-198 — signal routing Fixes #9267 Signed-off-by: Eduard Ralph <eduard@ralphovi.net>
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.
Summary
User impact: Change how names are displayed (for example switching from surname-first to given-name-first in Preferences), and the People list keeps showing rows in the old sort order until you close and reopen the database. The names update on screen, but the ordering is stale, so the list looks wrongly sorted.
This makes the People list re-sort itself as soon as the display-name (or place) format changes, without needing a reopen.
Reported in Mantis #9267.
What to look at
The flat People list caches its computed sort order to stay fast; the fix invalidates that cache when the name/place format changes so the list re-sorts immediately. To reproduce: open the People view, then Edit → Preferences → Display and change the Display Name format — pre-fix the row order is unchanged until reopen, post-fix the rows reorder in place.
Root cause
The flat People list caches the
(sortkey, handle)map across rebuilds to avoid recomputing sort keys when the database hasn't changed (e.g. search or filter narrowing). When the user changes the Display Name format in Edit→Preferences→Display, the sort key itself changes (e.g. surname-first → given-first), but the view's rebuild path (nameformat-changed→build_tree) redisplays the rows without invalidating the cache, leaving the list sorted by the previous format until the database is reopened.Fix
Add a
rebuild_sort()method to the model layer:BaseModel (
gramps/gui/views/treemodels/basemodel.py:59-71) — definerebuild_sort()as a no-op default. The hierarchical (tree) model clears and re-adds every row on each rebuild, so it re-sorts from scratch; the flat model caches the sort order and needs to override this to invalidate the cache.FlatBaseModel (
gramps/gui/views/treemodels/flatbasemodel.py:494-497,:548-562) — add a_sort_dirtyflag initializedFalsein__init__. Overriderebuild_sort()to set itTrue. In_rebuild_searchand_rebuild_filter, change the cache-reuse guard fromif not allkeys:toif self._sort_dirty or not allkeys:(lines 612 and 647), and clear the flag after recomputing.BasePersonView (
gramps/plugins/lib/libpersonview.py:191-202) — add a_format_changed()handler that callsself.model.rebuild_sort()(guarded for the pre-buildNonecase) beforebuild_tree(). Wire it to bothnameformat-changedandplaceformat-changedsignals.The same view callback is safe on both flat and tree person models because
rebuild_sort()now exists on the common base; the tree model's no-op does nothing (it re-sorts on every rebuild anyway), and the flat model's override sets the flag. This addresses iteration 1's rejection: iteration 1 addedrebuild_sort()only toFlatBaseModel, so a name-format change on the Person Tree view would raiseAttributeErrorat runtime.Verification
upstream/maintenance/gramps61—gramps/gui/views/treemodels/basemodel.py:59-71—rebuild_sort()defaultgramps/gui/views/treemodels/flatbasemodel.py:491-626— cache initialization and reuse guardsgramps/plugins/lib/libpersonview.py:178-202— signal routing and format-changed handlergramps/gui/views/treemodels/test/flatbasemodel_sort_test.py(new), two classes —FlatBaseModelSortRebuildTestdrives the production_rebuild_search()/_rebuild_filter()over a realFlatNodeMapwith a mutable sort-key function (surname vs. given name; the orders are reverses, so re-sort is unambiguous). Pre-fixrebuild_sort()doesn't exist →AttributeError(RED); post-fix rows re-sort to the new format (GREEN).TreeModelSortRebuildSafetyTestguards the iteration-1 rejection: assertsBaseModelprovidesrebuild_sort()and thatTreeBaseModel.rebuild_sort()is a harmless no-op (returnsNonewithout raising). Pre-fix →AttributeErroron tree format change; post-fix → GREEN.engine/interface/test_bug_9267_name-format-sort.pylaunches the flat People list, captures the display-order Gramps-ID sequence, applies a format change via UI automation, and re-captures — pre-fix IDs unchanged (FAIL), post-fix IDs reordered (PASS).Fixes #9267