Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid errors and incorrect listings in Trash Manager #16433

Merged
merged 2 commits into from
Aug 29, 2023

Conversation

smg6511
Copy link
Collaborator

@smg6511 smg6511 commented May 23, 2023

What does it do?

Changed where condition to correctly group $query search targets (pagetitle | longtitle) and perform early return of criteria if no deletable resources exist.

Why is it needed?

Ensures the trash manager list only shows deleted items when additional filtering is applied.

How to test

Clear all caches and experiment with deleting resources and purging them in the trash manager. Also, use the search field in the trash manager to verify all works as expected.

Related issue(s)/PR(s)

Provides a more reliable solution attempted in #16228 and avoids the issue #16415 is trying to solve. Note that while both #16228 and #16415 are targeted to 2.x, the prepareQueryBeforeCount in the changed GetList processor is identical for both 2.x and 3.x, so this solution can be directly backported.

Change where condition to correctly group $query param and perform early return of criteria if no deletable resources exist
@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label May 23, 2023
@smg6511 smg6511 changed the title Trash Manager GetList Avoid errors and incorrect listings in Trash Manager May 23, 2023
@codecov
Copy link

codecov bot commented May 23, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01 ⚠️

Comparison is base (b16a856) 21.73% compared to head (ec35d18) 21.73%.

Additional details and impacted files
@@             Coverage Diff              @@
##                3.x   #16433      +/-   ##
============================================
- Coverage     21.73%   21.73%   -0.01%     
  Complexity    10483    10483              
============================================
  Files           561      561              
  Lines         31614    31618       +4     
============================================
  Hits           6872     6872              
- Misses        24742    24746       +4     
Impacted Files Coverage Δ
...c/Revolution/Processors/Resource/Trash/GetList.php 0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@smg6511 smg6511 added the pr/review-needed Pull request requires review and testing. label May 24, 2023
@smg6511 smg6511 added pr/ready-for-merging Pull request reviewed and tested and ready for merging. and removed pr/review-needed Pull request requires review and testing. labels Aug 16, 2023
@opengeek opengeek merged commit e6a55ab into modxcms:3.x Aug 29, 2023
@smg6511 smg6511 added this to the v3.0.4 milestone Sep 23, 2023
@smg6511 smg6511 deleted the 3.x-trash-filtering-fix branch March 30, 2024 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed CLA confirmed for contributors to this PR. pr/ready-for-merging Pull request reviewed and tested and ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants