feat: Modified course_home_url for externally hosted courses

[irfanuddinahmad]

Description

This PR conditionally modifies the course_home_url for externally hosted courses based on the COURSE_HOME_URL_OVERRIDES_FOR_EXTERNAL_COURSES django setting.

The URL will flow downstream to both the B2B and B2C learner dashboards. B2B enterprise-course-enrollments API gets the course_home_url via another B2C helper function get_course_run_url in edx-platform (source) whereas the B2C learner home init API calls course_home_url directly (source).

[adamstankiewicz]

Left a couple suggestions. Also, [curious] any way we could benchmark the performance impacts of these changes, i.e. the call to course-discovery per enrollment?

[clarification] If we make these changes here, how does it relate to the changes being proposed by ENT-7163, which is adding to the cache via a management command? Will the proposed management command caching via ENT-7163 be applied to the cache utilized via get_course_data this PR?

[kdmccormick]

Hi folks. Both the keys and defaults values of these new settings are 2U-specific and thus cannot be added to edx-platform.

An alternative could be to create COURSE_HOME_URL filter hook, and then in a 2U plugin, use that hook to modify the course home URL based on the 2U-specific logic.

[feanil]

@jmbowman @openedx/arch-bom tagging you as well since this is the kind of thing we don't want landing in the platform and maybe the team making this change needs some support to build this correctly in a plugin using the filters system of the Hooks Extension Framework.

[robrap]

@kdmccormick: Thanks for the alternative approach.
@feanil: Thanks. We’ll discuss education to ensure everyone is clear.

[adamstankiewicz]

Hi folks. Both the keys and defaults values of these new settings are 2U-specific and thus cannot be added to edx-platform.

An alternative could be to create COURSE_HOME_URL filter hook, and then in a 2U plugin, use that hook to modify the course home URL based on the 2U-specific logic.

If we opted for a setting like COURSE_HOME_URL_OVERRIDES_BY_COURSE_TYPE_AND_PRODUCT_SOURCE shown above, the default setting committed to code could be set as an empty list, or provide an example with "fake" values in the setting, so there would be nothing 2U-specific in code itself. That said, I don't love that proposed solution either; we're definitely open to other approaches here.

Thanks for the alternative suggestion about the filters system within the hooks extensions framework. I don't think the teams working on the integration of bringing GetSmarter Executive Education courses from 2U into the edx.org platform are really aware of the hooks extension framework, myself included.

@irfanuddinahmad @muhammad-ammar Let's learn more about the hooks extensions framework (from linked OEP) and any supporting docs/examples we can find, and see how we might be able to use this hooks extension framework here.

[adamstankiewicz]

FWIW, It might be helpful if there were more discoverable documentation for openedx-filters. As it stands right now, I'm only finding the high-level OEP, high-level ADR decisions, a handful of links to prior PR examples, and this repo of sample filters (which doesn't really help explain how to use these filters in practice).

As a newcomer to this package, it's personally a bit unclear how to get started or understand its practical use. For example, the majority of the RST files in docs folder in the repo are empty, the README doesn't mention any useful information on how to build a new filter, the link to Read the Docs returns a 404, etc. It took some digging to find, but this doc has been the most helpful thus far for me to better understand how to work with openedx-filters more practically: https://github.com/openedx/edx-platform/blob/master/docs/guides/hooks/filters.rst. Can we get a guide similar to this one co-located within openedx-filters so it's easier to find?

