[#44148] - [6.0] Unify loadFormData() to return an object
- Pending
- Medium
- Build: 6.0-dev
- # 44148
- Diff
- Hackwar:6.0-cmsobject-stdclass
User tests: Successful: Unsuccessful:
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.
- OR -
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
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_banners com_categories com_config com_contact com_content com_fields com_finder com_installer com_languages com_menus com_messages com_modules com_newsfeeds com_plugins com_redirect com_tags com_templates |
| Labels |
Added:
PR-6.0-dev
|
||
yes, and no :)
It not about what our code expecting, but about how to make it "right".
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 if (empty($data))
The method loadFormData() were set to return array since begining of its existance. And that our Models return object (in some cases) it is bug in that models.
I do not know how to resolve all of these, but personally, I think in conext of Form it still should return array, as it more easy to work with, also it prevents the data "referencing" (because objects always pased by reference).
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
if (empty($data))
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.
I am sorry, this not going to work.
It is should be an opposite actually, and I think we talked about opposite.
The result of loadFormData() should be always array.