Skip to content

Commit 01d33e8

Browse files
committed
Make surname consumers consistently honor name-group mapping
When surnames are grouped together via "Group As → Group All" (the database-wide name-group mapping), every surname consumer must treat them as one group. Historically, TopSurnamesGramplet and FilterByName used Name.get_group_name() which ignores the global mapping; SameSurnames quick view's filter rule matched on raw get_surname(). This fix routes every consumer through the canonical NameDisplay.name_grouping_name(db, name), which honors both the per-name override and the database-wide mapping, so all consumers now agree on the grouping. Fixes #6825 Signed-off-by: Eduard Ralph <eduard@ralphovi.net>
1 parent b679c08 commit 01d33e8

4 files changed

Lines changed: 175 additions & 21 deletions

File tree

gramps/plugins/gramplet/test/topsurnamesgramplet_test.py

Lines changed: 117 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
# Gramps modules
3535
#
3636
# -------------------------------------------------------------------------
37+
from gramps.gen.db import DbTxn
38+
from gramps.gen.db.utils import make_database
3739
from gramps.gen.lib import Name, Person, Surname
3840
from gramps.gen.types import PersonHandle
3941
from gramps.plugins.gramplet.topsurnamesgramplet import record_surnames
@@ -44,6 +46,20 @@
4446
# Test helpers
4547
#
4648
# -------------------------------------------------------------------------
49+
class FakeDb:
50+
"""
51+
Minimal database stand-in exposing only the name-group mapping that
52+
``record_surnames`` consults. An empty mapping leaves every surname
53+
grouped under itself (the no-grouping case the original tests exercise).
54+
"""
55+
56+
def __init__(self, mapping: dict[str, str] | None = None) -> None:
57+
self._mapping = mapping or {}
58+
59+
def get_name_group_mapping(self, surname: str) -> str:
60+
return self._mapping.get(surname, surname)
61+
62+
4763
def make_name(surname_text: str) -> Name:
4864
"""
4965
Build a Name carrying a single surname.
@@ -67,14 +83,17 @@ def make_person(handle: str, primary: str, alternates: tuple[str, ...] = ()) ->
6783
return person
6884

6985

70-
def tally(people: list[Person]) -> tuple[dict[str, int], dict[str, PersonHandle]]:
86+
def tally(
87+
people: list[Person], db: object | None = None
88+
) -> tuple[dict[str, int], dict[str, PersonHandle]]:
7189
"""
7290
Run record_surnames over a list of people and return the tallies.
7391
"""
92+
db = db if db is not None else FakeDb()
7493
surnames: dict[str, int] = defaultdict(int)
7594
representative_handle: dict[str, PersonHandle] = {}
7695
for person in people:
77-
record_surnames(person, surnames, representative_handle)
96+
record_surnames(db, person, surnames, representative_handle)
7897
return surnames, representative_handle
7998

8099

@@ -152,5 +171,101 @@ def test_multiple_primary_carriers_keep_a_matching_representative(self):
152171
self.assertEqual(surnames["Webb"], 3)
153172

154173

174+
# -------------------------------------------------------------------------
175+
#
176+
# SurnameGroupingConsistencyTest (bug #6825)
177+
#
178+
# -------------------------------------------------------------------------
179+
class SurnameGroupingConsistencyTest(unittest.TestCase):
180+
"""
181+
Bug #6825: surname consumers must agree on the grouping.
182+
183+
When two surnames are grouped together via the database-wide name-group
184+
mapping ("Group As -> Group All"), every surname consumer must treat
185+
them as one group. Historically the Top Surnames gramplet and the Same
186+
Surnames quick view disagreed: both grouped by ``Name.get_group_name()``
187+
(which ignores the database-wide mapping) and the quick view's filter
188+
rule matched on the raw surname.
189+
"""
190+
191+
@classmethod
192+
def setUpClass(cls):
193+
# A real in-memory database is needed: the Same Surnames filter rule
194+
# applies against the live db, and the global name-group mapping
195+
# lives in the db, not on the Name objects.
196+
cls.db = make_database("sqlite")
197+
cls.db.load(":memory:")
198+
cls.smith = make_person("H_SMITH", "Smith")
199+
cls.jones = make_person("H_JONES", "Jones")
200+
with DbTxn("setup", cls.db) as txn:
201+
cls.db.add_person(cls.smith, txn)
202+
cls.db.add_person(cls.jones, txn)
203+
# "Group As Smith -> Group All": a global mapping, no per-name
204+
# override on the Jones name.
205+
cls.db.set_name_group_mapping("Jones", "Smith")
206+
207+
@classmethod
208+
def tearDownClass(cls):
209+
cls.db.close()
210+
211+
def test_samesurnames_filter_honors_global_mapping(self):
212+
"""
213+
The Same Surnames filter rule gathers a globally-grouped surname.
214+
215+
Pre-fix this is RED: the rule matched the raw surname, so Jones
216+
(grouped under Smith only via the global mapping) was not gathered
217+
with Smith.
218+
"""
219+
from gramps.plugins.quickview.samesurnames import SameSurname
220+
221+
rule = SameSurname(["Smith"])
222+
self.assertTrue(rule.apply_to_one(self.db, self.smith))
223+
self.assertTrue(rule.apply_to_one(self.db, self.jones))
224+
225+
def test_samesurnames_run_path_gathers_the_group(self):
226+
"""
227+
Launching the quick view from either person returns the whole group.
228+
"""
229+
from gramps.plugins.quickview.samesurnames import (
230+
_same_surname_handles,
231+
)
232+
233+
expected = {"H_SMITH", "H_JONES"}
234+
self.assertEqual(set(_same_surname_handles(self.db, self.smith)), expected)
235+
self.assertEqual(set(_same_surname_handles(self.db, self.jones)), expected)
236+
237+
def test_topsurnames_honors_global_mapping(self):
238+
"""
239+
The Top Surnames tally groups the globally-mapped surname under
240+
Smith.
241+
"""
242+
surnames, _ = tally([self.smith, self.jones], db=self.db)
243+
self.assertEqual(surnames.get("Smith"), 2)
244+
self.assertNotIn("Jones", surnames)
245+
246+
def test_consumers_agree_on_the_grouping(self):
247+
"""
248+
The two consumers group the same people together (the invariant).
249+
"""
250+
from gramps.plugins.quickview.samesurnames import (
251+
_same_surname_handles,
252+
)
253+
254+
same_surnames_group = set(_same_surname_handles(self.db, self.smith))
255+
256+
surnames, representative_handle = tally([self.smith, self.jones], db=self.db)
257+
# The single group the gramplet reports, expanded to its members.
258+
top_group = set(
259+
_same_surname_handles(
260+
self.db,
261+
self.db.get_person_from_handle(representative_handle["Smith"]),
262+
)
263+
)
264+
265+
self.assertEqual(same_surnames_group, top_group)
266+
self.assertEqual(same_surnames_group, {"H_SMITH", "H_JONES"})
267+
self.assertEqual(list(surnames), ["Smith"])
268+
269+
155270
if __name__ == "__main__":
156271
unittest.main()

gramps/plugins/gramplet/topsurnamesgramplet.py

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
from gramps.gen.plug import Gramplet
3131
from gramps.gen.config import config
3232
from gramps.gen.const import GRAMPS_LOCALE as glocale
33+
from gramps.gen.display.name import displayer as name_displayer
3334
from gramps.gen.lib import Person
3435
from gramps.gen.plug.menu import NumberOption
3536
from gramps.gen.types import PersonHandle
@@ -52,6 +53,7 @@
5253
#
5354
# ------------------------------------------------------------------------
5455
def record_surnames(
56+
db,
5557
person: Person,
5658
surnames: dict[str, int],
5759
representative_handle: dict[str, PersonHandle],
@@ -60,16 +62,22 @@ def record_surnames(
6062
Tally one person's surnames and choose representatives for them.
6163
6264
The person is counted under every group name taken from their primary and
63-
alternate names. They become the representative for a surname only when it
64-
is their primary group name, or when no representative has been chosen yet.
65-
The Same Surnames quick view re-derives the surname from the
66-
representative's primary name, so a representative whose primary surname
67-
differs would open a report for a different surname than the one clicked.
65+
alternate names. Group names are resolved with
66+
:meth:`~.display.name.NameDisplay.name_grouping_name`, which honours both
67+
the per-name "group as" override and the database-wide name-group mapping,
68+
so two surnames the user grouped together are tallied as one --
69+
consistently with the Same Surnames quick view (bug #6825).
70+
71+
They become the representative for a surname only when it is their primary
72+
group name, or when no representative has been chosen yet. The Same
73+
Surnames quick view re-derives the surname from the representative's
74+
primary name, so a representative whose primary surname differs would open
75+
a report for a different surname than the one clicked.
6876
"""
6977
primary_name = person.get_primary_name()
70-
primary_surname = primary_name.get_group_name().strip()
78+
primary_surname = name_displayer.name_grouping_name(db, primary_name).strip()
7179
allnames = set(
72-
name.get_group_name().strip()
80+
name_displayer.name_grouping_name(db, name).strip()
7381
for name in [primary_name] + person.get_alternate_names()
7482
)
7583
for surname in allnames:
@@ -117,7 +125,7 @@ def main(self):
117125

