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

Improve file system error feedback #16479

Merged
merged 2 commits into from
Mar 25, 2024
Merged

Conversation

smg6511
Copy link
Collaborator

@smg6511 smg6511 commented Sep 28, 2023

What does it do?

Creates/updates error messaging to be more accurate and descriptive when a file/folder can not be moved or otherwise changed. Also, in second commit, aligns the code formatting to our current rule set.

Why is it needed?

Current error feedback is for the most part unactionable because it's too ambiguous.

How to test

  1. Create at least one file and one folder that have read-only permission.
  2. Verify the new messaging appears and its language is clear by attempting to: a) edit (file), rename*, or move a read-only file/directory; or b) create new files/folders in your new read-only directory.
  • Note that, interestingly, in many cases renaming or moving a read-only file is possible when you have write permissions on the parent/target directory. This is just how unix works and MODX currently doesn't attempt to apply the fine-grained measures that'd be necessary to lock down files to that degree.

Related issue(s)/PR(s)

Resolves #16074

@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Attention: Patch coverage is 22.44898% with 38 lines in your changes are missing coverage. Please review.

Project coverage is 21.57%. Comparing base (f197da7) to head (c7241f2).

Files Patch % Lines
core/src/Revolution/Sources/modMediaSource.php 22.44% 38 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##                3.x   #16479   +/-   ##
=========================================
  Coverage     21.57%   21.57%           
- Complexity    10566    10571    +5     
=========================================
  Files           561      561           
  Lines         31940    31966   +26     
=========================================
+ Hits           6892     6898    +6     
- Misses        25048    25068   +20     

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

@smg6511 smg6511 added the pr/review-needed Pull request requires review and testing. label Oct 30, 2023
Provides more action-specific messaging when a file/folder action can not be taken
Note that one substantive change was made in modMediaSource ~L1849: The default flag for htmlspecialchars_decode was changed in php 8.1, thus applying explicitly what was the default flag at the time this was originally written.
@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 Feb 26, 2024
@opengeek opengeek added this to the v3.1.0 milestone Mar 25, 2024
@opengeek opengeek merged commit c5a4116 into modxcms:3.x Mar 25, 2024
7 checks passed
@smg6511 smg6511 deleted the 3.x-feature-16074 branch March 30, 2024 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

Strange error message when trying to save a file
4 participants