@kdmccormick @feanil Also worth noting (if you're not already aware), there's (unfortunately) other existing, committed code / default settings related to 2U/GetSmarter's Executive Education courses... a sample:

• https://github.com/openedx/frontend-app-publisher/blob/9e59252663197a3a31a7b23714b065a42576da5c/src/data/constants/index.js#L23
• https://github.com/openedx/ecommerce/blob/3bae03045b358125341f934e12f0d2c86f29a325/ecommerce/extensions/urls.py#L20
• https://github.com/openedx/course-discovery/blob/20673a2a05d4fde40115103796c6876395fb00ef/course_discovery/settings/base.py#L675
• https://github.com/openedx/course-discovery/blob/20673a2a05d4fde40115103796c6876395fb00ef/course_discovery/apps/course_metadata/models.py#L604

[kdmccormick]

On the surface, COURSE_HOME_URL_OVERRIDES_BY_COURSE_TYPE_AND_PRODUCT_SOURCE does not contain the words "2U" or "ExecEd", which is good, but can you folks see how it is still coding edX-specific business needs into the core? The fact that the setting name is ten words long indicates that it does not really belong as a core setting. This is precisely the type of use case that openedx-filters was designed for; please try it out.

Also worth noting (if you're not already aware), there's (unfortunately) other existing, committed code / default settings related to 2U/GetSmarter's Executive Education courses...

@adamstankiewicz Thanks for those samples. We're aware that Discovery & Publisher have edX-specific logic, and we're currently in the process of deciding whether it'd better to clean those repos up or just remove them from the Open edX project.

Along those same lines: We should not be introducing further Discovery-specific concepts into edx-platform, either. product_source falls in this category. Up until now, this has been violated in a few parts of the platform, but we are beginning to push back towards LMS having no knowledge of Discovery.

[kdmccormick]

FWIW, It might be helpful if there were more discoverable documentation for openedx-filters. As it stands right now, I'm only finding the high-level OEP, high-level ADR decisions, a handful of links to prior PR examples, and this repo of sample filters (which doesn't really help explain how to use these filters in practice).

As a newcomer to this package, it's personally a bit unclear how to get started or understand its practical use. For example, the majority of the RST files in docs folder in the repo are empty, the README doesn't mention any useful information on how to build a new filter, the link to Read the Docs returns a 404, etc. It took some digging to find, but this doc has been the most helpful thus far for me to better understand how to work with openedx-filters more practically: https://github.com/openedx/edx-platform/blob/master/docs/guides/hooks/filters.rst. Can we get a guide similar to this one co-located within openedx-filters so it's easier to find?

Thanks for the feedback @adamstankiewicz . I'll bring this to the team and we'll see how we can get these docs into a better state.

I expect that the first few hook plugins that 2U write will take a bit of time, but as we iterate on the docs and gain shared experience on them hook plugins, writing hook plugins will feel as easy as the old Core Code + Django Setting approach, and eventually even easier! Sorry that you folks are the unwitting early adopters @irfanuddinahmad and @adamstankiewicz ; we appreciate you working with us 😄

[johnnagro]

@kdmccormick meanwhile we're trying to hit a deadline here, do you have any estimation of when we'd be able to see better documentation and examples? we're trying to make an assessment of the way forward here.

[macdiesel]

Kyle, we are looking to release a feature to a client on June 13th. These new requirements feel pretty late in the game for us. Additionally there are no paved paths for us to use, there is no documentation on how to do so, and no one has done it before. All we get is a link to an ADR from 2 years ago?

This doesn't set us up for success. How am I supposed to deliver this feature on time now? I'm all for improving our separation of concerns, but at what cost and when? Could we at least have some paved roads first?

[robrap]

Personally, I'd love to see Marketplace/Enterprise concerns moved to their own system where your teams have full control, and you don't feel like anyone is slowing you down. In that world, all courses would be "externally hosted", so the name of this setting would no longer make sense. I've started a draft Google Doc proposal around that, but it needs more discussion and work.

Note that my proposal is not going to directly solve this question of how to land this change and meet a June 13th deadline. That said, if we were all in alignment of where we want to go and how we plan to get there, then it would shine a different light on any final bits of tech debt as we transition and learn how to move in an entirely different direction.

[kdmccormick]

@macdiesel Keeping core Open edX code organization-agnostic isn't a new requirement. I agree that the documentation can be improved; we're prioritizing that now. That said, the OEP is two years old because the library has been around for two years and was featured at two Open edX conferences in a row.

We'd like to unblock you so you can hit your deadline. @ormsbee has an idea that would skip openedx-filters for now, although we would eventually move this solution into openedx-filters. I'll let him elaborate.

[ormsbee]

@macdiesel: What if we make a new, optional setting for COURSE_EXPERIENCE_COURSE_HOME_URL, and set it to the path of the function you want to execute there. So by default it's nothing, and the course_home_url does it's normal thing. But if you set:

COURSE_EXPERIENCE_COURSE_HOME_URL = "edx_arch_experiments.course_experience.course_home_url"

Then the first part of the course_home_url just proxies out and returns the results of that function instead of doing its normal one:

from django.utils.module_loading import import_string

def course_home_url(course_key):
    custom_course_home_url_fn_path = getattr(settings, 'COURSE_EXPERIENCE_COURSE_HOME_URL')
    if custom_course_home_url_fn_path:
        custom_fn = import_string(custom_course_home_url_fn_path)
        custom_url = custom_fn(course_key)
        if custom_url:
            return custom_url

    # normal control flow here

You could implement the function in any plugin that's currently installed for edx.org's deployment–I just picked edx_arch_experiments because I know it's there. It would be ugly because you'd be importing from edx-platform for get_course_data, but it should work without having to do much coding or ramp-up. We can have followup discussion on the openedx-filters side on docs and how to make this particular use case simpler (patching a function rather than necessarily invoking a pipeline), but I think this would allow you to meet your deadlines while keeping 2U concerns separate.

Please let me know what you think. Take care.

[robrap]

@ormsbee: I don't know enough about openedx-filters. Could a filter solution be implemented inside edx-enterprise, or would the Enterprise team need a plugin for edx-platform for both this short term solution and the long term (filter) solution?

If Enterprise only needs the plugin for the short term solution and will agree to a deadline for removing it from edx-arch-experiments, that is a potential home. See edx/edx-arch-experiments#206 for more details. However, if they will need a plugin that they manage permanently, then it may make sense to simply create it.

[ormsbee]

@robrap: I think it's fine to put either the custom home URL function or some future filter implementation in edx-enterprise. I broadly agree with your long term vision of decoupling enterprise concerns from platform, and in that process I would expect much of what is edx-enterprise today to be factored out into 2U-specific-and-owned code.

[robrap]

Thanks @ormsbee.

1. Your request is pretty simple if it doesn’t require a new plugin.
2. It sounds like you’d like to call the new setting DOA (deprecated on arrival), so it doesn’t become a permanent part of the api? That would just entail proper docs, and potentially a commitment to refactor into the filter solution in the future?

[adamstankiewicz]

Personally, I'd love to see Marketplace/Enterprise concerns moved to their own system where your teams have full control, and you don't feel like anyone is slowing you down. In that world, all courses would be "externally hosted", so the name of this setting would no longer make sense. I've started a draft Google Doc proposal around that, but it needs more discussion and work.

@robrap While I agree the "split edx-enterprise out of the platform" is a conversation worth having, I don't feel it's as relevant to this particular use case. The solution we were hoping to solve for here is to support both the B2C (home.edx.org) and B2B learner dashboards (enterprise.edx.org). Treating edx-enterprise as its own service does not solve for supporting enrollments from external courses on the B2C learner dashboard.

I broadly agree with your long term vision of decoupling enterprise concerns from platform, and in that process I would expect much of what is edx-enterprise today to be factored out into 2U-specific-and-owned code.

@ormsbee Does Axim have a sense for whether or how the enterprise stuff in the Open edX platform gets used by the community? The question of whether enterprise is officially part of the Open edX platform has always been fuzzy, and no one ever seems to have a great answer. How can we make an informed decision about what the Open edX community desires/needs from an enterprise perspective? Without knowing these sorts of requirements/needs for Enterprise within the community, it will be incredibly difficult to make good decisions around how to best split up edx-enterprise (and all other Enterprise services/MFEs) into what's 2U and what's Open edX.

---

@robrap @kdmccormick @ormsbee Keep in mind, too, it's not just the course home url we need different based on whether an enrollment is for an externally hosted course; it's also basic metadata such as start and end dates, and upgrade_deadline too, given the decision made (outside of our Enterprise teams, with little input from our engineering squads AFAIK) to have the externally course data conform to a different schema than what a course typically looks like in the Open edX platform (i.e., putting the metadata about dates, availability, etc. in Course.additional_metadata rather than transforming the externally hosted course data to conform to the existing Open edX course schema).

Many of the problems around the metadata we're facing now like the aforementioned dates wouldn't be an issue had we been conforming to the existing Open edX course schema for the externally hosted courses. Instead, we're now in a world where we're needing to introduce downstream business logic in many different services (both Learning Platform and Enterprise) to conditionally differentiate between Open edX hosted courses and externally hosted courses, which as we're correctly calling out here is not really open-source friendly.

[ormsbee]

@robrap:

2. It sounds like you’d like to call the new setting DOA (deprecated on arrival), so it doesn’t become a permanent part of the api? That would just entail proper docs, and potentially a commitment to refactor into the filter solution in the future?

I don't know if it'd be DOA. It might turn out to be useful enough where we keep it as proposed in the long term, or we might decide to fold it into something else–but the deliberation process might take a while. In the meanwhile, we'd have something that isn't too coupled, even if the scheduled cleanup/refactoring were delayed for some reason. So yeah, if the proposal is acceptable to the team, let's call it experimental in the comments to signal to folks that they maybe want to hold off on using it themselves unless they like living on the edge.

@adamstankiewicz:

@ormsbee Does Axim have a sense for whether or how the enterprise stuff in the Open edX platform gets used by the community?

I don't have a good sense of this. I know that there is the occasional forum post asking for more information about it, or trying it out. My impression is that there are a number of features in there that community members would be interested in, but that it's too coupled with edX-specific logic in places to make it easy to use. But while I'm aware of the very obvious surface-level coupling, like having 2U specific fields in the models and config, I don't really have a good sense for what the deeper business logic coupling is there, so please take my opinion with a grain of salt.

Maybe it's worth making a forum post to see who replies?

The question of whether enterprise is officially part of the Open edX platform has always been fuzzy, and no one ever seems to have a great answer. How can we make an informed decision about what the Open edX community desires/needs from an enterprise perspective? Without knowing these sorts of requirements/needs for Enterprise within the community, it will be incredibly difficult to make good decisions around how to best split up edx-enterprise (and all other Enterprise services/MFEs) into what's 2U and what's Open edX.

Yes, it's definitely been murky. Determining whether or not enterprise features (or which features) should be part of the core product offering is the responsibility of the Product Working Group. I know the work is ongoing, but I don't know when to expect a decision there. I suspect that when the decision does come, it's not going to be a simple yes/no, but instead something more like "These eight things that edx-enterprise does should be considered Core Product, and these other fourteen things are not," or something to that effect. That's just my speculation though–@jmakowski1123 and @kdmccormick could both speak to it better than I can.

In the absence of that guidance, I also don't know what's worth making generic vs. 2U specific within edx-enterprise itself. In my purely personal opinion (i.e. I'm NOT speaking on behalf of Axim), if I were 2U, I would focus most heavily on making sure new edx-enterprise features are decoupled from edx-platform, and not worry as much about making sure they are generically reusable. So for example, I'm okay with a setting to override the course_home_url function, where the implementation of that function in edx-enterprise is very 2U-specific. I would prefer that over having edx-platform's course_home_url function directly import a enterprise_course_home_url_override function that's in edx-enterprise and then making that generically configurable. Does that make sense?

Many of the problems around the metadata we're facing now like the aforementioned dates wouldn't be an issue had we been conforming to the existing Open edX course schema for the externally hosted courses. Instead, we're now in a world where we're needing to introduce downstream business logic in many different services (both Learning Platform and Enterprise) to conditionally differentiate between Open edX hosted courses and externally hosted courses, which as we're correctly calling out here is not really open-source friendly.

Okay, so I admit that one of my first questions when seeing this code was, "Why we are in the LMS at all if it's an external course?", but I didn't want to open up that can of worms because I assumed there was a good reason for it that involved a long trail of tradeoffs and dependencies. That being said, while having a customizable course_home_url seems pretty reasonable (multiple folks have implemented alternative courseware frontends for instance), I am concerned if we're going to need to add a bunch of stuff downstream to manipulate dates and scheduling. I don't want to hold up this particular issue over it, but I am very interested in understanding more about these issues.