Skip to content

Commit 747ad4c

Browse files
committed
Resolve child editors before parent commits (issue #7924)
When a primary editor (e.g., EditFamily) is confirmed while a child primary editor it spawned is still open with unsaved data, the parent was committing before the child's completion callback could run. This caused the parent to persist without the child's reference (e.g., a new person as mother), orphaning the child's data. The fix relocates the child-resolve step to before the parent's commit, in the shared editor machinery (EditPrimary). Now when a parent is confirmed, the save path first resolves open dependent child editors by driving each child's existing save-guard (the "Save Changes?" prompt), so the child's completion callback lands on the parent's object before the parent reads its references and commits. If any child is left unresolved (the user chose "keep editing"), the parent save aborts entirely so no partial graph is persisted. This also closes a hole in the close-path (window-X, Cancel + Save) which previously bypassed the child-resolve step and committed the parent with the same reference-drop defect. The pure decision logic (which children must be resolved, in what order) is extracted to the new module gramps/gui/savecascade.py so it can be unit-tested headless while the live save path drives the same functions on the real window tree. Fixes #7924 Signed-off-by: Eduard Ralph <eduard@ralphovi.net>
1 parent 0d9e148 commit 747ad4c

4 files changed

Lines changed: 379 additions & 2 deletions

File tree

gramps/gui/editors/editprimary.py

Lines changed: 131 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
from gramps.gen.lib import PrimaryObject
5252
from ..dbguielement import DbGUIElement
5353
from ..uimanager import ActionGroup
54+
from ..savecascade import children_to_resolve
5455

5556

5657
class EditPrimary(ManagedWindow, DbGUIElement, metaclass=abc.ABCMeta):
@@ -81,6 +82,10 @@ def __init__(
8182
self.db = state.db
8283
self.callback = callback
8384
self.ok_button = None
85+
# The editor's confirm handler, captured by define_ok_button and driven
86+
# by _save_with_dependent_children. Defaults to save so the shared save
87+
# guard is safe even before/without define_ok_button (#7924).
88+
self._ok_function = self.save
8489
self.get_from_handle = get_from_handle
8590
self.get_from_gramps_id = get_from_gramps_id
8691
self.contexteventbox = None
@@ -177,9 +182,128 @@ def object_is_empty(self):
177182

178183
def define_ok_button(self, button, function):
179184
self.ok_button = button
180-
button.connect("clicked", function)
185+
# Do not wire the editor's confirm handler straight to the OK button.
186+
# Route it -- and every other save door (see close()) -- through
187+
# _save_with_dependent_children first: when this editor is confirmed
188+
# while a child primary editor it spawned is still open holding unsaved
189+
# data whose completion callback must land a reference on this editor's
190+
# working object, that child is resolved BEFORE this editor reads its
191+
# references and commits. See Mantis #7924.
192+
self._ok_function = function
193+
button.connect("clicked", self._save_with_dependent_children)
181194
button.set_sensitive(not self.db.readonly)
182195

196+
def _save_with_dependent_children(self, *args):
197+
"""Confirm handler shared by every save door: resolve open dependent
198+
child editors, then save.
199+
200+
A primary editor is non-modal, so it can be confirmed while a child
201+
primary editor it spawned -- e.g. an EditPerson opened as "add a new
202+
person as the mother" from EditFamily -- is still open with unsaved
203+
data. That child's completion callback, which writes the child's
204+
handle onto *this* editor's working object (EditFamily.new_mother_added),
205+
only runs when the *child* saves. If this editor commits first, it
206+
persists an object graph with that reference silently dropped and the
207+
child's data orphaned -- the #7924 defect.
208+
209+
So drive each open dependent child's own save-guard first. If any child
210+
is left unresolved (the user chose "keep editing" on its "Save Changes?"
211+
prompt, or its save aborted on a validation error), abort this editor's
212+
save entirely -- it stays open and nothing is committed, so no partial
213+
graph is ever persisted.
214+
215+
This is wired to BOTH the OK button (define_ok_button) and the
216+
close/Cancel/window-X save path (close()'s SaveDialog), so the child is
217+
resolved no matter which door commits the parent (Mantis #7924).
218+
"""
219+
if not self._resolve_dependent_children():
220+
return
221+
self._ok_function(*args)
222+
223+
def _resolve_dependent_children(self):
224+
"""Resolve open child primary editors holding unsaved data before commit.
225+
226+
Returns True if the save may proceed -- every dependent child editor
227+
was resolved (saved, or discarded by the user), or there were none.
228+
Returns False if a child was left unresolved, in which case the parent
229+
save must abort.
230+
231+
Children are resolved deepest-first (see
232+
:func:`~gramps.gui.savecascade.children_to_resolve`) so that, in a
233+
nested chain, each level's completion callback has already landed on
234+
its own parent before that parent reads its references.
235+
"""
236+
if self.uistate is None:
237+
return True
238+
try:
239+
item = self.uistate.gwm.get_item_from_track(self.track)
240+
except (IndexError, KeyError):
241+
return True
242+
for child in children_to_resolve(item, self._is_unresolved_dependent_child):
243+
if not child._resolve_before_parent_commit():
244+
return False
245+
return True
246+
247+
def _is_unresolved_dependent_child(self, window):
248+
"""True if ``window`` is an open child primary editor with a pending
249+
reference this editor's commit would drop.
250+
251+
Only a primary editor opened WITH a completion callback (``callback``
252+
is not None -- e.g. EditFamily.new_mother_added) writes a handle back
253+
onto the spawning editor's object, so only such a child must be
254+
resolved before this editor commits. A child opened to edit an
255+
*existing* object carries no callback (EditFamily.edit_person passes
256+
none) -- committing this editor drops nothing, so it is left alone and
257+
the common case is unchanged (#7924 over-trigger guard). Selectors and
258+
secondary/reference windows are excluded by the EditPrimary check.
259+
"""
260+
return (
261+
window is not self
262+
and isinstance(window, EditPrimary)
263+
and getattr(window, "opened", False)
264+
and getattr(window, "callback", None) is not None
265+
and window.data_has_changed()
266+
)
267+
268+
def _resolve_before_parent_commit(self):
269+
"""Drive this child editor's save-guard as part of a parent's commit.
270+
271+
Called on a child primary editor when its parent is confirmed while
272+
this child is still open with unsaved data. It reuses the very same
273+
"Save Changes?" guard the direct-close path shows (:meth:`close`) --
274+
Save runs this editor's own save (its completion callback lands our
275+
reference on the parent's object), "Close without saving" discards our
276+
edits, Cancel keeps this editor open.
277+
278+
Whether the parent save may proceed is read from *this* editor's own
279+
window state AFTER the attempt, never assumed up front: a successful
280+
save (or an explicit discard) closes this editor (``self.opened``
281+
becomes False); a Cancel ("keep editing") or a validation-aborted save
282+
leaves it open. So:
283+
284+
* closed -> return True (resolved; the parent may commit);
285+
* open -> return False (unresolved; the parent must abort).
286+
"""
287+
if config.get("interface.dont-ask"):
288+
# The user opted out of the "Save Changes?" prompt. Honour that as
289+
# "save without asking" -- a silent save of this child, NOT as
290+
# skipping its save (which would drop its reference and revive the
291+
# #7924 defect for dont-ask users). A validation error still leaves
292+
# the editor open, handled by the check below.
293+
self.save()
294+
else:
295+
SaveDialog(
296+
_("Save Changes?"),
297+
_(
298+
"If you close without saving, the changes you "
299+
"have made will be lost"
300+
),
301+
self._do_close,
302+
self.save,
303+
parent=self.window,
304+
)
305+
return not self.opened
306+
183307
def define_cancel_button(self, button):
184308
button.connect("clicked", self.close)
185309

@@ -252,7 +376,12 @@ def close(self, *obj):
252376
"have made will be lost"
253377
),
254378
self._do_close,
255-
self.save,
379+
# Save via the shared guard, not self.save directly: closing a
380+
# parent editor with the window-X or Cancel and choosing Save
381+
# must ALSO resolve an open dependent child first, or the parent
382+
# commits with the child's reference dropped -- the #7924 defect
383+
# via a second save door (see _save_with_dependent_children).
384+
self._save_with_dependent_children,
256385
parent=self.window,
257386
)
258387
return True

