docs: add an ADR for footer links

[Cup0fCoffee]

Description

This PR introduces an ADR for adding links to the footer component.

References

#311
BB-7755

[openedx-webhooks]

Thanks for the pull request, @Cup0fCoffee! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[xitij2000]

I think there are a few more questions that this ADR can answer, and clarification it can give.

[Cup0fCoffee]

@xitij2000 Based on your feedback, it seems like using the existing branding API is the way. We don't need custom links, so there is not need in merging the two approaches.

[xitij2000]

Looking good! Just some small feedback.
I think parts of the ADR should be updated to focus the language more from a general community aspect than the current implementation we'd like to work on.

[codecov]

Codecov Report

Patch and project coverage have no change.

Comparison is base (fb869bc) 81.25% compared to head (e8f74f5) 81.25%.
Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #326   +/-   ##
=======================================
  Coverage   81.25%   81.25%           
=======================================
  Files           4        4           
  Lines          32       32           
  Branches        4        4           
=======================================
  Hits           26       26           
  Misses          6        6           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[arbrandes]

First off, thanks a lot for this effort! As you'll see below I have reservations, but none trump the fact that the end goal is a desirable one, and there's no better way to address that than with an ADR (or with the PR that instigated the creation of the ADR).

Let me start by saying that I'm with you in wanting to make the footer configurable in a way that doesn't overburden operators. Which is to say that I understand all the arguments for the proposed approach over, say, the MFE config API. Putting on my architect hat, though, this doesn't look like the ideal way to make MFE configuration more practical, understandable, and usable.

1. We'd be (ab)using a legacy API that wasn't written with MFEs, React, or any of that in mind, and thus perpetuating a design decision that is already on its way out. (Plus, inserting a non-React piece of arbitrary HTML anywhere in a React app doesn't sound great.)

2. It represents yet another distinct request to the server with all the possible UX issues this entails, where it could (in theory) be avoided by reusing mfe_config.

3. It's yet another separate place in the platform operators have to go to to customize the frontend. As Adam put it elsewhere, it's complicated enough having the layout be controlled not via React props but via Python-served JSON (aka mfe_config). Having it in two (or more) separate places, using different paradigms... ain't great.

4. This API would only be used by the footer component, with no saving grace from being used across the board for customization.

I could go on, but I'm sure you get where I'm going with this. Can I humbly ask that you take a look at what would it take to use MFE config instead, even if it requires making it properly localizable? (I'm sure y'all can see how just having i18n would be a great architectural addition!)

Disclaimer: I'm not saying MFE config is ideal for something as basic as footer elements, either. I'm actually with @adamstankiewicz 's first instinct to have this (or some of this) be configurable with externally hosted stylesheets or some other more static/scalable method (such as with Piral!). But none of those possibilities are mature enough, yet, and I'd rather get this bit of configurability in sooner rather than later.

C/C @abdullahwaheed

[arbrandes]

Suggested change

	style the footer component. That would eliminating some of the frequent reasons
	style the footer component. That would eliminate some of the frequent reasons

[xitij2000]

First off, thanks a lot for this effort! As you'll see below I have reservations, but none trump the fact that the end goal is a desirable one, and there's no better way to address that than with an ADR (or with the PR that instigated the creation of the ADR).

Let me start by saying that I'm with you in wanting to make the footer configurable in a way that doesn't overburden operators. Which is to say that I understand all the arguments for the proposed approach over, say, the MFE config API. Putting on my architect hat, though, this doesn't look like the ideal way to make MFE configuration more practical, understandable, and usable.

1. We'd be (ab)using a legacy API that wasn't written with MFEs, React, or any of that in mind, and thus perpetuating a design decision that is already on its way out.  (Plus, inserting a non-React piece of arbitrary HTML anywhere in a React app doesn't sound great.)

2. It represents yet another distinct request to the server with all the possible UX issues this entails, where it could (in theory) be avoided by reusing mfe_config.

3. It's yet another separate place in the platform operators have to go to to customize the frontend.  As Adam put it elsewhere, it's complicated enough having the layout be controlled not via React props but via Python-served JSON (aka mfe_config).  Having it in two (or more) separate places, using different paradigms... ain't great.

4. This API would only be used by the footer component, with no saving grace from being used across the board for customization.

I could go on, but I'm sure you get where I'm going with this. Can I humbly ask that you take a look at what would it take to use MFE config instead, even if it requires making it properly localizable? (I'm sure y'all can see how just having i18n would be a great architectural addition!)

Disclaimer: I'm not saying MFE config is ideal for something as basic as footer elements, either. I'm actually with @adamstankiewicz 's first instinct to have this (or some of this) be configurable with externally hosted stylesheets or some other more static/scalable method (such as with Piral!). But none of those possibilities are mature enough, yet, and I'd rather get this bit of configurability in sooner rather than later.

C/C @abdullahwaheed

Since I recommended looking at the existing footer API here, I think my main reasoning was:

• For instances that have already customised it, it will just work.
• The customisation will work across the core platform and MFEs.
• Translations are easier since you can have different language versions supplied by the backend, vs having to do the same in the frontend. AFAIK right now there isn't a mechanism for that. One way would be to configure all translations in the backend I guess.

I think using the MFE config API is also what @Cup0fCoffee was leaning towards initially.

[arbrandes]

Yup, the reasoning from an operator's perspective (in particular, an operator with existing deployments) makes total sense, and we should keep that top of mind. But I feel we should weigh it against future operators and code maintainers as well.

IIRC a "v2 API" as a future alternative was mentioned elsewhere, which is what makes me think we should probably bring it about sooner rather than later, as a part of this work. Just as a lone example, architecturally speaking I would prefer to have us modify the legacy frontend to use a theoretical new API (an internationalized frontend config API?) than to have the MFEs use an old one.

By the way, I realize this might mean biting off more (way more) than you originally intended. But if you don't mind humoring me, let's keep the conversation going. I think we can arrive at something that doesn't have as many downsides and also doesn't break anybody's bank.

I'd like to hear thoughts from folks such as @regisb, @dcoa, @felipemontoya, and others in BTR as likely users of this as well.

[regisb]

If I understand correctly, the core of the issue is that we don't have a way to easily extend MFEs. If we did, then it would be super easy to customise the footer links. Would you say this is a fair statement? Is this what we are actually talking about? (sorry if I'm not on the same page)

Given our past experiences with edx-platform and tutor, I think we can all agree that the "right" way to extend MFEs (or any given app for that matter) is not to add new settings for every single use case.

My point of view (and I think at least @felipemontoya will be on my side here ;)) is that the most robust way of extending an app is with actions and filters. We are in luck because this mechanism is already in place in edx-platform.

