-
Notifications
You must be signed in to change notification settings - Fork 1
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
Pluggable Upgrade Messaging #332
Comments
Thanks for your submission, @openedx/open-edx-project-managers will review shortly. |
@zacharis278 thank you for submitting this. We think the best person to review and provide feedback is @arbrandes , who is on holiday until next week. I'll circle back then. |
@arbrandes something that came up out of further digging investigation. It seems the the near term approach to plugins actually involves committing them to the learning MFE under the plugins folder: https://github.com/openedx/frontend-app-learning/tree/master/plugins. Is this acceptable location for a 2U specific experiment? Can components in this folder be changed without a product review? Or, would we need to do the additional work in figuring out how to include this proposed plugin as a separate repository? cc: @alangsto |
@zacharis278, having some plugins live in the Learning MFE codebase might make sense if:
As you can imagine, these won't always apply. Even if they do, though, ultimately it should be up to the organization writing the plugins to decide where they should live (such as in a mono-repo). As for the single plugin that is in there, it was just a part of the proof-of-concept for the corresponding feature. I expect it to be removed as soon as its primary user (2U) figures out a way to add it to their deployment pipeline from a separate repository, as it fails the second condition above. So, to answer your question explicitly: no, it is not meant to be an acceptable location for experiments, or for org-specific code. |
Thanks for submitting, @zacharis278 . Very glad to see this project. I just want to confirm I'm understanding the implications from a product perspective. cc @arbrandes I've been a bit confused about how the upgrade messaging widget is built to begin with. The widget itself is called the "Notifications" widget on the off-the-shelf community release (see below), when it appears in the right sidebar of the Learning MFE. Is it that edx is simply leveraging this Notifications widget for a custom "notification" to learners about upselling? Does this proposal aim to make that entire "Notifications" widget pluggable, or is it just targeted at the edx.org custom messaging about upselling within that Notifications space? From a Core Product perspective, the edX upsell widget is not part of the Core Product. I'm not sure how the Notifications app is designed, and whether it's possible to generalize it for any notification use case. I'd rather see a completely pluggable environment in that right sidebar space, that could be used to customize 1) any notifications app, which gives us leeway to develop more general apps driven by learner progress data down the road, and 2) the discussions app. |
@jmakowski1123 Our original intent was narrowly targeted at making just content inside existing upgrade widget pluggable. However, I think a more general approach of allowing us to plug in a custom widget into the existing notifications bar would be a valuable compromise. We can always build our own custom clone of the existing component and modify that. It's a bit more work, but doesn't blow up our scope. If agreeable I can update the proposal to reflect that. There's a few things to look into further on if the existing component would need to be configurable to not conflict with pluggable ones that serve the same purpose. I'll have to do some research and return back with answers to that question. |
@arbrandes Thank you for the context. I'll check in with the 2U team that owns that plugin to see if we can sync up on how to add plugins as a separate repo in the future. |
Thanks so much, @zacharis278 . The way I was envisioning it, we'd actually just have a slot in the right sidebar space for any notifications app. Is that what you mean when you say "plug in a custom widget into the existing notifications bar"? Maybe we're really saying the same thing :) Just an fyi, I've flagged Brian Mesick to take a look at this as well, from a data pipeline perspective. |
@jmakowski1123 do you have any thoughts on the other proposed plugin slots to replace/customize the content lock message for FBE, or is that good to go? What else do we need process wise to get the go ahead to merge PRs related to these plugin locations? |
@jmakowski1123 @arbrandes - heads up that the team is starting to build this plugin as described and amended in this ticket to align with internal timelines, so any additional feedback would be more valuable and actionable sooner than later. My read of this ticket and discussion is that we're fairly aligned - is that correct? |
@jmakowski1123 @arbrandes the work to add these extension points is now queued up as openedx/frontend-app-learning#1368. Are we able to move forward with getting this reviewed and merged? |
This is good from my end, as long as it's good with @arbrandes . Thanks for amending to accommodate the plugin approach. |
I've reviewed the first PR. The gist of it is that it'll need to be refactored so that it does in fact create a generic notification plugin slot, as discussed above. A corollary of this is that the upgrade notification itself will have to be removed from the codebase (or moved into a plugin). |
Abstract
As the operator of an Open edX site, 2U would like to experiment with different approaches to the platform's display of upgrade messaging in the learning MFE. We believe that increased partner branding and placing emphasis on certificates will increase our conversion rates.
Context & Background
Right now we don't have a good method to ship experimental or temporary code without forking the Open edX repo. Forking comes with some long term costs that don't make sense to take on for such a limited feature. We also foresee making continued changes to this component in a way that would likely not make sense to contribute back into an Open edX release. Creating appropriate extension points in the UI would allow operators like 2U to customize these select components without relying on a fork.
Scope & Approach
To support customization of the upgrade experience on Open edX this project would add extension points to alter or replace two components in the learning MFE: the upgrade messaging in the sidebar (figure A), and the message on content locked by feature based enrollment (figure B).
The project would make use of the frontend-plugin-framework to implement the two required plugin slots. This would require adding and configuring the framework within the learning MFE as well creating the plugin slots as extension points. The use of these extension points would be optional, the default experience will be unchanged without further configuration by the owner of the site.
For further details on the supported features of the plugin framework and how configuration works see: https://github.com/openedx/frontend-plugin-framework/blob/master/README.rst
figure A
figure B
Value & Impact
Greater flexibility of upgrade messaging for operators of Open edX sites. As a long term benefit this would also add the configuration needed to add further plugin extension points in the future.
Milestones and/or Epics
This work would likely be composed of only 1-2 PRs meeting the following requirements:
The frontend-plugin-framework is installed and configured in frontend-app-learningThis is no longer needed since it has been added with feat: whole course translations frontend-app-learning#1330upgrade sidebar widgetnotification sidebar component such that custom notification widgets may be added.Named Release
Redwood
Timeline
The engineering work to alter any Open edX repositories is minimal. If approved, our goal would be to have a PR with these proposed changes by the end of April.
The development of the 2U upgrade component would continue beyond that date.
Proposed By
2U
Additional Info
No response
The text was updated successfully, but these errors were encountered: