feat: LMS Directory — let a learner pick which Open edX platform to sign in to - #664
feat: LMS Directory — let a learner pick which Open edX platform to sign in to#664IvanStepanok wants to merge 38 commits into
Conversation
Fix/issue 581
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); stock single-tenant flow is untouched when the flag is off. - Core: LMSDirectoryConfig flag + ConfigProtocol.lmsDirectory; Config.baseURL resolves to the selected LMS when enabled; CoreStorage.selectedLMSBaseURL - Authorization: TenantPicker feature — directory search, QR login, per-LMS runtime theming, selection coordinator, remote + mock services - App: RouteController landing branch, AppDelegate flag-gated registration, LMSDirectoryRouter, NSCameraUsageDescription for QR - Profile: 'Report this LMS' flow (flag-gated) posting to the registry - default_config: LMS_DIRECTORY block for dev/stage/prod - Regenerated mocks for the new ConfigProtocol/CoreStorage members
The directory talks to the site registry, not the app's API host. It was being blocked by the global connectivity check (which pings API_HOST_URL) and by a ViewModel override that forced an offline state from the same signal — so in dev, where API_HOST_URL is an unreachable placeholder, the catalog never loaded even though the registry was reachable. Reachability is now decided by the actual registry call: a genuine URLError maps to .offline, an unreachable stock host does not. Removed the connectivity dependency from the directory service and view model.
- LMSDirectoryViewModel.loadFeatured now maps LMSDirectoryError.offline to the
.offline state (was swallowed into a generic .error), matching search.
- Add AuthorizationTests/…/LMSDirectory regression tests:
* RemoteLMSDirectoryService decodes registry items and maps real URLError
(notConnected / timedOut / cannotConnectToHost) to .offline — proving the
directory no longer depends on stock-host ConnectivityProtocol.
* LMSDirectoryViewModel constructs without any connectivity dependency and
surfaces .offline for both search and curated/featured loading.
Completes the per-LMS experience so a selected platform actually works and looks like itself: - Config.oAuthClientId / feedbackEmail honor the per-LMS values persisted at selection (was the cause of failed logins — the app sent the empty stock client id instead of the platform's registered mobile client). - SignInView shows the selected LMS logo (falls back to the app logo) and a 'Selected LMS / Change' banner returning to the directory landing. - LMSSelectionRouting.showLanding() + LMSDirectoryRouter implementation. - CoreTests: overrides apply when enabled+selected, ignored when the flag is off, and fall back to config when nothing is selected.
|
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. |
# Conflicts: # Theme/Theme/Theme.swift
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.
The app can now take its platform list from one JSON file instead of a live
service, and that file can either be fetched from a URL or shipped inside the
app. Nothing downstream can tell which — so whoever ships a build decides where
the list lives, and the app learns nothing about that choice.
LMS_DIRECTORY:
ENABLED: true
DIRECTORY_URL: "https://example.com/directory.json" # or
DIRECTORY_FILE: "lms_directory.json" # bundled, no network
Image fields in the document are web addresses or names of images added to the
app; anything that is not http(s) is looked up in the bundle. One field, one
rule, so a hand-edited document stays readable.
Two 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 fall back to open search when it is offline — which is what
used to happen, silently.
- Every platform's sign-in background is known before anything is tapped, so the
artwork is prefetched into Kingfisher while the learner is still choosing.
Theme.Images now holds a decoded image rather than a URL, so the branded header
draws in its first frame instead of fading in after the rest of the screen.
15 tests: document parsing, the cache that keeps it to one request, the failure
modes (malformed, HTTP error, offline, missing bundled file), and the rule that
splits web addresses from bundled names.
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 — so a build reading its directory from a document must not offer it. The gate used to be 'not curated', read from a UserDefaults flag written when the picker was built and never cleared. That asked the wrong question and could go stale: a document build that had once run against a service would still show the button, pointing at a registry it no longer talks to. It now derives from the configured source. supportsReporting is true only for a live service, which is exactly the phase-2 universal app. The curated check stays alongside it, because a service in curated mode refuses reports too. 25 tests on iOS now, covering the source rule and this gate.
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, 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 reads that answer 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. Verified signed in on a simulator, 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, 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 answer 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; a failed refresh changes nothing, since a network error is not evidence. LMSDirectoryState is observable, so the Profile screen updates when the answer arrives rather than on the next visit. LMSDirectoryUITests drives the whole thing signed in. It skips unless a harness configures it — see tools/mobile-verification in the registry repository.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #664 +/- ##
========================================
Coverage ? 0
========================================
Files ? 0
Lines ? 0
Branches ? 0
========================================
Hits ? 0
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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/LMS_DIRECTORY.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. While writing that down: a document missing feature_flags decoded on Android and failed on iOS, which would have made the documented minimal example a lie on one of the two platforms. It decodes here now, with a test for the smallest file a person could reasonably write by hand. Also removes an audit report that was committed to this branch by mistake.
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/LMS_DIRECTORY.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
CoreTestsandAuthorizationTests: 103 tests, 0 failures on an iPhone 17Pro simulator. They cover reading a document from a URL and from the bundle,
image sources resolving to either an address or a bundled name, and every
transition of the directory mode — including an upgrade from a build that stored
only a boolean, and the same registry changing its mode at the same address.
LMSDirectoryUITestsdrives the running app signed in and asserts on whether thereporting entry point is there. It skips itself unless the environment configures
it (
TEST_RUNNER_LMS_DIRECTORY_UITEST=1, an expectation, and credentials), sinceit needs something to sign in to; the header explains what to point it at.
Verified on a simulator, 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.
ProfileViewobserves itso the entry point appears when the answer arrives rather than on a later visit.
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.http(s)address or the name of a file in thebundle. One field, one rule — the alternative was a parallel set of
*_assetfields and a precedence rule to go with them.