Updating base configuration for tutor-discovery

[BbrSofiane]

I’ve setup the publisher MFE in my development instance of tutor to see if I can use it to make it easier to create new course runs for existing courses. I’m a bit a stuck using the MFE BUT, this post is to suggest a couple of additions to the tutor-discovery plugin default configuration.

1. Add MFEs to CORS_ORIGIN_WHITELIST for discovery

• For production settings is as simple as adding the MFE_HOST
• For development settings, we would need to either add all of the MFEs or just the ones that need the discovery service. The latter requires a more work figure out how to identify those so I would lean towards adding all MFEs for development.

2. Add values to studio-url and organization-api-url when creating the partner.

• studio-url is required to publish new course runs.
• organization-api-url is required to edit organizations and publish the changes to the LMS.

What are your thoughts on this @maintainers? I’m happy to create the PR.

[regis]

Thanks for the proposal @BbrSofiane!

@BbrSofiane

For production settings is as simple as adding the MFE_HOST

Yes, exactly. Just implement the “discovery-production-settings” patch.

@BbrSofiane

For development settings, we would need to either add all of the MFEs or just the ones that need the discovery service. The latter requires a more work figure out how to identify those so I would lean towards adding all MFEs for development.

Hmmm I would tend to disagree. The reason is that most people want to avoid running the discovery service, when possible, for a lighter Open edX platform. This means that we need to know which MFEs actually require the discovery service.

I suggest that you only add the services that you need to the discovery CORS_ORIGIN_WHITELIST in development mode. When we face a similar issue with other MFEs, we can replicate the same patch.

Well, actually, it seems to me that we already know which services depend on the Discovery service, as indicated by the value of CORS_ORIGIN_WHITELIST in dev mode: course-discovery/course_discovery/settings/devstack.py at 6bc78aafee84306db2f9d136a14205d271ee89ec · openedx/course-discovery · GitHub

CORS_ORIGIN_WHITELIST = (
    'localhost:8734',   # frontend-app-learner-portal-enterprise
    'localhost:18400',  # frontend-app-publisher
    'localhost:18450',  # frontend-app-support-tools
    'localhost:2000',   # frontend-app-learning
)

As you noticed, the publisher MFE is there, but also the learning MFE. This dependency should thus be indicated in the tutor-mfe README in Maple.

@BbrSofiane

Add values to studio-url and organization-api-url when creating the partner.

Isn’t this already the case? tutor-discovery/tutordiscovery/templates/discovery/hooks/discovery/init at 89f2dc9ed566953d19cc706191aa6e71cfd94a54 · overhangio/tutor-discovery · GitHub Or do we need to create an extra partner for the MFE? If yes, the publisher MFE should implement the discovery/init hook.

Do you think that the Publisher MFE should be included in the tutor-mfe plugin? In Lilac, Maple or both? If the publisher MFE is not required, it should be installed as a separate plugin or with specific installation instructions in the tutor-mfe plugin. If we must create a separate tutor-publisher plugin, who will maintain it? I can take care of it, but if you would rather maintain it I’ll happily link to it from the Tutor docs.

[BbrSofiane]

I added the publisher MFE by adding an entry to config file. My expectation was that by adding an MFE_APP and the discovery-plugin, I should be able to have simple MFE that relies on the discovery service working. Assuming the MFE doesn’t require any complex configuration.

Obviously, expectations can be set. As it is, any MFE that relies on the discovery service will need to be implemented outside of tutor-mfe because they will need to override CORS_ORIGIN_WHITELIST.

That could be addressed by adding logic around CORS_ORIGIN_WHITELIST in course-discovery to make the service available for MFE_APPs. In production it’s as simple as CORS_ORIGIN_WHITELIST = ("{{ MFE_HOST }}").

So that as a developer I don’t have to implement an MFE and a tutor-plugin unless I really need it.

@regis

Hmmm I would tend to disagree. The reason is that most people want to avoid running the discovery service, when possible, for a lighter Open edX platform. This means that we need to know which MFEs actually require the discovery service.

In this scenario, the tutor user should know that they need the discovery service.

@regis

Isn’t this already the case? tutor-discovery/init at 89f2dc9ed566953d19cc706191aa6e71cfd94a54 · overhangio/tutor-discovery · GitHub Or do we need to create an extra partner for the MFE? If yes, the publisher MFE should implement the discovery/init hook.