gramps/gui/savecascade.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
#
2+
# Gramps - a GTK+/GNOME based genealogy program
3+
#
4+
# Copyright (C) 2026 The Gramps Developers
5+
#
6+
# This program is free software; you can redistribute it and/or modify
7+
# it under the terms of the GNU General Public License as published by
8+
# the Free Software Foundation; either version 2 of the License, or
9+
# (at your option) any later version.
10+
#
11+
# This program is distributed in the hope that it will be useful,
12+
# but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
# GNU General Public License for more details.
15+
#
16+
# You should have received a copy of the GNU General Public License along
17+
# with this program; if not, see <https://www.gnu.org/licenses/>.
18+
#
19+
20+
"""
21+
Pure logic for the "resolve dependent child editors before a parent editor
22+
commits" decision (Mantis #7924).
23+
24+
This module is deliberately free of any GTK / ``gramps.gui`` imports so the
25+
decision it embodies -- *which* open child windows a parent editor must
26+
resolve before it persists its object graph, and *in what order* -- can be
27+
unit-tested headless while the live save path (:class:`gramps.gui.editors.
28+
editprimary.EditPrimary`) drives the very same functions on the real window
29+
tree.
30+
31+
The window tree is the structure :class:`gramps.gui.managedwindow.
32+
GrampsWindowManager` maintains: a *branch* is a list whose first element is
33+
the head (parent) window of a group and whose remaining elements are the
34+
windows spawned from it (each itself a branch or a leaf); a *leaf* is any
35+
non-list item (a single managed window). See ``GrampsWindowManager`` for the
36+
authoritative description of the tree.
37+
"""
38+
39+
40+
def descendant_leaves(item):
41+
"""Return the leaf windows spawned *below* the tree node ``item``.
42+
43+
``item`` is a node of the ``GrampsWindowManager`` window tree (obtained
44+
e.g. via ``get_item_from_track``). If it is a branch, its head window
45+
(``item[0]``) is excluded and every descendant leaf is returned ordered
46+
**deepest-child-first**, so a caller resolving a save cascade processes
47+
the innermost spawned editor before the editor that spawned it (the
48+
nested Place -> enclosing-Place -> ... chain in #7924). A leaf node (a
49+
window with no spawned children) yields an empty list.
50+
"""
51+
leaves = []
52+
if isinstance(item, list):
53+
for sub_item in item[1:]:
54+
_collect(sub_item, leaves)
55+
return leaves
56+
57+
58+
def _collect(item, leaves):
59+
"""Append the leaf windows under ``item`` to ``leaves``, deepest-first."""
60+
if isinstance(item, list):
61+
for sub_item in item[1:]:
62+
_collect(sub_item, leaves)
63+
# The head of this sub-branch comes *after* the windows it spawned,
64+
# so children are always resolved before their own parent.
65+
_collect(item[0], leaves)
66+
else:
67+
leaves.append(item)
68+
69+
70+
def children_to_resolve(item, needs_resolving):
71+
"""Return the descendant windows a parent editor must resolve before it
72+
commits, deepest-child-first.
73+
74+
``item`` is the parent editor's node in the window tree; ``needs_resolving``
75+
is a predicate ``needs_resolving(window) -> bool`` the caller supplies. In
76+
production it is true for an open, dirty child *primary* editor that still
77+
carries a completion callback (one whose callback still has to land a
78+
reference on the parent's object); being a plain callable keeps this
79+
decision fully unit-testable with fakes.
80+
"""
81+
return [win for win in descendant_leaves(item) if needs_resolving(win)]

0 commit comments

Comments
 (0)