bug PR-5.2-dev PR-5.3-dev Pending

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
24 Apr 2024

Summary of Changes

We are in the process of migrating away from the CMS filesystem package to use the framework version of it. This PR converts all instances where File::makesafe() is used to use the framework version. In that process, all other calls to the CMS File class in that file were converted to properly handle possible exceptions from the framework equivalent. This intentionally did not add additional error handling. I expect that to be a separate effort in the future.

Testing Instructions

Codereview.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

avatar Hackwar Hackwar - open - 24 Apr 2024
avatar Hackwar Hackwar - change - 24 Apr 2024
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 24 Apr 2024
Category Administration com_content com_contenthistory com_templates Libraries Front End Plugins
avatar Hackwar Hackwar - change - 25 Apr 2024
Labels Added: PR-5.2-dev
avatar HLeithner
HLeithner - comment - 25 Apr 2024

missing test instructions, you change the template and the media manager plugins and more at the same time.

avatar HLeithner
HLeithner - comment - 29 May 2024

@Hackwar the tests are failing because of this PR

avatar muhme muhme - test_item - 24 Aug 2024 - Tested unsuccessfully
avatar muhme
muhme - comment - 24 Aug 2024

I have tested this item ? unsuccessfully on f33810e

OK System Tests (excluding installation step):
I ran the system tests both before and after applying the PR. All specifications passed in both runs.

Code validation failed: should exceptions be caught without any action? That may be fine in LocalAdapter.php:859 when deleting a TMP file fails, but even there it's not good practice to comment why the exception is ignored? and why is the exception ignored in LocalAdapter.php:306?

Failed: Since no tests are defined. Isn't the code review meant for theory and needs to be checked in practice? If test cases are added, I would test again.


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

avatar HLeithner
HLeithner - comment - 2 Sep 2024

This pull request has been automatically rebased to 5.3-dev.

avatar HLeithner HLeithner - change - 2 Sep 2024
Title
[5.2] Refactor all instances of File::makesafe() to use framework
[5.3] Refactor all instances of File::makesafe() to use framework
avatar HLeithner HLeithner - edited - 2 Sep 2024
avatar Hackwar Hackwar - change - 3 Sep 2024
Title
[5.3] Refactor all instances of File::makesafe() to use framework
[5.2] Refactor all instances of File::makesafe() to use framework
avatar Hackwar Hackwar - edited - 3 Sep 2024
avatar Hackwar Hackwar - change - 5 Sep 2024
Labels Added: bug
avatar joomla-cms-bot joomla-cms-bot - change - 6 Sep 2024
Category Administration com_content com_contenthistory com_templates Libraries Front End Plugins Administration com_content com_contenthistory com_media com_templates Libraries Front End Plugins
avatar joomla-cms-bot joomla-cms-bot - change - 6 Sep 2024
Category Administration com_content com_contenthistory com_templates Libraries Front End Plugins com_media Administration com_content com_contenthistory com_templates Libraries Front End Plugins
avatar joomla-cms-bot joomla-cms-bot - change - 12 Sep 2024
Category Administration com_content com_contenthistory com_templates Libraries Front End Plugins Administration com_content com_contenthistory com_templates External Library Composer Change Libraries Front End Plugins
avatar Hackwar Hackwar - change - 12 Sep 2024
Labels Added: Composer Dependency Changed
avatar Hackwar Hackwar - change - 13 Sep 2024
Title
[5.2] Refactor all instances of File::makesafe() to use framework
[5.3] Refactor all instances of File::makesafe() to use framework
avatar Hackwar Hackwar - edited - 13 Sep 2024
avatar joomla-cms-bot joomla-cms-bot - change - 13 Sep 2024
Category Administration com_content com_contenthistory com_templates Libraries Front End Plugins External Library Composer Change Repository Administration com_admin SQL Postgresql com_associations com_banners com_categories com_contact com_content com_contenthistory com_fields com_finder com_joomlaupdate
avatar rdeutz rdeutz - change - 18 Sep 2024
Labels Added: PR-5.3-dev
Removed: Composer Dependency Changed
avatar joomla-cms-bot joomla-cms-bot - change - 18 Sep 2024
Category Administration com_content com_contenthistory Repository com_admin SQL Postgresql com_associations com_banners com_categories com_contact com_fields com_finder com_joomlaupdate Administration com_content com_contenthistory com_templates Libraries Front End Plugins

Add a Comment

Login with GitHub to post a comment