? ? Pending

User tests: Successful: Unsuccessful:

avatar richard67
richard67
30 May 2021

Pull Request for #34233 (comment) point 6.

Summary of Changes

Currently we have three places where we use text fields to specify the path to a folder which has to exist and be below the Joomla root:

  • Field "Attachments Folder" in the Mail Templates component (com_mails) options.
  • Fields "Path to Files Folder" and "Path to Images Folder" in the Media Manager (com_media) options.

This is bad because text fields should only be used to define names for new files or folders to be created, and because there is no validation done if the folder really exists if we use text fields to define paths to existing folders.

But currently we have no comfortable way to recursively, i.e. including all sub folders, navigate through all folders in the Joomla root.

For the mean time, as long as we don't have a better solution, this PR here adds a new validation rule "FolderPathExistsRule" to be used in such cases and which checks if the folder exists below the Joomla root, in addition to the validation which is done by the "FilePathRule".

The FilePathRule validation should not be extended to check if a file or folder exists because it shall still be possible to use it for validating paths to not existing files or folders.

The new rule added by this PR should not be used with folderlist fields because 1. it doesn't make sense for them, the "OptionsRule" should be used to validate these fields, and 2. it would not work for these fields because the "directory" element of such fields is not handled by the new rule.

In addition to these changes, this PR removes the "bin" folder from the existing "exclude" elements of the 2 text fields "Path to Files Folder" and "Path to Images Folder" in the Media Manager (com_media) options, because in J4 there is no "bin" folder anymore, not even after updates from 3.10.

Testing Instructions

The test starts on an installation of current 4.0-dev branch or latest 4.0 nightly build without the patch of this PR applied.

All actions are performed in backend.

  1. Go to media manager options.

  2. In one or both of the fields "Path to Files Folder" and "Path to Images Folder", enter a path which is valid e.g. regarding forbidden characters but which doesn't exist below the Joomla root, e.g. "gaga" or "images/gaga".

  3. Save and close.
    Result: Changes are saved, green success message shown.

  4. Go to "System -> Templates -> Mail Templates" and edit the options.

  5. In field "Attachments Folder", enter a path which is valid e.g. regarding forbidden characters but which doesn't exist below the Joomla root, e.g. "gaga" or "images/gaga".

  6. Save the change.
    Result: Changes are saved, green success message shown.

  7. Apply the patch of this PR.

  8. Go to media manager options which have still the path to not existing folder(s) from step 2.

  9. Without making any changes, use the "Save" button.
    Result: The form is not saved, you get an alert about invalid field.

  10. Change the path(s) so they are valid and point to existing folder(s), and save and close.
    Result: Changes are saved, green success message shown.

  11. Go to "System -> Templates -> Mail Templates" and edit the options which have still the path to not existing folder(s) from step 5.

  12. Without making any changes, use the "Save" button.
    Result: The form is not saved, you get an alert about invalid field.

  13. Change the path so it is valid and points to an existing folder which is in the exclude list, e.g. "administrator", and use the "Save" button.
    Result: The form is not saved, you get an alert about invalid field.

  14. Change the path so it is valid and points to an existing folder, and save and close.
    Result: Changes are saved, green success message shown.

Actual result BEFORE applying this Pull Request

See section "What this PR does not fix" in PR #34233 : You can specify paths to not existing folder for the three fields.

Expected result AFTER applying this Pull Request

You can't specify paths to not existing folder for the three fields. The form can't be saved, and an alert is shown, telling which field is invalid.

When specifying paths to existing folders, the fields can be saved.

Documentation Changes Required

Documentation of form field validation rules in J4 needs to be extended by the new rule, including to mention that it only makes sense for text fields and will not work and not make sense with filelist or filderlist fields.

avatar richard67 richard67 - open - 30 May 2021
avatar richard67 richard67 - change - 30 May 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 30 May 2021
Category Administration com_media Libraries Unit Tests
avatar richard67 richard67 - change - 30 May 2021
The description was changed
avatar richard67 richard67 - edited - 30 May 2021
avatar richard67 richard67 - change - 30 May 2021
Labels Added: ? ?
avatar richard67 richard67 - change - 30 May 2021
The description was changed
avatar richard67 richard67 - edited - 30 May 2021
avatar richard67 richard67 - change - 30 May 2021
Labels Added: ?
Removed: ?
avatar richard67
richard67 - comment - 30 May 2021

Wait guys I have to fix another mistake first.

avatar richard67 richard67 - change - 30 May 2021
Labels Added: ?
Removed: ?
avatar PhilETaylor
PhilETaylor - comment - 30 May 2021

Something not right - I cannot save images or media :-(

Screenshot 2021-05-30 at 16 23 54

avatar PhilETaylor
PhilETaylor - comment - 30 May 2021

This is because $pathCleaned = media and so doesnt exist as its not an absolute path.

4f10df6 30 May 2021 avatar richard67 CS
avatar richard67
richard67 - comment - 30 May 2021

This is because $pathCleaned = media and so doesnt exist as its not an absolute path.

Has been fixed with my last commits. Please check again.

avatar richard67
richard67 - comment - 30 May 2021

Something not right - I cannot save images or media :-(

@PhilETaylor Has been fixed meanwhile, please test again.

avatar PhilETaylor PhilETaylor - test_item - 30 May 2021 - Tested successfully
avatar PhilETaylor
PhilETaylor - comment - 30 May 2021

I have tested this item successfully on ee0f510


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34285.

avatar joomdonation joomdonation - test_item - 31 May 2021 - Tested successfully
avatar joomdonation
joomdonation - comment - 31 May 2021

I have tested this item successfully on ee0f510


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34285.

avatar richard67 richard67 - change - 31 May 2021
Labels Added: ?
Removed: ?
avatar richard67 richard67 - alter_testresult - 31 May 2021 - PhilETaylor: Tested successfully
avatar richard67 richard67 - alter_testresult - 31 May 2021 - joomdonation: Tested successfully
avatar richard67
richard67 - comment - 31 May 2021

Previous test results are still valid because the commit after that was just a clean branch update for solving a conflict caused by my other, merged PR.

avatar richard67 richard67 - change - 31 May 2021
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 31 May 2021

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34285.

avatar rdeutz rdeutz - change - 31 May 2021
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-05-31 08:12:10
Closed_By rdeutz
Labels Added: ?
Removed: ?
avatar rdeutz rdeutz - close - 31 May 2021
avatar rdeutz rdeutz - merge - 31 May 2021
avatar richard67
richard67 - comment - 31 May 2021

Thanks all!

Add a Comment

Login with GitHub to post a comment