Form: fix crash and surface clear errors for malformed XML (bug 11707) - #6
Merged
Merged
Conversation
A family section whose title lacked the expected 'X/Y' separator caused a ValueError: not enough values to unpack when the form editor opened, crashing the Forms gramplet. The underlying issue was that the addon trusted the XML definitions and had no schema validation or user-facing error reporting for broken files. Split the validation out of form.py into a pure-Python form_validator module (no GTK/Gramps imports) so it can be unit-tested without a GUI. The Form loader now: * parses each file defensively (ExpatError -> ErrorDialog), * runs the validator before loading (invalid files -> ErrorDialog with the file path, offending form id, and the rule that failed), * skips any <form> element that fails validation while still loading sibling well-formed forms from the same file. split_family_title() in form_validator belt-and-braces the FamilySection constructor so a missing separator no longer raises, even if validation is bypassed. Also adds diagnostic logging: * INFO log of forms loaded per file, * DEBUG trace of each file parsed and each form id loaded/skipped, * DEBUG when EditForm opens (event/citation handles), * WARNING in FamilySection if its title lacks 'X/Y'. Tests: * Form/tests/test_form_validator.py -- 32 pure-Python unit tests, covers split_family_title, every validation branch, parse_and_validate file handling, and a sanity check that every shipped form_*.xml passes validation. * Form/tests/test_integration_form.py -- 6 integration tests that patch ErrorDialog to verify the loader surfaces syntax errors, invalid family titles, missing role, and invalid section types; partially broken files still load their valid forms; shipped files trigger no dialogs. Partially addresses bug 0011010 (request for user error dialog for unsupported elements) by covering its core ask: clear errors for invalid section types, missing/empty role, missing/empty type, and XML syntax errors. Fixes gramps-project/gramps#11707 Refs gramps-project/gramps#11010 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
ValueError: not enough values to unpack (expected 2, got 1)crash in the Forms addon'sFamilySectionwhen a<section type='family'>title omits theX/Yseparator (gramps-project/gramps#11707).form_validatormodule so the loader can reject malformed XML up front, with user-facingErrorDialogs naming the file, form, and rule that failed.role, missing/emptytype, and XML syntax errors.What changed
Loader (
Form/form.py)ErrorDialogs with the file path and a precise message.<form>entries — one bad section no longer nukes the whole file.INFOcount of forms loaded per file,DEBUGper file/form trace.Validator (
Form/form_validator.py, new)split_family_title()— belt-and-braces helper soFamilySectionnever raises on a malformed title.validate_form_element()/validate_form_dom()— returns a list of human-readable errors; free of GTK/Gramps imports so it runs in any CI job.parse_and_validate()— file-level entry point that catches both parse and schema errors cleanly.Editor (
Form/editform.py)split_family_title()instead of the rawtitle.split('/')that blew up in #11707.DEBUGonEditFormopen and aWARNINGinFamilySectionif the title lacks the separator (so future regressions are visible in logs, not just as a traceback).Translations (
Form/po/template.pot) — three new translatable dialog titles.Tests
All green locally (38 passed, 7 subtests):
Form/tests/test_form_validator.py— 32 pure-Python unit tests: every branch ofsplit_family_title, every validation rule,parse_and_validatefile handling, plus a sanity test that every shippedform_*.xmlpasses validation.Form/tests/test_integration_form.py— 6 integration tests that patchErrorDialogand assert: syntax errors, invalid family titles (the exact #11707 repro), missingrole, and invalid section types all raise a clear dialog; partially-broken files still load their valid forms; shipped files trigger zero dialogs.Test plan
python3 -m pytest Form/tests/ -v— 38 passedpython3 -m pytest Form/tests/test_form_validator.pyruns pure Python, no Gramps requiredruff check Form/— cleanpython3 -m py_compile Form/*.py— cleancustom.xmlwith an invalid<section type='Person'>shows the new ErrorDialog naming the file, form, and rule.custom.xmlwith the #11707 family-title repro shows a clear dialog instead of theValueErrortraceback.Fixes gramps-project/gramps#11707
Refs gramps-project/gramps#11010
🤖 Generated with Claude Code