118126
cnt = 0
119127
for person in self.dbstate.db.iter_people():
120-
record_surnames(person, surnames, representative_handle)
128+
record_surnames(self.dbstate.db, person, surnames, representative_handle)
121129
cnt += 1
122130
if not cnt % _YIELD_INTERVAL:
123131
yield True

gramps/plugins/quickview/filterbyname.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from gramps.gen.utils.file import media_path_full
3030
from gramps.gui.plug.quick import run_quick_report_by_name_direct
3131
from gramps.gen.lib import Person
32+
from gramps.gen.display.name import displayer as name_displayer
3233
from gramps.gen.datehandler import get_date
3334

3435
import os
@@ -463,7 +464,12 @@ def run(database, document, filter_name, *args, **kwargs):
463464
namelist = defaultdict(int)
464465
for person in database.iter_people():
465466
names = [person.get_primary_name()] + person.get_alternate_names()
466-
surnames = list(set([name.get_group_name() for name in names]))
467+
# Tally by the grouping name (per-name "group as" override AND the
468+
# database-wide name-group mapping), matching the Same Surnames
469+
# quick view the double-click below opens (bug #6825).
470+
surnames = list(
471+
set(name_displayer.name_grouping_name(database, name) for name in names)
472+
)
467473
for surname in surnames:
468474
namelist[surname] += 1
469475
stab.columns(_("Surname"), _("Count"))

gramps/plugins/quickview/samesurnames.py

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
from gramps.gen.simple import SimpleAccess, SimpleDoc
3232
from gramps.gui.plug.quick import QuickTable
3333
from gramps.gen.lib import Person
34+
from gramps.gen.display.name import displayer as name_displayer
3435
from gramps.gen.filters.rules import Rule
3536
from gramps.gen.filters import GenericFilterFactory
3637

@@ -60,7 +61,12 @@ class SameSurname(Rule):
6061
def apply_to_one(self, db, person):
6162
src = self.list[0].upper()
6263
for name in [person.get_primary_name()] + person.get_alternate_names():
63-
if name.get_surname() and name.get_surname().upper() == src.upper():
64+
# Match on the grouping name (honouring the per-name "group as"
65+
# override AND the database-wide name-group mapping) rather than the
66+
# raw surname, so a name the user grouped under another surname is
67+
# gathered with it (bug #6825).
68+
group = name_displayer.name_grouping_name(db, name)
69+
if group and group.upper() == src:
6470
return True
6571
return False
6672

@@ -107,6 +113,33 @@ def apply_to_one(self, db, person):
107113
return False
108114

109115

116+
def _same_surname_handles(database, person):
117+
"""
118+
Return the handles of everyone sharing ``person``'s surname group.
119+
120+
The grouping is resolved with
121+
:meth:`~.display.name.NameDisplay.name_grouping_name`, which honours both
122+
the per-name "group as" override and the database-wide name-group mapping,
123+
so the people gathered here are grouped the same way the surname gramplets
124+
group them (bug #6825). ``person`` may be a
125+
:class:`~.lib.person.Person` or a plain group-name string (as passed by
126+
the surname links).
127+
"""
128+
if isinstance(person, Person):
129+
rsurname = name_displayer.name_grouping_name(
130+
database, person.get_primary_name()
131+
)
132+
else:
133+
rsurname = person
134+
surname_filter = GenericFilterFactory("Person")()
135+
if rsurname != "":
136+
rule = SameSurname([rsurname])
137+
else:
138+
rule = IncompleteSurname([])
139+
surname_filter.add_rule(rule)
140+
return surname_filter.apply(database, database.iter_person_handles())
141+
142+
110143
def run(database, document, person):
111144
"""
112145
Loops through the families that the person is a child in, and displays
@@ -118,21 +151,13 @@ def run(database, document, person):
118151
stab = QuickTable(sdb)
119152
if isinstance(person, Person):
120153
surname = sdb.surname(person)
121-
rsurname = person.get_primary_name().get_group_name()
122154
else:
123155
surname = person
124-
rsurname = person
125156
# display the title
126157
sdoc.title(_("People sharing the surname '%s'") % surname)
127158
sdoc.paragraph("")
128159
stab.columns(_("Person"), _("Birth Date"), _("Name type"))
129-
filter = GenericFilterFactory("Person")()
130-
if rsurname != "":
131-
rule = SameSurname([rsurname])
132-
else:
133-
rule = IncompleteSurname([])
134-
filter.add_rule(rule)
135-
people = filter.apply(database, database.iter_person_handles())
160+
people = _same_surname_handles(database, person)
136161

137162
matches = 0
138163
for person_handle in people:

0 commit comments

Comments
 (0)