Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0.] Remove obsolete and outdated trigger #24941

Merged
merged 3 commits into from May 19, 2019

Conversation

Projects
None yet
5 participants
@roland-d
Copy link
Contributor

commented May 18, 2019

Summary of Changes

This removes the outdated and obsolete trigger. The correct trigger happens the line above.

Testing Instructions

  1. Go to Systems
  2. Go to Plugins
  3. Open plugin Action Log - Joomla
  4. Click on Save
  5. Notice you get the following error:

image

6. Apply the patch 7. Reload the page 8. Click on `Save` 9. Plugin is now saved

Expected result

Plugin is saved

Actual result

Class 'JEventDispatcher' not found error

Documentation Changes Required

None

Remove obsolete and outdated trigger
Signed-off-by: Roland Dalmulder <contact@rolandd.com>
@richard67

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

I have tested this item successfully on f2d08b1

Note: The testing instructions only tell 1 example when that error happens. It happens also with other actions like e.g. enable/disable plugin with button in list view. This PR fixes all of them.


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

@@ -1320,9 +1320,6 @@ public function checkIn($pk = null)
);
$this->getDispatcher()->dispatch('onTableAfterCheckin', $event);
$dispatcher = \JEventDispatcher::getInstance();

This comment has been minimized.

Copy link
@alikon

alikon May 18, 2019

Contributor

i don't think it is correct, in this way you don't track anymore the onAfterCheckin event
should be coverted in j4 style

Factory::getApplication()->triggerEvent('onAfterCheckin', array($this->_tbl));

This comment has been minimized.

Copy link
@richard67

richard67 May 18, 2019

Contributor

Yes, that works, too, I've just tested. Error also disappears when replacing lines 1323 and 1324 in the unpatched file by the line which @alikon suggests. So I think he is right.

This comment has been minimized.

Copy link
@alikon

alikon May 18, 2019

Contributor

maybe the confusion arise cause there are 2 event :

  1. onAfterCheckin used by actionlog
  2. onTableAfterCheckin unused by core
@richard67

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

@roland-d I think @alikon is right: Instead of removing lines 1323 and 1324, you should replace those 2 lines by the one line
Factory::getApplication()->triggerEvent('onAfterCheckin', array($this->_tbl));
like he suggests.

@richard67

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

I have not tested this item.


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

Use the correct trigger
Signed-off-by: Roland Dalmulder <contact@rolandd.com>
@roland-d

This comment has been minimized.

Copy link
Contributor Author

commented May 18, 2019

@alikon There is definitely a confusion with 2 triggers with basically the same name. What is the difference between them? One is J4 and the other is for J3?

@richard67 I have added the trigger as suggested

@richard67

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

Would be confusing me if the was no confusion in J4 ;-) (joke)

@richard67

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

I have tested this item successfully on 485f985

Note: The testing instructions only tell 1 example when that error happens. It happens also with other actions like e.g. enable/disable any plugin with the button in the plugins list view. This PR fixes all of them.


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

@alikon

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

@alikon

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

I have tested this item successfully on 485f985


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

@alikon

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC label May 18, 2019

@infograf768

This comment has been minimized.

Copy link
Member

commented May 19, 2019

FYI, there are other places where the error happens: for example when playing with custom admin menus. PR also solves issue there.

@infograf768 infograf768 merged commit 6152423 into joomla:4.0-dev May 19, 2019

3 checks passed

Hound No violations found. Woof!
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr Build is passing
Details

@joomla-cms-bot joomla-cms-bot removed the RTC label May 19, 2019

@infograf768

This comment has been minimized.

Copy link
Member

commented May 19, 2019

Thanks.

@infograf768 infograf768 added this to the Joomla 4.0 milestone May 19, 2019

@roland-d roland-d deleted the roland-d:feature/remove-obsolete-trigger branch May 19, 2019

@roland-d

This comment has been minimized.

Copy link
Contributor Author

commented May 19, 2019

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.