In the case of MFEs, there is just one little issue, which is that they don't share the same runtime as edx-platform. (sidenote: decoupling from the monolith was actually the whole purpose of MFEs in the first place)

I see two ways of addressing this issue:

1. Access the filters & actions API via a web API. (Permissions might be tricky)
2. Import some js from edx-platform which will natively call the filters & actions API, somehow. (This will strengthen even more the coupling between MFEs and edx-platform)

[arbrandes]

A couple more thoughts after looking at this a litle closer:

First, I didn't realize that the proposed API endpoint also splits out JSON. This makes it much less problematic than I initially figured. Scratch my objection on the "inserting pre-rendered HTML into a React app" grounds.

However, to second @regisb point, I'm still concerned about adding an API call just for the footer configuration. I mean, even the runtime MFE configuration mechanism is itself not without controversy: having to hit the backend for configuration that could otherwise be happily served statically is not an option in all deployment environments, so we have to be judicious in requiring its use. In other words, even if we go with this option for now, it can't be the only way to get a customized footer.

Which gets us to the Holy Grail of having a generic, scalable extensibility mechanism for MFEs, which, like Regis, I believe to be the real solution, here. I'm not sure filters are the right mechanism (literally not sure - haven't looked into them very closely), but I do think this is a perfect use case for some method of extensibility.

@davidjoy and 2U have been looking into iframe-based plugins, but, again, not sure that would be the ideal fix for this situation.

[xitij2000]

@regisb I think this case is slightly different since this is about the footer component which is designed to be replaceable, so it's entirely possible for us to create an alternate footer that works the way we'd like (including using the branding API or config API). However, we felt that having an upstream approach to make minor customisations to the footer would be a good idea.

I think here, the question is to either:

• use the existing mechanism for customising footer links and apply it to MFEs
• use the existing mechanism for customising MFEs and apply it to footer links
• use a new mechanism

On a general note, since we're now moving towards content customisation as opposed to branding, actions and filters sound like a better model for sure. They might not apply to this case because I feel creating a plugin for a plugin to change these links would be an overkill for smaller instances. Bigger instances might just replace the footer.

I'm not sure if I understand the mechanism you're proposing, though. Do you mean that the actions and filters that apply to the frontend will also be defined in the backend and will be exposed to the frontend via some actions and filters API? Would these essentially be JavaScript functions that the frontend will load and inject in the right places?

