feat: multi-tenant — let a learner choose which Open edX platform to sign in to - #670
feat: multi-tenant — let a learner choose which Open edX platform to sign in to#670IvanStepanok wants to merge 5 commits into
Conversation
A build normally talks to exactly one site, named in config.yaml. With LMS_DIRECTORY enabled it instead shows the platforms listed in a JSON document, re-themes to whichever the learner picks — logo, accent colour, sign-in artwork — and signs in against that one. Logging out returns to the list, so a device can move between platforms without a new build. Off by default: with ENABLED false none of this runs. The document is the whole contract. It comes from a URL or from a file inside the app, and nothing downstream can tell which — so a build can ship its list and its images and never ask the network for either. Because the whole list arrives at once, every platform's sign-in background is known before anything is tapped, which is what makes it possible to warm that artwork while the learner is still choosing rather than showing a placeholder afterwards. Image fields carry either an http(s) address or the name of a file shipped with the app. One field, one rule: the alternative was a parallel set of *_asset fields and a precedence rule to go with them, which puts the rule in documentation instead of in the value. Theme.Images.headerBackground now holds a decoded image rather than a URL. The header used AsyncImage, which has no cache, so it refetched and flashed a placeholder on every appearance. Kingfisher does the fetching; Theme does not depend on it, because Core already depends on Theme. Documentation/LMS_DIRECTORY.md has the format, both delivery modes and a worked example.
openedx-webhooks
added
open-source-contribution
|
Thanks for the pull request, @IvanStepanok! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
…ader image Every file here now opens with the same five-line block the rest of the repository uses, and the explanation that was sitting in those blocks moved onto the declaration it describes, as a doc comment. One real fix came out of comparing the two platforms side by side. Theme.Images.headerBackground is a plain static, so a screen that had already drawn kept the stock artwork even after the selected platform's image arrived — visible on a cold start, where the picker's prefetch has not run. It now posts a change notification and the header view listens for it. Android, whose Coil fills the image in when it lands, was already right.
|
To test this MR you can create a JSON file by using my service: https://providers.openedx-lms.stepanok.com |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #670 +/- ##
==============================
==============================
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The mobile working group settled this the other way round from how the file was written: a multi-instance app has one public OAuth client id of its own, and each backend registers that id — the directory is not where per-platform credentials live. The runtime already worked that way: Config falls back to the app's own client id when no platform has been selected or the selected one names none. The wire format did not — `api` and `oauth_client_id` were required, so a document written to the agreed model failed to decode here while Android, whose fields are already nullable, read it fine. `api` is now optional in full, `host_url` defaults to the address the learner picked, and a missing client id means "the app's own". A platform that does name one still overrides, for that platform only.
The file the apps read now says `format`, `include`, `name`, `description`, `url` and `logo`, matching the schema being settled in the mobile working group rather than the column names of the registry that happened to produce the first one. A file written by hand and a file exported from a registry are now the same shape, which was the point. Two things fall out of the rename. `id` becomes optional — a platform that names none is identified by its address, which is unique within a directory anyway, so the smallest useful entry is a name and a URL. And the long `description` is gone: it was carried through the model and never shown, while the line the list actually draws was called `short_description`, so that one takes the plain name. `logo_upload_url` goes with it. There is one logo per platform; which image that is, is the publisher's decision, not something the reader should arbitrate.
|
Looks great: one UX piece of feedback: there's no good way to get back to the LMS site selection page if the selected site has pre-login discovery. If a learner accidentally clicks the wrong LMS that has pre-login discovery enabled, the learner has to click "sign in" and then click "change" in the selected LMS block in order to get back to the site selector. I'm logging minor notes as well, I'll share once I've finished testing |
The id came from the registry that produced the first document, where it is a primary key. It means nothing to a client, so the schema is better without it. What replaces it is the position in the file, not the URL. Two entries may legitimately name the same address — the same LMS listed twice under different branding — and my own live directory does exactly that. Identifying by URL merges them: the list drew the second entry in the first row, and opening it would have handed over the first entry's branding and OAuth client. That is a wrong platform, not a cosmetic glitch, and it only shows up on a directory that happens to contain a duplicate. Both platforms now number the entries as they read them, and a test on each side holds a two-entry document that shares one address.
|
@IvanStepanok Sorry for the slow turnaround here — this has been sitting longer than it should have. I've started digging into it and will post a full review today or tomorrow at the latest. Thanks for your patience! :) |
5 participants
What this adds
A build of this app normally talks to exactly one Open edX site, named in
config.yaml. WithLMS_DIRECTORYenabled it instead shows a list of platforms,lets the learner pick one, re-themes to it — logo, accent colour, sign-in artwork
— and signs in against that one. Logging out returns to the list, so a device can
move between platforms without a new build.
Off by default. With
ENABLED: falsenone of this runs and the app behavesexactly as it does today. Every shipped config in this PR has it disabled with no
source set.
Where the list comes from
One JSON document, given either way — never both:
A bundled file wins over an address: a build that ships its own copy has
deliberately opted out of the network, and quietly preferring a remote list would
undo that. Image fields inside the document are either web addresses or names of
files shipped with the app, so a build can be fully offline — list and artwork
both — and still be branded.
Because the whole list arrives at once, every platform's sign-in background is
known before anything is tapped. That is what makes it possible to warm the
artwork while the learner is still choosing, instead of watching the branded
screen assemble itself afterwards.
Format, both delivery modes and a worked example:
Documentation/LMS_DIRECTORY.md.Scope
Multi-tenancy only: read a list, choose, sign in, log out, choose again. No
search, no QR sign-in, no reporting a platform, and nothing that talks to a
catalog service — the app knows about a document and nothing else. Those belong
to a universal-app phase and will come as their own PRs.
Testing
CoreTestsandAuthorizationTests: 75 tests, 0 failures on an iPhone 17 Prosimulator. They cover which source a config resolves to, reading a document from
a URL and from the bundle (including a decoding failure, an HTTP error and a lost
connection), image sources resolving to either an address or a bundled name, the
picker's states, and a selection reaching the coordinator that re-themes the app.
One of them pins the smallest document a person could reasonably write by hand,
because the same file has to work on Android too.
Notes for reviewers
Theme.Images.headerBackgroundholds a decodedUIImagerather than a URL.The sign-in header used
AsyncImage, which has no cache, so it refetched andflashed a placeholder on every appearance. Kingfisher does the fetching;
Themedoes not depend on it, becauseCorealready depends onTheme.LMSSelectionCoordinatoris built inline on the main actor rather thanregistered with Swinject: the coordinator is
@MainActor-isolated whileSwinject's factory closure is not, and a registration drops the global actor.