Skip to content

Replace Hooks with EventDispatcher Part 3#14853

Merged
nupplaphil merged 51 commits intofriendica:developfrom
Art4:eventdispatcher-part3
May 13, 2025
Merged

Replace Hooks with EventDispatcher Part 3#14853
nupplaphil merged 51 commits intofriendica:developfrom
Art4:eventdispatcher-part3

Conversation

@Art4
Copy link

@Art4 Art4 commented Mar 13, 2025

This is a follow up PR of #14799 and replaces more calls of Hook::callAll() with the new EventDispatcher.

This PR replaces the call of Hook::callAll() with the EventDispatcher in 59 places and 27 files. (31 places in 21 files are left)

@Art4 Art4 mentioned this pull request Mar 13, 2025
64 tasks
@Art4
Copy link
Author

Art4 commented Apr 11, 2025

This PR is ready for review before it becomes too big. I replaced the call of Hook::callAll() with the EventDispatcher in 59 places and 27 files. (31 places in 21 files are left)

I will fix the code style in a separate commit to make the review easier.

@Art4 Art4 marked this pull request as ready for review April 11, 2025 08:30
@nupplaphil nupplaphil added this to the 2025.02 milestone May 3, 2025
@nupplaphil
Copy link
Collaborator

@Art4 Did you test different addons if they're still working with this refactoring?

@nupplaphil
Copy link
Collaborator

And moving the content into the $hook_data array and retrieving from there - is this compatible to all addons?!

Comment on lines +143 to +145
* An addon indicates successful login by setting 'authenticated' to non-zero value and returning a user record
* Addons should never set 'authenticated' except to indicate success - as hooks may be chained
* and later addons should not interfere with an earlier one that succeeded.
Copy link
Collaborator

Choose a reason for hiding this comment

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

marking for "addon policies" ?

@Art4
Copy link
Author

Art4 commented May 9, 2025

@Art4 Did you test different addons if they're still working with this refactoring?

No, I didn't, because I'm not familiar with all the addons and I cannot effort the time trying to understand how to run them to test a specific hook. Instead I've written tests for every hook, making sure the new events will map the old hook names and parameter syntax.

And moving the content into the $hook_data array and retrieving from there - is this compatible to all addons?!

Yes, thanks to HookEventBridge we have a complete list of all Events mapping to the old hook names. There you can also see if an event is handled special to convert the data to the "old hook style" to make it compatible.

@nupplaphil
Copy link
Collaborator

@Art4 - Tried to help you (php-cs & merge dev-branch) - but I think I bricked something

Please just rebase this branch and drop my 3 commits in case

Sorry ^^

@Art4 Art4 force-pushed the eventdispatcher-part3 branch from 7821bf3 to ea17201 Compare May 12, 2025 14:33
@Art4 Art4 marked this pull request as draft May 12, 2025 14:35
@Art4 Art4 marked this pull request as ready for review May 13, 2025 06:19
@Art4
Copy link
Author

Art4 commented May 13, 2025

@nupplaphil I merged the develop branch and fixed the code style.

@nupplaphil nupplaphil merged commit 3481c3a into friendica:develop May 13, 2025
31 checks passed
@Art4 Art4 deleted the eventdispatcher-part3 branch May 13, 2025 06:32
@AlfredSK
Copy link

AlfredSK commented May 13, 2025

After updating the node with this PR I get an exception and error 500 when visiting the profile page (clicking on the home/house icon) of my user. There's a type error. I'll open an issue...

@AlfredSK
Copy link

Issue: #14930

@Art4
Copy link
Author

Art4 commented May 13, 2025

Issue: #14930

Thank you. I've created a bug fix in #14932.

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.

3 participants