I assumed that the model used for backend actions and filters could inform a similar mechanism in the frontend. There is already some concept of actions via events like APP_AUTH_INITIALIZED, APP_CONFIG_INITIALIZED etc. For filters, I think React components can fit well in that. i.e. you can wrap one React component in another to extend it, or entirely replace it for something new.

@bradenmacdonald had proposed the "swizzling" mechanism from docasaurus for this, which I think can cover most use-cases. It proposes two mechanisms, ejecting a component so you can fully customise and replace it, and wrapping one component in another so you can enhance it. In that case you could wrap the Unit component to add a sidebar for instance, and eject the footer component to fully replace it.

However, that deals fully with build-time configuration not run-time, although with a mechanism similar to module fedaration perhaps it could be both.

@arbrandes re: another API call for just the footer configuration. I think it is just the footer configuration for now, but it can definitely be expanded to do more things. It's the branding API, so I think there is scope for it to do much more.

Perhaps instead of a branding API v2, we could instead look at a config API v2 (or just extensions to the existing API) that can handle things like translations etc. For example, we can add a filter to the config API that will allow plugins to translate the response of that API based on values they know are translatable.

One approach worth considering is server-side rendering. If we can do that, the footer API, and even the MFE config API can be called during server-side rendering and the frontend can avoid those additional calls unless something changes.

The iframe-based plugin approach sounds good, but might not work for this use case. Long back when that PR was initially created I had played around with that approach a bit and got it working with federated components as well. That would allow injecting components at run-time inside slots instead of iframes, so they could be passed rich props, and could dispatch events. I prototyped this for making the discussion sidebar a federated component that could be a loaded by the learning MFE (or potentially used as a comment component elsewhere). I don't know if that approach is still under consideration.

[felipemontoya]

Very interesting discussion. Thanks for tagging me @arbrandes.

Re: using filters as the way to extend an app. Most certainly I agree, but I don't see a way to use the current openedx-filters at the front end. We can extend the APIs that the frontend calls so that you can alter the frontend component and add a plugin and get around having to fork edx-platform, but we can't run filters in the frontend yet. I quite like the "swizzling" approach. I need to investigate more on that to see if we could make that work on runtime (via federation or piral or something else).

Re: hitting two APIs to get the configuration we could easily add a filter that allows us to programatically extend the MFE-config API and for instance automatically (provided that the filter has an implemented step to do so) add the content of the branding API as one of the keys of the MFE-config. We could even do this without a filter and add a branding key that includes footer information and can grow to include the header and other components that are themed often. We were thinking about doing this in our own swapped footer and header components so I applaud that you are trying to do this upstream.

Finally, I'm curious about how would this work in a world where the piral framework has landed. Would we still be needing to have a base footer component that kind of works for most installations? Does piral make it so easy to swap a component in runtime that this problem will completely disappear?

[arbrandes]

@felipemontoya,

Does piral make it so easy to swap a component in runtime that this problem will completely disappear?

This is not Piral-exclusive (meaning, we could do it without Piral), but yes, it would give us a mechanism to federate whatever module we feel warrants it. In this case, you could have multiple versions of just the footer component built - one for each tenant in your deployment - and then the correct one would get pulled in by the user's browser as needed.

But, as @xitij2000 put it, this is not the easiest possible way out, particularly for small deployments. After all, replacing the footer component is tantamount to forking it. Even if you federate it out, you'll still want to rebase your footer code every release. (Exactly like Comprehensive Theming). Configuration helps alleviate this.

On the other (third?) hand, as @regisb put it, using configuration can only take you so far without becoming a nightmare. With every new toggle, you add to the burden of documentation, testing, and maintenance. So it's not all sunshine and rainbows, either for developers or deployers.

I don't think we need to make a decision for the whole project, here and now. (Spoiler alert, we'll soon be having a frontend-wide extensibility discussion, during which this issue will be brought up.) But this issue is pretty symbolic. Do we add more configuration? Do we wait for a more comprehensive customization framework? Both?

[Cup0fCoffee]

Thank you all for the comments - it has been insightful and interesting to read.

I think for now we are ok with not upstreaming the changes and maintaining a fork for ourselves.

We are looking forward to the changes in the MFEs, and given the chance, will be happy to contribute to making them more customizable and extendable.

[openedx-webhooks]

@Cup0fCoffee Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

[openedx-webhooks]

@Cup0fCoffee Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.