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] Media Manager Events correctly triggered #28886

Merged
merged 3 commits into from
May 24, 2020

Conversation

dgrammatiko
Copy link
Contributor

@dgrammatiko dgrammatiko commented Apr 30, 2020

Pull Request for Issues #28877 #28878

Summary of Changes

Refactor the Model to trigger the correct Events with the correct payload for files/folders created/updated/deleted

Testing Instructions

Download install and enable the following plugin
mediamanagertest.zip

Test steps:

  • Upload a file in the media manager
  • Delete the file you just uploaded
  • Edit the image you just uploaded, eg crop it
  • Create a folder
  • Delete the newly created folder

Check plugins/content/mediamanagertest for a file log.txt the contents should be like

File uploaded
-------------------------
Event Type: onContentBeforeSave
Event Payload: {"adapter":"local-0","name":"Screenshot 2020-04-29 at 19.22.58.png","path":"\/","new":true}
-------------------------

-------------------------
Event Type: onContentAfterSave
Event Payload: {"adapter":"local-0","name":"Screenshot 2020-04-29 at 19.22.58.png","path":"\/","new":true}
-------------------------

File updated
-------------------------
Event Type: onContentBeforeSave
Event Payload: {"adapter":"local-0","name":"screenshot 2020-04-29 at 19.22.58.png","path":"\/","new":false}
-------------------------

-------------------------
Event Type: onContentAfterSave
Event Payload: {"adapter":"local-0","name":"screenshot 2020-04-29 at 19.22.58.png","path":"\/","new":false}
-------------------------

File Deleted
-------------------------
Event Type: onContentBeforeDelete
Event Payload: {"data":{"adapter":"local-0","path":"\/screenshot.png"}}
-------------------------

-------------------------
Event Type: onContentAfterDelete
Event Payload: {"data":{"adapter":"local-0","path":"\/screenshot.png"}}
-------------------------

Folder Created
-------------------------
Event Type: onContentBeforeSave
Event Payload: {"adapter":"local-0","name":"test","path":"\/","new":true}
-------------------------

-------------------------
Event Type: onContentAfterSave
Event Payload: {"adapter":"local-0","name":"test","path":"\/","new":true}
-------------------------

Folder Deleted
-------------------------
Event Type: onContentBeforeDelete
Event Payload: {"data":{"adapter":"local-0","path":"\/test"}}
-------------------------

-------------------------
Event Type: onContentAfterDelete
Event Payload: {"data":{"adapter":"local-0","path":"\/test"}}
-------------------------

Expected result

Actual result

Documentation Changes Required

No, bug fix

Remarks:

  • Moving/copying files won't trigger any event. This is kinda awkward...

@laoneo @dneukirchen

@SharkyKZ
Copy link
Contributor

Now Media Action - Resize plugin doesn't work.

@dgrammatiko
Copy link
Contributor Author

Good catch, I guess 45165a5 should fix that

@infograf768
Copy link
Member

Is that also spposed to solve #28842

@dgrammatiko
Copy link
Contributor Author

@infograf768 partly.

Also, internal server error when saving name of file or folder.

This should be ok now. Reason was that one of the current events had a wrong signature (expected to be called with 4 params but was called with 3)


return $object->name;
return $name;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure but we returned here the name attribute of the object because plugins could change it during the event.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I'll change that

@infograf768
Copy link
Member

Ref: #28886 (comment) & #28842 (comment)
I still get the Internal Server Error when saving the file with the same name. It's OK when modifying the name of the file.

@dgrammatiko
Copy link
Contributor Author

I still get the Internal Server Error when saving the file with the same name

Sorry I misunderstood, I was talking about renaming to something different.

@infograf768
Copy link
Member

Still wrong.
change_image

@adj9
Copy link

adj9 commented May 2, 2020

I have not tested this item.

@dgrammatiko I installed the plugin and function. Subsequently I looked for the log.txt file that you describe, but I did not find it .... I only have the indication of User Action Log. Where is the file?


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

@dgrammatiko
Copy link
Contributor Author

dgrammatiko commented May 2, 2020

I looked for the log.txt

it is created in the plugin's folder: plugins/content/mediamanagertest

@adj9
Copy link

adj9 commented May 16, 2020

Apply PR it all worked out. Going to Latest actions (from home dashboard) and clicking on the name of the uploaded image, I have error 0
image

@dgrammatiko
Copy link
Contributor Author

@adj9 I think that's ok as the plugin is a dummy implementation to showcase that the events are triggered correctly, in other words it doesn't actually do what it was supposed to do

@alikon
Copy link
Contributor

alikon commented May 16, 2020

so it's only a problem related to the com_actionlogs ?

@dgrammatiko
Copy link
Contributor Author

@alikon do components require special code to trigger anything for the com_actionlogs?
If so then someone needs to patch the media manager to support that but that's out of the scope of this PR

@alikon
Copy link
Contributor

alikon commented May 23, 2020

I have tested this item ✅ successfully on 5b27cf6


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

1 similar comment
@Quy
Copy link
Contributor

Quy commented May 24, 2020

I have tested this item ✅ successfully on 5b27cf6


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

@Quy
Copy link
Contributor

Quy commented May 24, 2020

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 24, 2020
@wilsonge wilsonge merged commit 4c8de56 into joomla:4.0-dev May 24, 2020
@wilsonge
Copy link
Contributor

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 24, 2020
@wilsonge wilsonge added this to the Joomla 4.0 milestone May 24, 2020
@dgrammatiko dgrammatiko deleted the patch-2 branch May 25, 2020 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants