Skip to content

Fix resource delete, trash restore, and nested system pages - #17003

Open
Ibochkarev wants to merge 4 commits into
modxcms:3.xfrom
Ibochkarev:fix/14167-form-delete-cascade-children
Open

Fix resource delete, trash restore, and nested system pages#17003
Ibochkarev wants to merge 4 commits into
modxcms:3.xfrom
Ibochkarev:fix/14167-form-delete-cascade-children

Conversation

@Ibochkarev

@Ibochkarev Ibochkarev commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

What changed and why

Checking Deleted on a resource form saved only that resource. Children stayed deleted=0, so the tree still showed them as live. Restoring a child from trash left its parent deleted, so the child vanished from trash while staying under a trashed folder. Purging a parent permanently removed children but the success message listed only the parent. Moving site_start, the 404 page, or the unavailable page under a folder, then deleting that folder, bypassed the root-level protection.

Resource/Update leaves deleted unchanged on its own save, then runs Resource/Delete or Resource/Undelete. Trash restore undeletes the ancestor chain. Purge includes descendant ids in the success count. Delete fails if the resource or any descendant is site_start, error_page, or site_unavailable_page. Create and move refuse a deleted parent.

How to test

  1. Create a parent with a child. Check Deleted on the parent form and save. Both should be deleted with deletedon / deletedby set.
  2. Restore only the child from trash. The parent should restore as well.
  3. Delete a parent with a child, then purge only the parent. The message count should include the child, and both rows should be gone.
  4. Move a resource that is site_start (or 404 / unavailable) under a folder and try to delete the folder. Delete must fail. The same check already blocks deleting that page at the root.
  5. Try to create a resource under a deleted parent. Create must fail.

Gate E (this session):

  • php -l on touched processors: exit 0
  • core/vendor/bin/phpunit -c _build/test/phpunit.xml --filter 'ResourceUpdateDeleteCascadeTest|ResourceTrashHierarchyTest': OK (6 tests, 64 assertions), exit 0
  • phpcs on new files: 0 errors
  • revolution.test: restore child restored parent (ids 12438/12439); delete of a folder containing site_start blocked; create under deleted parent blocked

Related issue(s)/PR(s)

Resolves #14167. Related: #14350 listed deleted children in trash.

Compatibility notes

Form delete requires the same resource delete / undelete policy as the context menu.

Form delete fires OnBeforeDocFormDelete, OnDocFormDelete, and OnResourceDelete (with children), matching tree Delete.

Restore of a child now also undeletes deleted ancestors (and fires OnResourceUndelete for them).

Breaking change assessment

No public API signature change except removal of unused fireDeleteEvent / fireUnDeleteEvent on Resource/Update (no remaining call sites in core). Safe for patch-level consumers unless an extra called those methods directly.

Test coverage

  • ResourceUpdateDeleteCascadeTest: form delete/undelete cascade
  • ResourceTrashHierarchyTest: trash restore of a child restores the parent; purge counts descendants; nested site_start blocks parent delete; create under a deleted parent fails

Contributors

@Ruslan-Aleev reported. @Jako confirmed children must appear in trash because purge removes them with the parent.

AI tool use

Implemented in Cursor with the issue-to-pr pipeline. Review passes on the form-delete cascade: code-reviewer and thermo-nuclear-code-quality-review. Live checks on revolution.test.

The Deleted checkbox on Resource/Update only flipped the current
resource, so children stayed live in the tree. Defer the flag to
Resource/Delete and Resource/Undelete so form save matches context-menu
soft-delete, including deletedon/deletedby.
@Ibochkarev Ibochkarev added bug The issue in the code or project, which should be addressed. area-core labels Aug 18, 2026
Restoring a child from trash left its deleted parent in place, and
purging a parent only mentioned the parent in the success message.
Deleting a folder that contained site_start, error_page, or
site_unavailable_page also skipped the root-level protection.

Restore now undeletes the ancestor chain. Purge counts removed
descendants. Parent delete fails if the tree includes a system page.
Creating or moving under a deleted parent is refused.
@Ibochkarev Ibochkarev changed the title Fix form delete not marking child resources as deleted Fix resource delete, trash restore, and nested system pages Aug 18, 2026
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 23.58%. Comparing base (5a484de) to head (0cd2521).
⚠️ Report is 25 commits behind head on 3.x.

Files with missing lines Patch % Lines
core/src/Revolution/Processors/Resource/Update.php 68.08% 15 Missing ⚠️
core/src/Revolution/Processors/Resource/Delete.php 91.89% 3 Missing ⚠️
...n/Processors/Resource/RestoresDeletedAncestors.php 87.50% 3 Missing ⚠️
...c/Revolution/Processors/Resource/Trash/Restore.php 62.50% 3 Missing ⚠️
...re/src/Revolution/Processors/Resource/Undelete.php 50.00% 2 Missing ⚠️
...src/Revolution/Processors/Resource/Trash/Purge.php 92.30% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                3.x   #17003      +/-   ##
============================================
+ Coverage     21.67%   23.58%   +1.91%     
- Complexity    10786    10828      +42     
============================================
  Files           566      567       +1     
  Lines         33149    33288     +139     
============================================
+ Hits           7186     7852     +666     
+ Misses        25963    25436     -527     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Connector actions still map Resource/Delete to Processors\Resource, so
that package cannot be renamed. Exclude the soft-reserved keyword sniff
there and in the legacy processor tests.
Codecov flagged 27 uncovered patch lines. Exercise deferred Delete
failure, weblink form delete, move under a deleted parent, nested
error_page, purge of parent+child ids, and Undelete of a child.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-core bug The issue in the code or project, which should be addressed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deleting resources - small bugs

1 participant