docs: [FC-0074] centralize docs for the hooks extension framework #599
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @mariajgrimaldi! What's next?Please work through the following steps to get your changes ready for engineering review: 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. 🔘 Let us know that your PR is ready for review:Who will review my changes?This repository is currently maintained by Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:
When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
mariajgrimaldi
force-pushed
the
MJG/hooks
branch
from
5f59e21 to
9510dc8
Compare
mariajgrimaldi
changed the title
| Use an Open edX Filter when but not limited to: | ||
|
|
||
| - Enrich the data or parameters passed to a specific component, e.g., fetch reusable LTI configurations from external plugins. | ||
| - Intercept and modify the input of a specific component, e.g., include "Edit" link to an HTML block if certain conditions are met. |
There was a problem hiding this comment.
I'm not sure this is the best example of modifying a component. What are some others?
There was a problem hiding this comment.
All these examples were extracted from a list of real-life use cases we've compiled: https://github.com/mariajgrimaldi/openedx-events-filters-analysis/blob/main/docs/Extensions_built_using_Hooks_Extensions_Framework.pdf. Since I'm writing the use-case list, that is just a filtered version of that document, I'll get some examples from there instead.
There was a problem hiding this comment.
You can review the changes here: c953566
|
@sarina, thank you for all the suggestions! I hope to finish addressing your review by tomorrow :) |
There was a problem hiding this comment.
This looks great! Before merging, can you add this document to the "What's New" section on the dev home? https://docsopenedxorg--599.org.readthedocs.build/en/599/developers/index.html
|
@sarina, I left a comment here for you to review #599 (comment). Thank you! |
| - Enforce specific constraints or business rules of a specific component, e.g., don't allow registration for non-authorized email domains. | ||
| - Implement additional features or behavior in a specific component, e.g., add registration extra fields to the user registration form. | ||
|
|
||
| .. For more detailed use cases, see the TODO: ADD REFERENCE TO REAL LIFE USE CASES documentation. |
There was a problem hiding this comment.
This is good to merge I guess after the real life examples docs are merged!
There was a problem hiding this comment.
@mariajgrimaldi before I merge, will you plan to address this in a different PR?
There was a problem hiding this comment.
I was going to, but I can quickly do it right now. I'll let you know once it's done.
There was a problem hiding this comment.
From the technical standpoint this PR is correct and looks good.
I have a minor concer about the reference to customers as the driving force behind developers. I don't think we should include that assumption even if right now it looks familiar for companies such as edunext, opencraft or RG.
|
@felipemontoya: thank you for the review! Can you explain your concern a bit further? I don't quite understand what you mean. |
| What is the Hooks Extension Framework? | ||
| ************************************** | ||
|
|
||
| Based on the :doc:`edx-platform:concepts/extension_points`, this framework aims to extend the Open edX platform in a maintainable way without modifying its core. The main goal is to leverage the existing extension capabilities provided by the `Open edX Django plugins`_ architecture, allowing developers to implement new features to fit customer needs while reducing the need for core modifications and minimizing maintenance efforts. |
There was a problem hiding this comment.
| Based on the :doc:`edx-platform:concepts/extension_points`, this framework aims to extend the Open edX platform in a maintainable way without modifying its core. The main goal is to leverage the existing extension capabilities provided by the `Open edX Django plugins`_ architecture, allowing developers to implement new features to fit customer needs while reducing the need for core modifications and minimizing maintenance efforts. | |
| Based on the :doc:`edx-platform:concepts/extension_points`, this framework aims to extend the Open edX platform in a maintainable way without modifying its core. The main goal is to leverage the existing extension capabilities provided by the `Open edX Django plugins`_ architecture, allowing developers to implement new features while reducing the need for core modifications and minimizing maintenance efforts. |
There was a problem hiding this comment.
@felipemontoya: would this be enough to address your concern?
|
@felipemontoya: I believe I've addressed your comment with this commit, thank you! |
|
@sarina: thank you so much! Merge when you see fit :) |
|
Should we put this in the "What's New" section on https://docsopenedxorg--599.org.readthedocs.build/en/599/developers/index.html ? And/or in the "Concepts" card? I think this is an important document to highlight! |
|
@sarina, yes! I'm on it :) |
source/developers/index.rst
Outdated
| @@ -24,6 +24,7 @@ Open edX Developers | |||
|
|
|||
| * :doc:`references/running_pr_tests` | |||
| * :doc:`references/internal_data_formats/index` | |||
| * :doc: `contents/hooks_extension_framework` | |||
There was a problem hiding this comment.
| * :doc: `contents/hooks_extension_framework` | |
| * :doc:`contents/hooks_extension_framework` |
There was a problem hiding this comment.
Yes, I'm realizing I did this all wrong. I'll let you know once it's done.
source/developers/index.rst
Outdated
| @@ -46,6 +47,7 @@ Open edX Developers | |||
| * :doc:`concepts/platform_overview` | |||
| * :doc:`concepts/backend_layout_and_approach` | |||
| * :doc:`concepts/accessibility` | |||
| * :doc: `contents/hooks_extension_framework` | |||
There was a problem hiding this comment.
| * :doc: `contents/hooks_extension_framework` | |
| * :doc:`contents/hooks_extension_framework` |
|
@sarina: it's finally done! Thank you so much for all the help :) |
|
🎉 |
Description
This PR adds documentation about the Hooks Extension Framework under the developers section to centralize common concepts between Open edX Events and filters, such as framework overview, differences, comparisons, etc.
You can see the docs rendered here: https://docsopenedxorg--599.org.readthedocs.build/en/599/developers/concepts/index.html
The documentation in https://github.com/openedx/edx-platform/tree/master/docs/hooks should be removed after this gets merged. What this docs is missing from edx-platform guides, it's the index of filters and events within edx-platform:
https://github.com/openedx/edx-platform/blob/master/docs/hooks/filters.rst#index-of-filters
https://github.com/openedx/edx-platform/blob/master/docs/hooks/events.rst#index-of-events
This PR will not replace those indexes since they need extra effort to keep up to date, making the contribution process more cumbersome since we will need to update them here each time a new event/filter is added to a service. I prefer enhancing the current list of events and filters in each repo to reference in some way where they're being used instead.