[#44828] - [5.4] Change backend views to exceptions instead of legacy error handling
- Pending
- Medium
- Build: 5.3-dev
- # 44828
- Diff
- Hackwar:5.3-exceptions-backend1
User tests: Successful: Unsuccessful:
Summary of Changes
With the new flag in the LegacyErrorHandlingTrait to check if exceptions should be used, we now can finally move away from the legacy setError()/getError() to a proper error handling with exceptions. This PR converts the backend views to use this new flag. When set, instead of using the legacy methods, the model code should throw exceptions instead.
Testing Instructions
- Go into every backend view (list and edit views) and check if they still display as before. Since we are only changing the views right now, nothing else has to be checked.
- To check the new behavior in case of an error, we have to artificially trigger an error. You can do so in the list views by for example editing the database query in the
getListQuery()method of the respective model. For example, go toadministrator/components/com_content/src/Model/ArticlesModel.phpand add a column to the select statement which doesn't exist in the database. Then visit the article list view in the backend and see the error. Before applying the patch, you get a strange error, after applying the error should be more descriptive. And you should of course still get an error.
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
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_associations com_banners com_cache com_categories com_checkin com_contact com_content com_contenthistory com_fields com_finder com_installer |
| Title |
|
||||||
| Labels |
Added:
PR-5.3-dev
PR-5.4-dev
|
||
| Labels |
Removed:
PR-5.3-dev
|
||
@exlemor How have you applied the PR? With patchtester? Or have you used the download from drone?
It can be that something is missing because the PR is not up to date with the base branch.
I will trigger a branch update now.
If you have used patchtester, revert the patch, refresh the pulls list and then apply again.
If you have used the download, please wait until a new download is available.
Thanks in advance.
@Hackwar Could you add the PHPStan errors to the phpstan-baseline.neon file? After the branch update from the 5.4-dev branch, where PHPStan is meanwhile mandatory, we have lots of errors Error: Call to deprecated method setUseExceptions() of class Joomla\CMS\MVC\Model\BaseModel:.
@Hackwar before I validate this as a successful test...
I re-installed clean environment and took about 30 minutes to check every backend view (list and edit views) - and everything worked..
BUT, when I did the error triggering, I still got a crash in both cases just with the Patch applied it was considerably shorter - not sure I would call it more descriptive though - (for someone who isn't a coder I mean) so if you meant a shorter stack trace that doesn't include legacy setError()/getError() - then that matches your expectations and I can validate it as successful.
Hi @Hackwar, testing this PR, applied Patch, went to Content --> Articles, crash:
An error has occurred.
0 Call to undefined method Joomla\Component\Content\Administrator\Model\ArticlesModel::setUseExceptions()
same at Content --> Categories, Featured Articles, Field Groups, Site Modules, Administrator Modules, etc
(sorry)