Skip to content

Detect broken XML paths and offer to fix on form load#1220

Open
jingcheng16 wants to merge 3 commits intomasterfrom
jc/fix-missing-path
Open

Detect broken XML paths and offer to fix on form load#1220
jingcheng16 wants to merge 3 commits intomasterfrom
jc/fix-missing-path

Conversation

@jingcheng16
Copy link
Copy Markdown
Contributor

@jingcheng16 jingcheng16 commented Apr 8, 2026

Product Description

When a form has hand-edited XML with wrong paths (e.g., a repeat group referencing /data/members_repeat instead of /data/sec_members/members_repeat), the form silently breaks — Question ID becomes blank and uneditable, and saving the form causes data loss. User is confused and don't know how to fix it, fill in a Question ID won't work.

After this change, a modal appears on form load showing the broken paths and their corrections (e.g., /data/members_repeat/data/sec_members/members_repeat), with a "Fix and Reload" button that corrects the XML automatically.

Screenshot 2026-04-07 at 11 23 42 PM

I then realized I can apply same logic to broken path in bind node.
Screenshot 2026-04-07 at 11 33 23 PM
Thus the modal will become the following:
Screenshot 2026-04-07 at 11 34 17 PM

Technical Summary

Ticket: https://dimagi.atlassian.net/browse/SAAS-19569

Why Question ID breaks

When a control element's path doesn't resolve via getMugByPath, the parser creates an orphaned "control-only" mug with no data node. This causes a chain of failures:

  1. isControlOnly = trueabsolutePath returns null (code)
  2. absolutePath null → hashtagPath null
  3. The nodeID setter calls moveMug("rename", value) which skips the entire rename block for control-only mugs
  4. Result: Question ID stays blank and can't be set through the UI

How this PR fixes it

During parsing, when getMugByPath fails, findDataNodeByLeafName searches for an unclaimed DataBindOnly mug with the same node name (e.g., finds members_repeat at /data/sec_members/members_repeat). If found, the correction is stored on form.pathCorrections. After parsing, showPathCorrectionModal displays the corrections. "Fix and Reload" does a find-and-replace on the raw XML and reloads the form, which resolves the control to the correct data node and restores Question ID.

Feature Flag

N/A

Safety Assurance

Safety story

This only activates when getMugByPath returns null for a control element's path — a case that previously resulted in silent data loss. Normal forms with valid paths are unaffected. The fix is opt-in via the modal (user can click "Ignore" to skip).

Automated test coverage

No existing tests cover malformed XML path handling. Manual testing with broken XML files confirms the modal appears and fix works correctly.

QA Plan

  • Load a form with a wrong repeat path — modal should appear showing /data/members_repeat/data/sec_members/members_repeat
  • Click "Fix and Reload" — form reloads with corrected paths, Question ID works
  • Click "Ignore" — form loads as before (broken), no crash
  • Load a normal form — no modal, no change in behavior

Rollback instructions

  • This PR can be reverted after deploy with no further considerations

Labels & Review

  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

🤖 Generated with Claude Code

@jingcheng16 jingcheng16 force-pushed the jc/fix-missing-path branch 2 times, most recently from 946c73e to a0ccc94 Compare April 8, 2026 03:09
@jingcheng16 jingcheng16 force-pushed the jc/fix-missing-path branch from a0ccc94 to b359ba4 Compare April 8, 2026 03:17
jingcheng16 and others added 2 commits April 7, 2026 23:23
When a form has hand-edited XML with wrong control element paths,
detect the broken paths during parsing by matching leaf names against
existing data nodes. After loading, show a modal listing the broken
paths and their corrections with a "Fix and Reload" button that
corrects the XML and reloads the form.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Neither button should be pre-focused so the user makes a deliberate
choice between "Fix and Reload" and "Ignore".

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jingcheng16 jingcheng16 force-pushed the jc/fix-missing-path branch from b359ba4 to b9ac428 Compare April 8, 2026 03:23
Apply the same leaf-name matching to bind elements. When a bind
references a path like /data/total_score but the data node exists
at /data/sec_scoring/total_score, include it in the path corrections
shown in the modal instead of silently discarding the bind.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jingcheng16 jingcheng16 marked this pull request as ready for review April 8, 2026 03:35
@jingcheng16 jingcheng16 requested a review from millerdev April 8, 2026 03:35
Copy link
Copy Markdown
Contributor

@millerdev millerdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should attempt to fix manual XML editing mistakes. All kinds of errors could be made, and there is no way we can possibly handle all of them. We may end up breaking the form even more with automated fixups. At best we would be guessing, which violates the Zen "In the face of ambiguity, refuse the temptation to guess."

Comment thread src/parser.js
* @param path - the broken path (e.g., "/data/members_repeat")
* @returns - the matching DataBindOnly mug, or null
*/
function findDataNodeByLeafName(form, path) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if multiple data nodes are matched? It doesn't know which is the correct one. As implemented, it chooses the last match.
Imagine a scenario where two different mugs are missing data nodes, and both have the same node leaf name. I think both would be assigned the same data node, which would be incorrect.

@jingcheng16
Copy link
Copy Markdown
Contributor Author

jingcheng16 commented Apr 8, 2026

All kinds of errors could be made, and there is no way we can possibly handle all of them. We may end up breaking the form even more with automated fixups.

@millerdev It's true. 😞 Then in this case, what can we do to help user except for ask them to delete the repeat group and rebuild it? I built this mainly because I think all warning message is misleading, and user is not able to fix it in the UI.
All children in the repeat group would have this error:
Screenshot 2026-04-08 at 3 17 33 PM

@millerdev
Copy link
Copy Markdown
Contributor

millerdev commented Apr 8, 2026

what can we do to help user except for ask them to delete the repeat group and rebuild it?

That is one option. If they have edited XML by hand, they may know enough to fix the error by editing XML, so maybe that could be suggested first? If they do not know how to fix it then the fall-back option of deleting the group and rebuilding is probably the best path forward.

It's true, the error message you referenced is pretty technical. I think it is a reasonable and correct error message if they have made a mistake while hand-editing XML. At the point of seeing it they should know that they have made a mistake while editing XML, and should probably attempt to fix it there rather than in the UI.

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