[5.4] Deprecate app property in FieldsPlugin

[heelc29]

Summary of Changes

deprecate $app property in FieldsPlugin

Testing Instructions

only deprecation > code review
check fields plugin still working

Link to documentations

Please select:

• No documentation changes for docs.joomla.org needed
• Pull Request link for manual.joomla.org: Add deprecation note for joomla/joomla-cms#45695 Manual#488

[ceford]

I have tested this item ✅ successfully on 7b55457

Just a thought: would it be better to use a local $language variable:

$language = $app->getLanguage()

and then use that in the loops. Otherwise, this looks OK to me.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45695.}

[laoneo]

I have tested this item ✅ successfully on 7b55457

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45695.}

[richard67]

@heelc29 Is there a reason why this PR is still draft?

[heelc29]

@heelc29 Is there a reason why this PR is still draft?

Since I discovered an use of the variable in the core and updated the testing instructions and also have to update the baseline file.

[brianteeman]

we need to do better than this. Instead of adding an exception why not fix it. I was taught by @nibra that when we deprecate something we should stop using it in the core at the same time.

[richard67]

we need to do better than this. Instead of adding an exception why not fix it. I was taught by @nibra that when we deprecate something we should stop using it in the core at the same time.

@brianteeman In general that's right, but here we need the exclusion due to the b/c code:

$app   = $this->getApplication() ?: $this->app;

The ?: $this->app part has to be kept for b/c until Joomla 7, and this part causes the PHPstan complaint.

[laoneo]

But then it would be better to add a phpstan ignore on that line.

[richard67]

But then it would be better to add a phpstan ignore on that line.

@laoneo I don't know. Maybe yes. As it is a relative new thing that we do not ignore PHPstan in CI, we don't have much practice with that yet. Up to now we were using the baseline file to ignore errors and as a task list.

I think we should discuss that and find a common way in the maintainers team.

[brianteeman]

Shouldn't there be some documentation explaining why this is being deprecated

[heelc29]

Shouldn't there be some documentation explaining why this is being deprecated

In future it is no longer supported by CMSPlugin #38060

joomla-cms/libraries/src/Plugin/CMSPlugin.php

Line 163 in 108662f

@trigger_error('The application should be injected through setApplication() and requested through getApplication().', E_USER_DEPRECATED);

[brianteeman]

that isnt a why? and its not in the accompanying manual entry

[richard67]

RTC as the previous human tests are still valid. There was only a change on the PHPstan baseline file after that and a clean branch update.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45695.}

[richard67]

To me the joomla/Manual#488 documentation PR looks complete.

[muhme]

✅ Final test before merge, JBT graft full package

• Enabled Log Almost Everything
• Created custom field 'color' and featured article and tested in administrator backend and site frontent
• Created one more article, a field group and fields user, url and usergroup and assigned to the group
  • All three fields are available and working in adminstrator backend
  • User field is missing in site frontend edit view, but is also missing before this PR -> opened Missing group list user field in site frontend editing – ok? #45874 for clarification
• No entries in administrator/logs/*

[muhme]

Thank you @heelc29 for your contribution. Thank you @brianteeman for support. Thank you @ceford and @laoneo for testing.