[#47261] - [6.2] Remove getErrors() from all views
- Pending
- Medium
- Build: 6.1-dev
- # 47261
- Diff
- Hackwar:6.1-legacy-error-views
User tests: Successful: Unsuccessful:
Pull Request resolves # .
- I read the Generative AI policy and my contribution is either not created with the help of AI or is compatible with the policy and GNU/GPL 2 or later.
Summary of Changes
This is another step in the process of converting from the old getErrors()/setError() to using exceptions instead. The code currently catches the exception and converts it to a call to setError(), then later code again reads the error and hands it over to the next error a layer up. At the end, we are reading the error and throw an exception. This PR simply cuts out the middle man and allows the original exception to be used directly. From my perspective, this should be backwards compatible and thus it should be possible to accept this in a minor release.
The setUseException() call can then again be removed in 7.0.
I did some further cleanup of the featured view of com_contact in this PR.
Testing Instructions
Trigger errors and see that they show up equally to before?
Link to documentations
Please select:
-
Documentation link for guide.joomla.org:
-
No documentation changes for guide.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_banners com_categories com_fields com_finder com_menus com_redirect com_users Front End com_contact com_content com_newsfeeds com_tags |
Does it make sense to make a change like this to use a deprecated method? Maybe that method should be un-deprecated?
yes make sense, because in 7 or 8 you only have to remove this method call and don't have to find this lines what hannes did now.
I just remember Nils teaching that when we deprecate something we should only do so when we have a replacement and we should use the replacement immediately and that the deprecation notice was for others to see
yes that's true for example for the Factory::getDatabase() to Factory::getContainer()->get('db') stuff because the change is simply the wrong way to go, we should inject everything we need and "never" use a super global like the Factory to get such stuff.
this is another case because this is the final solution, we switch to Exceptions and this method is the preparation for this and will simply be removed and not replaced with a "proper" solution.
thanks for the explanation
| Labels |
Added:
PR-6.1-dev
|
||
Question: in administrator/components/com_banners/src/View/Client/HtmlView.php do you still need $model->setUseExceptions(true); if the getErrors() function is no longer called? The same question applies to many of the HtmlView.php files in the PR.
This pull request has been automatically rebased to 6.2-dev.
| Title |
|
||||||
| Labels |
Added:
Feature
PR-6.2-dev
|
||
@ceford Yes, of course you need to do that call. The whole point is, that the view shouldn't have to actively check for errors, but that the model and table object should directly throw an exception. In order for that to work, the model needs to know that it needs to throw exceptions. That is why we need that flag in the first place. Calling that flag allows us to remove the generic error handling code from our views. Only when we switch that to on by default, then we can remove that call.
Does it make sense to make a change like this to use a deprecated method? Maybe that method should be un-deprecated?