Skip to content

[5.4] Fix deprecated direct property access of model state#45704

Merged
muhme merged 5 commits intojoomla:5.4-devfrom
heelc29:5.4/deprecations/modelstate
Aug 11, 2025
Merged

[5.4] Fix deprecated direct property access of model state#45704
muhme merged 5 commits intojoomla:5.4-devfrom
heelc29:5.4/deprecations/modelstate

Conversation

@heelc29
Copy link
Contributor

@heelc29 heelc29 commented Jul 7, 2025

Summary of Changes

Remove direct property access of model state
Change API model state also to State|Registery (missed in #39663)

Testing Instructions

Enable Log Deprecated API in Global Configuration
image

Visit different views

  • open existing banner for editing in backend
  • open existing banner for editing in backend (EDIT: banner client)
  • open existing contact for editing in backend
  • open existing article for editing in backend
  • open saved versions of banner/contact/article
  • open search terms (smart search) in backend
  • open existing newsfeed for editing in backend
  • open existing tag for editing in backend
  • open existing user note for editing in backend
  • open user contact form in frontend
  • open content archive view in frontend
  • open content category/categories view in frontend
  • open featured articles view in frontend
  • open archived articles view in frontend
  • open create article view in frontend
  • open privacy request/remind/confirm view in frontend
  • open tag/tags view in frontend
  • open user remind/reset view in frontend

Check log file logs/deprecated.php

Actual result BEFORE applying this Pull Request

direct property access of model state
deprecated Direct property access will not be supported in 7.0 in Joomla\CMS\MVC\Model\State::__get::Joomla\CMS\MVC\Model\State. - [ROOT]\libraries\src\MVC\Model\State.php - Line 94

Expected result AFTER applying this Pull Request

no direct property access of model state

Link to documentations

Please select:

  • No documentation changes for docs.joomla.org needed
  • Pull Request link for manual.joomla.org: TBD

@heelc29 heelc29 changed the title [5.4] Remove direct property access of model state [5.4] Fix deprecated direct property access of model state Jul 24, 2025
@heelc29 heelc29 marked this pull request as ready for review July 31, 2025 15:19
@exlemor
Copy link

exlemor commented Aug 2, 2025

I have tested this item ✅ successfully on cdeadcf

I have successfully tested this PR. Thanks @heelc29 for the hard work.

1 comment/inquiry (I apologize in advance if my inquiry is stupid: while the messages Direct property access will not be supported in 7.0 in Joomla\CMS\MVC\Model\State:: etc are no longer in the deprecated.php log :D, I am noticing one quite similar (I understand it is different) and out of doubt, I prefer mentioning it in case it mattered:

deprecated Instead of an empty value, the default value will be returned in 7.0 in Joomla\CMS\MVC\Model\State::get::Joomla\CMS\MVC\Model\State. - [ROOT]/libraries/src/MVC/Model/State.php - Line 56

(I am seeing a LOT of those as well)

Is that a side-effect of this PR, or completely irrelevant/unrelated? Sorry, again for the dumb question.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45704.

@heelc29
Copy link
Contributor Author

heelc29 commented Aug 5, 2025

deprecated Instead of an empty value, the default value will be returned in 7.0 in Joomla\CMS\MVC\Model\State::get::Joomla\CMS\MVC\Model\State. - [ROOT]/libraries/src/MVC/Model/State.php - Line 56

(I am seeing a LOT of those as well)

Is that a side-effect of this PR, or completely irrelevant/unrelated? Sorry, again for the dumb question.

@exlemor No worries at all - it's a valid question! In this case, it's not relevant to this PR, so no need to worry about it here.

But it looks like the code does not match the registry class. It compares against null and an empty string, while the deprecation message uses PHP’s empty() function, which also returns true for 0 (int/float) and empty arrays. @laoneo, should a PR be created?

public function get($path, $default = null)
{
if (isset($this->data->$path) && empty($this->data->$path)) {
@trigger_error(
\sprintf('Instead of an empty value, the default value will be returned in 7.0 in %s::%s.', __METHOD__, __CLASS__),
E_USER_DEPRECATED
);
return $this->data->$path;
}
return parent::get($path, $default);
}

https://github.com/joomla-framework/registry/blob/4452de9c66abfec59b535e27c62af3528a539311/src/Registry.php#L207-L209

@laoneo
Copy link
Member

laoneo commented Aug 5, 2025

If I 'm not wrong, then made it like that because of bc reasons.

@richard67
Copy link
Member

@heelc29 In your testing instructions you wrote:

  • open existing banner for editing in backend
  • open existing banner for editing in backend

I assume the 2nd one should be "banner client", is that right?

If so, could you update the instructions?

@exlemor Have you tested banner client, too?

@exlemor
Copy link

exlemor commented Aug 9, 2025

@heelc29 In your testing instructions you wrote:

  • open existing banner for editing in backend
  • open existing banner for editing in backend

I assume the 2nd one should be "banner client", is that right?

If so, could you update the instructions?

@exlemor Have you tested banner client, too?

Hello @richard67, I had followed instruction and stupid me I assumed it was a mistake (the double line) and I had not but I JUST did a second ago and validate that the message goes away with this PR which I just redownloaded as a Download Package so all good :).

@heelc29
Copy link
Contributor Author

heelc29 commented Aug 9, 2025

I assume the 2nd one should be "banner client", is that right?

Yes your right

@richard67
Copy link
Member

I have tested this item ✅ successfully on 9aa1341


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45704.

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45704.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 10, 2025
@muhme
Copy link
Contributor

muhme commented Aug 11, 2025

⚠️ Final test before merge, using JBT (which is disabling 'Behaviour - Backward Compatibility' by default)

  • Checked 'Direct property access will not be supported' are logged in administrator/logs/deprecated.php before this PR
  • Grafted Joomla_5.4.0-alpha4-dev+pr.45704-Development-Full_Package.zip, enabled 'Log Deprecated API', 'Log Almost Everything', 'Gather Search Statistics' and 'User Registration'
  • Installed 'Blog Sample Data'
  • Tested regarding test instructions
    • Privacy with Create/Extend/Confirm
    • Tags with Compact/All/Tagged Items and opened one tag from the list
  • All are successful, except one:
    • menu item type 'Archived Articles' (placed in Bottom menu) each frontend site click creates one entry:
      2025-08-11T05:13:50+00:00 WARNING 192.168.65.1 deprecated Direct property access will not be supported in 7.0 in Joomla\CMS\MVC\Model\State::__get::Joomla\CMS\MVC\Model\State. - [ROOT]/libraries/src/MVC/Model/State.php - Line 94
  • Additional tested all menu entry types

@muhme
Copy link
Contributor

muhme commented Aug 11, 2025

@heelc29 manual.joomla.org: TBD: is this PR purely internal refactoring with no user-facing effects, so no manual updates are needed?

@heelc29
Copy link
Contributor Author

heelc29 commented Aug 11, 2025

  • All are successful, except one:

    • menu item type 'Archived Articles' (placed in Bottom menu) each frontend site click creates one entry:
      2025-08-11T05:13:50+00:00 WARNING 192.168.65.1 deprecated Direct property access will not be supported in 7.0 in Joomla\CMS\MVC\Model\State::__get::Joomla\CMS\MVC\Model\State. - [ROOT]/libraries/src/MVC/Model/State.php - Line 94

its from #45841
image

@muhme Should I do a branch update and fix this one?

@muhme
Copy link
Contributor

muhme commented Aug 11, 2025

@heelc29 no, I will merge this one now, as the test effort is expensive, with a new PR we have only to test the one case, thank you

@muhme muhme merged commit 595008c into joomla:5.4-dev Aug 11, 2025
41 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 11, 2025
@muhme muhme added this to the Joomla! 5.4.0 milestone Aug 11, 2025
@muhme
Copy link
Contributor

muhme commented Aug 11, 2025

Thank you @heelc29 for your contribution. Thank you @laoneo for supporting this PR. Thank you @exlemor and @richard67 for testing.

@heelc29
Copy link
Contributor Author

heelc29 commented Aug 11, 2025

@heelc29 manual.joomla.org: TBD: is this PR purely internal refactoring with no user-facing effects, so no manual updates are needed?

maybe only for Change API model state also to State|Registery (missed in https://github.com/joomla/joomla-cms/pull/39663)??

@richard67
Copy link
Member

@heelc29 I've created PR #45889 with your suggestion. Could you test? That would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants