-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[6.0] Unify loadFormData() to return an object #44148
base: 6.0-dev
Are you sure you want to change the base?
Conversation
administrator/components/com_config/src/Model/ComponentModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_languages/src/Model/LanguageModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_templates/src/Model/TemplateModel.php
Outdated
Show resolved
Hide resolved
I am sorry, this not going to work. |
….php Co-authored-by: Quy <quy@nomonkeybiz.com>
Co-authored-by: Quy <quy@nomonkeybiz.com>
….php Co-authored-by: Quy <quy@nomonkeybiz.com>
@Fedik Right now in 90% of the cases, you get a CMSObject in return, so this is more correct than before. All our code either expects an object or converts to an object. Array has been superseded by reality here effectively. |
yes, and no :) Look, this looks not good (and other similar places): joomla-cms/libraries/src/MVC/Model/FormBehaviorTrait.php Lines 133 to 136 in 8e2814a
This will break everything where we have The method I do not know how to resolve all of these, but personally, I think in conext of |
I agree with @Fedik that it would be better to unify to array, but if object then we maybe should return null in such cases like mentioned above and not an empty object. |
4 out of 43 models in our system return an array instead of an object. |
Summary of Changes
We are in the process of migrating the core away from CMSObject and in that process I noticed that we are very inconsistent with the data that is returned from loadFormData(). There are cases where this is an array and cases where this is an object, both from the same method. Most code is also confused what it is supposed to be, array or object. This PR unifies this to an object and removes the need for CMSObject.
Testing Instructions
Test all the edit views of the extensions touched by this PR.
Codereview.
Expected result AFTER applying this Pull Request
Nothing should have changed.
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