? Pending

User tests: Successful: Unsuccessful:

avatar Fedik
Fedik
1 Jul 2022

Pull Request for Issue #38153 .

Summary of Changes

Make sure that the property $app exists, for plugins that strated migration to Service.
More info in comment #38153 (comment)

Testing Instructions

To mimic possible bug, edit

public function getCoreUpdateNotification(QuickIconsEvent $event)

Add there dump($this->app->getName());
And open Dashboard.

Actual result BEFORE applying this Pull Request

You will get error Call to a member function getName() on null

Expected result AFTER applying this Pull Request

You will get "administrator".

Documentation Changes Required

As part of #38060

ping @laoneo @nikosdion

avatar Fedik Fedik - open - 1 Jul 2022
avatar Fedik Fedik - change - 1 Jul 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 1 Jul 2022
Category Libraries
avatar Fedik Fedik - change - 1 Jul 2022
Labels Added: ?
avatar nikosdion
nikosdion - comment - 1 Jul 2022

The problem is that you get a notice if $app is not defined in the plugin. But defining $app protected would break plugins defining it private (even though they really shouldn't). If you declare it private it's as good as not being declared. You've reached an impasse.

I don't have a solution which does not break b/c in one way or another. Using setApplication is the kind of change that should have ever only made it in a major version because it has b/c implications any way you dice it. If you decide to keep it in a minor version something will break.

I have warned you about the implications, we have discussed it to oblivion, there's no solution. Everyone here is responsible for their own actions — including the passive-aggressive buffoons who just put thumbs down reactions to anything that tries not to completely break Joomla and alienate the ever-shrinking user base that's left.

In any case, this is not my monkey and not my circus. It is most definitely not my problem or a problem I really care about; my plugins won't suffer and even if they did I know how to make my plugins work.

avatar Fedik
Fedik - comment - 1 Jul 2022

yeah,

Another suggestion, we can do like this:
Leave it as it is (close this PR), and when after release of 4.2+ we will get reaports related to this, we can reopen it again.
Anyone against it? :)

avatar nikosdion
nikosdion - comment - 1 Jul 2022

I agree.

I would also say that we need to add a release note about the change in CMSPlugin behaviour, ideally in the developer site and linked to in the release announcement. It should be marked as potential backwards compatibility break.

The release note could even instruct developers who want to upgrade to the new application and database setters/getters to add a magic __get method if they have plugin view templates with $this->app / $this->db and reasonably expect their clients to have overridden them.

It would be a first for Joomla, warning developers about a potential b/c break and ways to mitigate it. At the very least it would set a good precedent of how to manage legacy without catching all interested parties off-guard 😄

avatar Fedik Fedik - change - 2 Jul 2022
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2022-07-02 08:56:45
Closed_By Fedik
avatar Fedik Fedik - close - 2 Jul 2022

Add a Comment

Login with GitHub to post a comment