You’re right for organization-api-url! I think we could add studio-url, I don’t see any downside if it makes the discovery more “ready” to use out of the box.

@regis

Do you think that the Publisher MFE should be included in the tutor-mfe plugin?

I don’t think we can use the full feature set of publisher without a Salesforce account. So on that basis I would say no, it should not be included by default. I think some of the features also rely on ecommerce… so yeah

If I can get it working and it works well for my use case (create new course runs for new customers), I’m happy to maintain a plugin.

[regis]

@BbrSofiane

Obviously, expectations can be set. As it is, any MFE that relies on the discovery service will need to be implemented outside of tutor-mfe because they will need to override CORS_ORIGIN_WHITELIST.

That could be addressed by adding logic around CORS_ORIGIN_WHITELIST in course-discovery to make the service available for MFE_APPs. In production it’s as simple as CORS_ORIGIN_WHITELIST = ("{{ MFE_HOST }}").

So that as a developer I don’t have to implement an MFE and a tutor-plugin unless I really need it.

I’m afraid that only the most simple MFEs will work out of the box with the base configuration from the tutor-mfe configuration. Others (such as the publisher MFE) will require more complex settings, and thus following the generic installation instructions will simply not be enough.

Basically, MFEs can fall in one of three categories:

1. MFEs that are enabled by default are added to tutor-mfe and may have complex settings.
2. Simple MFEs that can be enabled by setting a *_MFE_APP variable and don’t even need specific documentation can be installed without a plugin.
3. Other MFEs will need a dedicated plugin; but note that in most cases, a simple yaml plugin should be enough. For instance, in the case of the publisher MFE:

name: publisher
version: 12.0.0
config:
  defaults:
    MFE_APP:
      name: publisher
      repository: "https://github.com/edx/frontend-app-publisher"
      port: 18400
patches:
  discovery-development-settings: |
    CORS_ORIGIN_WHITELIST = list(CORS_ORIGIN_WHITELIST) + ["{{ MFE_HOST }}:{{ PUBLISHER_MFE_APP['port'] }}"]

@BbrSofiane

You’re right for organization-api-url! I think we could add studio-url, I don’t see any downside if it makes the discovery more “ready” to use out of the box.

@BbrSofiane

If I can get it working and it works well for my use case (create new course runs for new customers), I’m happy to maintain a plugin.

Considering the requirement on a Salesforce account, I agree this would be the best solution, yes.

[BbrSofiane]

@regis

Basically, MFEs can fall in one of three categories:

1. MFEs that are enabled by default are added to tutor-mfe and may have complex settings.
2. Simple MFEs that can be enabled by setting a *_MFE_APP variable and don’t even need specific documentation can be installed without a plugin.
3. Other MFEs will need a dedicated plugin; but note that in most cases, a simple yaml plugin should be enough. For instance, in the case of the publisher MFE:

That makes perfect sense.

@regis

Simple MFEs that can be enabled by setting a *_MFE_APP variable and don’t even need specific documentation can be installed without a plugin.

Just to clarify, this is only valid if the MFE only uses edx-platform endpoints. Is that part of what you mean when you say that they don’t need specific documentation?

@regis

name: publisher
version: 12.0.0
config:
  defaults:
    MFE_APP:
      name: publisher
      repository: "https://github.com/edx/frontend-app-publisher"
      port: 18400
patches:
  discovery-development-settings: |
    CORS_ORIGIN_WHITELIST = list(CORS_ORIGIN_WHITELIST) + ["{{ MFE_HOST }}:{{ PUBLISHER_MFE_APP['port'] }}"]

Here we would also need to add

 discovery-production-settings: |
    CORS_ORIGIN_WHITELIST = list(CORS_ORIGIN_WHITELIST) + ["{{ MFE_HOST }}"]

[regis]

@BbrSofiane

Just to clarify, this is only valid if the MFE only uses edx-platform endpoints. Is that part of what you mean when you say that they don’t need specific documentation?

Yes, exactly.

@BbrSofiane

Here we would also need to add

 discovery-production-settings: |
    CORS_ORIGIN_WHITELIST = list(CORS_ORIGIN_WHITELIST) + ["{{ MFE_HOST }}"]

Correct – but with the upcoming arrival of the learning MFE in the default installation (and thus in the tutor-mfe plugin), I would be in favour of adding this patch directly to tutor-mfe.