N°9950 - Error loading structural data on upgrade

[eespie]

Error loading structural data on upgrade when class has been removed

[Copilot]

Pull request overview

Allows upgrades to continue when historical structural data references a removed class.

Changes:

• Logs and skips unknown XML classes instead of throwing an exception.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[greptile-apps]

Greptile Summary

The PR allows structural data loading to continue when XML references a class absent from the compiled model, addressing upgrades where classes were removed.

• Replaces the unknown-class exception with logging and section skipping.
• The behavior applies to every XMLDataLoader caller, including interactive backoffice imports.

Confidence Score: 4/5

This PR should not merge until unknown-class tolerance is limited to the intended upgrade scenario or propagated as a failure for other import paths.

The unconditional continue allows the backoffice loader to commit a partial import and explicitly report success because the skipped class is recorded in neither the loader's errors nor warnings.

Files Needing Attention: setup/xmldataloader.class.inc.php

Important Files Changed

Filename	Overview
setup/xmldataloader.class.inc.php	Broadens unknown-class tolerance to every loader mode, causing non-upgrade imports to commit and report success after silently omitting invalid sections.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[XML input] --> B{Class exists?}
  B -->|Yes| C[Load records]
  B -->|No| D[Log and skip section]
  C --> E[EndSession succeeds]
  D --> E
  E --> F[Commit and report success]

Loading

_{Reviews (1): Last reviewed commit: "N°9950 - Error loading structural data o..." | Re-trigger Greptile}

[greptile-apps]

Unknown classes produce successful imports

When an administrator imports XML containing a class unavailable in the current data model, this branch records neither a loader error nor a warning, so the backoffice loader commits the valid sections and reports success while silently omitting every object in the unknown-class section.

Knowledge Base Used: Setup: Install/Upgrade Wizard

[dflaven]

It's Ok to continue ignoring the error. But maybe the message entry in the log should be an Info or Warning (since it does not break the setup) and the message should be more explicit, something like: "Loading of data for class '$sClass' ignored because the class does not exist."

[greptile-apps]

Good point. Error is misleading when we're intentionally continuing. Here's a suggestion using Warning (more appropriate than Info since it's still an unexpected condition worth flagging) with a clearer message:

Suggested change

	continue; // Trying to load non-existing class, ignore it
	SetupLog::Warning("Loading of data for class '$sClass' ignored because the class does not exist.");
	continue; // Trying to load non-existing class, ignore it

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.