[6.0] Unify loadFormData() to return an object

[Hackwar]

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:

• [X] No documentation changes for docs.joomla.org needed

• [ ] Pull Request link for manual.joomla.org:

• [X] No documentation changes for manual.joomla.org needed

[Fedik]

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.

[Hackwar]

@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.

[Fedik]

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): https://github.com/joomla/joomla-cms/blob/8e2814ab24146f37577141b97fccd3bc092ba51a/libraries/src/MVC/Model/FormBehaviorTrait.php#L133-L136

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).

[richard67]

Look, this looks not good (and other similar places):

https://github.com/joomla/joomla-cms/blob/8e2814ab24146f37577141b97fccd3bc092ba51a/libraries/src/MVC/Model/FormBehaviorTrait.php#L133-L136

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.

[Hackwar]

4 out of 43 models in our system return an array instead of an object.

[Bodge-IT]

OK, but code is not a democracy, can we # this out in Maintainers?

[LadySolveig]

Test all the edit views of the extensions touched by this PR. OR - Codereview.

Please provide a testing instruction that a "normal" user can follow or find someone to do this for you. Aditional Codereview is fine, but replaces only in special cases a human test.

[Fedik]

One more thing in the PR. Method $app->getUserState('foobar.data') does not guarantee the value to be an object, it still going to be mix depending when the data were set.

Example for default FormController it will be array https://github.com/joomla/joomla-cms/blob/552ee5751cae7bf97e11286854a466e34a1dcb54/libraries/src/MVC/Controller/FormController.php#L691

What I trying to say, this is not as simple as it seems by just looking getFormData().

[HLeithner]

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