Skip to content

Resolve child editors before parent commits (issue #7924)#2449

Open
eduralph wants to merge 1 commit into
gramps-project:maintenance/gramps61from
eduralph:enhancement/7924-child-editor-reference-lost-on-parent-save
Open

Resolve child editors before parent commits (issue #7924)#2449
eduralph wants to merge 1 commit into
gramps-project:maintenance/gramps61from
eduralph:enhancement/7924-child-editor-reference-lost-on-parent-save

Conversation

@eduralph

@eduralph eduralph commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

User impact: While editing a family you can open a second editor to add a brand-new person as the mother (or father). If you confirm the family editor first — before the new person's own editor is saved — the family is saved without that person. The person ends up saved but orphaned, and the family has no mother, so the link you were building silently disappears.

This resolves any open child editors first, so the parent editor picks up the new person's reference before it saves.

Reported in Mantis #7924.

What to look at

Primary editors are non-modal, so a parent (e.g. the Family editor) and a child editor it spawned (e.g. adding a new mother) can be open at once — but the child's handle only reaches the parent when the child saves. Confirming the parent first commits the family before the child ever saved, losing the link. The fix routes both of the parent's save doors (OK button and the close/"Save Changes?" path) through one guard that resolves open dependent children — deepest-first — before the parent reads its references. To try it: add a family, click to add a new person as the mother, type a name, then click the family's OK (without OK-ing the person editor first). Pre-fix the family commits with no mother and the person is orphaned; post-fix the child resolves first and the mother is linked.

Root cause

Primary editors are non-modal, so a parent editor (e.g. EditFamily) and a child primary editor it spawned (e.g. EditPerson, opened via "Add a new person as the mother") can be open at once. The child's handle is written onto the parent only by the child's completion callback (EditFamily.new_mother_addedgramps/gui/editors/editfamily.py:973), which runs when the child saves.

Confirming the parent first runs EditFamily.__do_save (gramps/gui/editors/editfamily.py:1226): it reads get_mother_handle() (:1299) — still None, because the child never saved — and commits the family without the mother, before _do_close() (:1344) tears the child down. The reference is lost at commit time, not teardown: even choosing "Save" on a later prompt writes the mother handle onto a family object that was already committed. Result: the person is saved but orphaned, the family has no mother (Mantis 7924).

Fix

Resolve open dependent child editors in the shared EditPrimary save path, before the parent reads its references and commits:

  • Route every parent save door through one guard, _save_with_dependent_children: both the OK button (define_ok_button) and the window-X / Cancel → "Save Changes?" path (close()). Covering only the OK button would leave the close/Save door committing an incomplete graph.
  • _resolve_dependent_children collects the open child EditPrimary instances that carry a completion callback (callback is not None) — the sign that the child writes a handle back onto the parent. A child opened to edit an existing object (EditFamily.edit_person — no callback) writes nothing back and is left alone, so the common case is unchanged.
  • Each such child is resolved via its own "Save Changes?" guard (the existing SaveDialog and completion-callback machinery — no new UI). Whether the parent may proceed is read from child.opened after the attempt: a saved/discarded child closes; a Cancel or a validation error leaves it open, and the parent save aborts (nothing committed).
  • The "which children, in what order" decision is extracted to a new pure module, gramps/gui/savecascade.py, free of any GTK import so it is unit-testable headless. It walks the GrampsWindowManager window tree and returns children deepest-first, so a nested Place → enclosing-Place chain resolves innermost first and each callback has landed before its parent reads references. The live save path drives the identical functions on the real tree.

Verification

  • Claim: The patch lets focused child editors that write a reference back be resolved (deepest-first) before the parent commits, while children editing an existing object are left alone.
  • Checked: on maintenance/gramps61gramps/gui/editors/editfamily.py:950 (add_mother_clicked opens EditPerson with the new_mother_added callback — writes a reference back) vs :1095 (edit_person opens EditPerson with no callback — correctly not resolved: the over-trigger guard); :1299,1344 — the family is committed reading get_mother_handle() before _do_close(), confirming the loss is at commit time. gramps/gui/editors/editprimary.pydefine_ok_button and close() are the two save doors, both now routing through _save_with_dependent_children; new methods _resolve_dependent_children, _is_unresolved_dependent_child, _resolve_before_parent_commit. gramps/gui/managedwindow.py:143 (get_item_from_track) and :192 (recursive_action) — the window-tree shape savecascade walks (deepest-first, head excluded). po/POTFILES.skip — registers the new savecascade.py module and its test (no translatable strings).
  • Test: gramps/gui/test/savecascade_test.py — 13 headless unit tests over the extracted decision (descendant_leaves, children_to_resolve): leaf/branch handling, deepest-first ordering, and the selection predicate including the callback-filtering guard (a dirty child without a pending reference is not force-resolved). They drive the same functions the live save path calls, not a copy. All pass with the fix; the full core unit suite passes with no new failures. The database end-state — after adding a family, adding a new mother, typing a name, and clicking the Family OK, the committed family's mother_handle resolves to the created person — is exercised by an AT-SPI/dogtail reproduction of the reporter's flow; without the fix the family commits with mother_handle == None and the person is orphaned.

Fixes #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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants