akeeba / com_datacompliance

A tool to help you work towards EU GDPR compliance for your Joomla!™ site
GNU General Public License v3.0
14 stars 5 forks source link

Attempt to assign property "datacompliance" on array #42

Closed Ruud68 closed 1 year ago

Ruud68 commented 1 year ago

Hi, Just ran into this error using this component / plugin. The issue is in the line below, where $data is an array and not an object.

https://github.com/akeeba/com_datacompliance/blob/5fc6543e55eaac61e4735e6cd96255990b14cba5/plugins/user/datacompliance/src/Extension/DataCompliance.php#L202

When I change the code to do a check and when data is array do $data['datacompliance'] = ['hasconsent' => $hasConsent ? Text::('JYES') : Text::('JNO'),];

then the error is gone, but as I do not know how you extension works I do not know if it still works correct.

here you do a check on $data if it is an array: https://github.com/akeeba/com_datacompliance/blob/5fc6543e55eaac61e4735e6cd96255990b14cba5/plugins/user/datacompliance/src/Extension/DataCompliance.php#L153

nikosdion commented 1 year ago

Thank you for this! Apparently some installations may pass $data as an array. Hopefully all this malarkey will be addressed when Joomla moves to concrete, statically typed events sometime within the the 4.x or 5.x lifetime.

Ruud68 commented 1 year ago

Hi @nikosdion , I think that when no data is passed (because there is nothing to pass), it defaults to an array. In debugging this (backtrace) the component data comes from this:

    /**
     * Method to get the data that should be injected in the form.
     *
     * @return  array  The default data is an empty array.
     *
     * @since   1.6
     */
    protected function loadFormData()

so the error is thrown because data = [] and not an object.

nikosdion commented 1 year ago

And that's why the core moving to concrete events will ultimately help catch this kind of inconsistent data types.

Form data in Joomla 1.6/1.7/2.5 was indeed array. I don't recall exactly when it became on object, but I seem to believe this was sometime around 3.x. Except for this default case, as you said.

Inconsistencies like that drive me off the wall. That's why I introduced the foundation work for migrating to concrete events without screwing up b/c. Now the core maintainers have started working on concrete events for the core events so, yay, there's hope :)