feat: LMS Directory — let a learner pick which Open edX platform to sign in to - #483
feat: LMS Directory — let a learner pick which Open edX platform to sign in to#483IvanStepanok wants to merge 10 commits into
Conversation
Lets a single build browse the Open edX platforms published by a site registry, re-theme to the chosen one, and sign in against it. Off by default (LMS_DIRECTORY.ENABLED=false); the app stays single-tenant when the flag is off. - core: LMSDirectoryConfig flag; Config.getApiHostURL() resolves to the selected LMS when enabled; CorePreferences.selectedBaseUrl/accentColor; lmsdirectory client (API/repo/models/Koin); LmsThemeController re-tints OpenEdXTheme to the platform's brand color - auth: LMS landing (Find my LMS / QR sign-in via ZXing), directory search/curated browse + manual entry, per-LMS theming on selection - profile: 'Report this LMS' bottom sheet (flag-gated) posting to the registry - app: flag-gated landing before sign-in; DI + startup wiring; seed theme from persisted accent on cold start - default_config: LMS_DIRECTORY block for dev/stage/prod - build: zxing-android-embedded for QR scanning
The catalog only returned summaries, so sign-in used the empty stock OAuth client and requests still hit the config host — login failed. Now: - fetch the full record on selection (LmsDetail): OAuth client id, feedback email, logo, accent; persist them in CorePreferences - Config.getOAuthClientId()/getFeedbackEmailAddress() honor the selected LMS - BaseUrlOverrideInterceptor rewrites every request to the selected host on the fly (the Retrofit client is built once, before selection) — the missing piece that let requests reach the chosen platform - SignIn shows the selected LMS logo + a 'Selected LMS / Change' banner - verified end-to-end: pick Sandbox -> branded sign-in -> logged into sandbox.openedx.org with the demo course loaded
- BaseUrlOverrideInterceptor routes requests to the selected host and is a no-op when nothing is selected (the mechanism that lets sign-in reach the platform). - LmsDetailDto maps the OAuth client id / feedback email / host that the catalog summary lacks, with blank values falling back correctly.
|
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. 🔘 Update the status of your PRYour PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate. Where 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. |
Upstream openedx#420 replaced the browser-OAuth-code login with SAML SSO, deleting getAccessTokenFromCode / browserAuthCodeLogin / loginAuthCode / BrowserAuthHelper / GRANT_TYPE_CODE. The LMS Directory sign-in was built on that chain, so rather than resurrect what upstream removed, the directory now signs in through upstream's standard login against the re-pointed base URL: - Keep the directory feature: pre-login picker, per-LMS branding (accent, logo, login-background header), the 'Selected LMS / Change' banner, and Report this LMS. - Drop the browser-auth-code path: removed signInAuthCode + loginFailure and the authCode wiring in SignInFragment/AppActivity/DI; sign-in uses username/password (or SSO) against the selected host via BaseUrlOverrideInterceptor. - Config keeps both getLMSDirectoryConfig() and upstream's SSO getters. - Tests: stub Config.getLMSDirectoryConfig() in SignInViewModelTest; pass the new config arg in ProfileViewModelTest. Verified locally: compile, full testDevelopDebugUnitTest, and detektAll all pass.
The iOS and Android work is now open as pull requests against the official Open edX app repos, not the old white-label branches: - iOS: openedx/openedx-app-ios#664 - Android: openedx/openedx-app-android#483 Update the Mobile apps section and the pilot-status note accordingly.
Mirrors the iOS change. The app can now take its platform list from one JSON
file instead of a live service, and that file can be fetched from a URL or read
out of the app's assets. Nothing above the source can tell which.
LMS_DIRECTORY:
ENABLED: true
DIRECTORY_URL: "https://example.com/directory.json" # or
DIRECTORY_FILE: "lms_directory.json" # in assets, no network
LmsDirectoryRepository now delegates to an LmsDirectorySource rather than
calling Retrofit itself. Reporting still needs the service, so the repository
takes the api only when there is one — a document is a one-way list.
Image fields are web addresses or names of files in assets; anything that is not
http(s) becomes file:///android_asset/…, which Coil reads natively, so only the
resolver knows about the distinction.
Three things fall out of reading everything at once:
- A document is always curated. There is no server to ask what mode to be in, so
a build cannot silently fall back to open search when it is offline — which is
what used to happen here, without even a log line the user could see.
- isCurated is now seeded from the config before any work starts, so a document
build no longer flashes the generic landing during the config round trip.
- Every sign-in background is known before a platform is tapped, so the artwork
is prefetched into Coil while the learner is still choosing, and the header no
longer crossfades in after the screen is already up.
17 new tests over the document, the image rule and the config's source rule; the
24 existing auth tests still pass.
Mirrors the iOS change. Reporting exists because the open catalog lets a stranger list anything; it belongs to the universal app, not to a provider shipping their own list. A build reading its directory from a document has nowhere to post, so the entry point must not appear. The gate used to be isReachable && !lmsDirectoryCurated, where the curated half came from a persisted preference written by the picker. That asked the wrong question and could go stale. It now derives from the configured source: supportsReporting is true only for a live service. The curated check stays, since a service in curated mode refuses reports too. The repository already refused to build a report without an api; this stops the button appearing at all rather than failing when pressed.
Whether a live catalog is curated is something only the server knows, and it says so on the platform picker — a screen the app stops showing once a platform has been chosen. The answer was therefore remembered in preferences, and then believed forever: point the build at a different registry, or at a document, and it kept obeying what the old one had said. The Profile tab read that flag to decide whether to offer reporting, so the entry point could stay hidden for a whole release. The answer is now stored beside a key naming the source that gave it, and read back only while the build still reads that source. A value left by an older build carries no key and is not trusted; a source change drops it at launch. ProfileViewModel derives the decision instead of the fragment reading the raw flag. Verified signed in on an emulator, both ways round: a document build offers no reporting, and the same app rebuilt against a service — data kept, picker skipped — offers it again.
The app remembered whether its catalog was curated as a boolean in preferences, so the absent value had to mean something, and it meant open. Two ordinary situations start there: an install upgraded from a build that stored no source, and a build newly pointed at a different registry. Both would show a learner "Report this LMS" for a catalog nobody had vouched for — and, the other way round, a stale flag could keep the entry point hidden for the life of a release. There are now three states. A document settles it locally, DIRECTORY_MODE settles it either way, and a live catalog is unknown until it answers for the exact source this build reads. Unknown shows nothing. The answer is refreshed at launch as well as by the platform picker, because a learner who has chosen a platform never sees that picker again. fetchConfigOrNull exists so that refresh can tell an unreachable registry from an answer: fetchConfig falls back to search, which is right for drawing the picker and wrong for recording what a catalog is. ProfileViewModel exposes the decision as a flow so the screen updates when the answer arrives.
Two things were wrong for anyone reading this repository rather than running it. The dev config had the feature on and pointed at somebody's private deployment, so a checkout built a multi-tenant app aimed at a host the reader has no business talking to. All three environments now ship it off with no URL set, which is what a default should be. And the config never made it obvious that DIRECTORY_URL takes either kind of address. It does — a .json address is a document, anything else is a live registry — so the comment now says so with a working example of each, and Documentation/LMSDirectory.md spells out the document format field by field, including how to bundle one with its images for a build that never asks the network. A test pins the smallest file a person could reasonably write by hand, because iOS accepts exactly the same one. Also drops the boolean this feature used to store once an upgraded install has recorded a real answer, so nothing is left behind that a later reader could mistake for one.
What this adds
A learner opens the app and picks which Open edX platform to sign in to, instead
of the app being built for exactly one site. The app then re-themes to the chosen
platform — its logo, accent colour and sign-in artwork — and authenticates
against it.
Behind
LMS_DIRECTORY.ENABLED, off by default. With the flag off nothinghere runs and the app behaves exactly as it does today; every default config in
this PR ships with the feature disabled and no URL set.
Where the platform list comes from
Three ways, and the config alone decides which:
A
DIRECTORY_URLending in.jsonis read as a document: one file, fetchedonce, that already contains every platform and its branding. Any other address is
treated as a live registry answering
/api/v1/directory. A file bundled withthe app wins over both — a build that ships its own copy has deliberately opted
out of the network.
The document is the part worth reviewing carefully, because it is what makes this
usable without anyone running a service. It also removed a real hazard: both apps
used to fall back to open search when a registry was unreachable, so a branded
build starting offline showed a search box over every public platform. A document
has no server to ask, so there is nothing to fall back to.
Format, both delivery modes and a worked example:
Documentation/LMSDirectory.md.Reporting a platform is not in this PR's scope
"Report this LMS" appears only when the directory is a live registry that says it
is an open catalog — the one situation where a stranger can list something nobody
has vouched for. A build reading a document never shows it.
That decision is enforced by an explicit three-state mode —
unknown | search | curated— rather than a boolean. A boolean has no room for "the server has notsaid yet", so its absent value had to mean something, and it meant open: an
upgraded install and a build newly pointed at another registry would both offer
reporting for a catalog nobody had vouched for. The mode is stored against a key
naming the source that produced it, refreshed on every launch (a learner who has
picked a platform never sees the picker again, so a registry that changes its mode
would otherwise go unnoticed), and anything unknown stays hidden.
Testing
Unit tests across
core,auth,profileandapp: 306 tests, 0 failures,with
detektAllclean over the whole project. They cover reading a document fromassets and from a URL, image references resolving to an address or to
file:///android_asset/…, and every transition of the directory mode — includingan upgrade from a build that stored only a boolean, and the same registry
changing its mode at the same address.
Verified on an emulator, signed in, both ways round: a build reading a document
offers no reporting, and the same install rebuilt against a registry — data kept,
platform picker skipped — offers it again.
Notes for reviewers
coregainedLmsDirectoryState, which owns the whole rule about what thedirectory is and how long that knowledge is good for.
ProfileViewModelexposes the decision as a flow so the screen updates when the answer arrives.
LmsDirectoryRepository.fetchConfigOrNullsits besidefetchConfigon purpose.fetchConfigfalls back to search when the registry is unreachable, which isright for drawing the picker and wrong for recording what a catalog is — a
network error is not evidence.
LmsImageSource, which returns either an http URLor
file:///android_asset/…; Coil reads both natively.removed the first time an upgraded install records a real answer.