feat: [FC-86] Added support for the new Catalog MFE#259
Conversation
|
Hi, isn't this a repeat of #257? |
|
@brian-smith-tcril I’d like to kindly suggest taking a look at this PR. We can merge it a bit later, once everything is ready. Would that work for you? |
brian-smith-tcril
left a comment
There was a problem hiding this comment.
Overall the changes here make sense, but I think we might need some minor changes to the logic.
I noticed
tutor-mfe/tutormfe/patches/openedx-lms-common-settings
Lines 11 to 13 in ca0665f
looks almost identical to
tutor-mfe/tutormfe/patches/openedx-lms-common-settings
Lines 8 to 10 in ca0665f
and
Lines 75 to 78 in ca0665f
is being added to CORE_MFE_APPS.
This makes me think that this PR is putting the catalog MFE in a "default on" state. For Ulmo, the goal is to allow site operators to switch from the legacy catalog to the Catalog MFE, but not default to it.
@arbrandes - do you know how we've handled this in the past?
|
@brian-smith-tcril, in the past the optionality of an MFE came down to either it being defined in an optional plugin (like Learner Record via tutor-credentials), or a feature/waffle flag. Because of the nature of the Catalog MFE (particularly, the fact it's the first thing anybody sees, also people can use it logged out), I think the best thing is to go via a plugin. |
Thanks! I have a question and would appreciate your thoughts. Since the Catalog MFE isn’t part of another service (like tutor-credentials) and we have This way, the Catalog MFE would be disabled by default but could still be enabled if needed. Please let me know if I’m misunderstanding anything. |
df25f8d to
3aee1e2
Compare
|
@regisb, any objections? Anybody else at Edly I should be asking? |
|
Going ahead and merging this so people can give early feedback. |
Description
This PR includes the configuration for the new Catalog MFE (Verawood release).
PR with an installation example for the Catalog MFE as a Tutor plugin - openedx/frontend-app-catalog#60
How Has This Been Tested?
Merge Checklist
Post-merge Checklist