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

[BD-32] Hooks demo #210

Closed
wants to merge 4 commits into from
Closed

[BD-32] Hooks demo #210

wants to merge 4 commits into from

Conversation

morenol
Copy link

@morenol morenol commented Mar 18, 2021

Description

PR exemplifying how to use Hooks Extension Framework on edx-plaform. Please, be aware that this is just an example, soon we will be publishing the final version.

Supporting information

Please, check out the OEP-50 where there's a deeper explanation of the project and the current version of Hooks Guide on the platform.

Testing instructions

Described in openedx/edx-django-utils#110

Other information

This PR depends on Hooks Extension Freamework: Tooling

@mariajgrimaldi mariajgrimaldi changed the title Lmm/hooks demo Hooks demo Mar 25, 2021
@@ -522,6 +526,16 @@ def post(self, request):
if response:
return response

try:
trigger_filter("openedx.lms.auth.pre_register.filter.v1", data)
Copy link
Member

Choose a reason for hiding this comment

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

it should be data = trigger_filter("openedx.lms.auth.pre_register.filter.v1", data) right?

Copy link
Author

Choose a reason for hiding this comment

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

We can do something like this.

out = trigger_filter(..)
data = out.get('data')

But in any case, I think that the changes to 'data' in the functions are already being stored in the data dictionary passed as parameter

Copy link
Member

@felipemontoya felipemontoya Mar 25, 2021

Choose a reason for hiding this comment

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

Allright, out = trigger_filter(..) makes more sense.

Now, about the data being pass as reference, this could work, but we should not count on this. A filter could replace the dict with a new one and we would by mistake ignore that.

@felipemontoya felipemontoya changed the title Hooks demo [BD-32] Hooks demo Apr 18, 2021
@morenol morenol closed this May 19, 2021
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.

3 participants