From b97f27f80b7d64173cb9751e01e4fc671f1539fa Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Thu, 20 Aug 2026 20:54:20 +0300 Subject: [PATCH 1/5] feat: let a learner choose which Open edX platform to sign in to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 the app's assets, 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 lets that artwork be warmed while the learner is still choosing rather than assembling itself afterwards. Image references carry either an http(s) address or the name of a file in assets, which Coil reads as file:///android_asset/… natively. 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. Documentation/LMSDirectory.md has the format, both delivery modes and a worked example. --- Documentation/LMSDirectory.md | 139 ++++++++++ .../main/java/org/openedx/app/AppActivity.kt | 9 +- .../main/java/org/openedx/app/AppRouter.kt | 31 ++- .../main/java/org/openedx/app/AppViewModel.kt | 7 + .../main/java/org/openedx/app/MainFragment.kt | 25 ++ .../main/java/org/openedx/app/OpenEdXApp.kt | 13 +- .../networking/BaseUrlOverrideInterceptor.kt | 50 ++++ .../app/data/storage/PreferencesManager.kt | 46 ++++ .../main/java/org/openedx/app/di/AppModule.kt | 4 +- .../org/openedx/app/di/NetworkingModule.kt | 3 + .../BaseUrlOverrideInterceptorTest.kt | 63 +++++ .../app/lmsdirectory/LmsDetailDtoTest.kt | 63 +++++ .../openedx/auth/presentation/AuthRouter.kt | 3 + .../lmsselection/SiteSelectionCallbacks.kt | 9 + .../lmsselection/SiteSelectionFragment.kt | 74 ++++++ .../lmsselection/SiteSelectionScreen.kt | 246 ++++++++++++++++++ .../lmsselection/SiteSelectionUIState.kt | 24 ++ .../lmsselection/SiteSelectionViewModel.kt | 142 ++++++++++ .../restore/RestorePasswordFragment.kt | 10 +- .../presentation/signin/SignInFragment.kt | 7 + .../auth/presentation/signin/SignInUIState.kt | 5 + .../presentation/signin/SignInViewModel.kt | 19 ++ .../presentation/signin/compose/SignInView.kt | 115 +++++++- .../presentation/signup/compose/SignUpView.kt | 11 +- auth/src/main/res/values/strings.xml | 10 + .../SiteSelectionViewModelTest.kt | 143 ++++++++++ .../signin/SignInViewModelTest.kt | 2 + core/build.gradle | 1 + .../java/org/openedx/core/config/Config.kt | 36 ++- .../openedx/core/config/LMSDirectoryConfig.kt | 56 ++++ .../core/data/storage/CorePreferences.kt | 25 ++ .../core/lmsdirectory/DirectoryModels.kt | 33 +++ .../openedx/core/lmsdirectory/LmsDetailDto.kt | 44 ++++ .../core/lmsdirectory/LmsDirectoryModule.kt | 45 ++++ .../lmsdirectory/LmsDirectoryRepository.kt | 38 +++ .../core/lmsdirectory/LmsDirectorySource.kt | 138 ++++++++++ .../core/lmsdirectory/LmsImageSource.kt | 51 ++++ .../core/lmsdirectory/LmsThemeController.kt | 54 ++++ .../org/openedx/core/ui/ComposeExtensions.kt | 20 +- .../org/openedx/core/ui/LmsHeaderImage.kt | 48 ++++ .../java/org/openedx/core/ui/theme/Theme.kt | 37 ++- .../core/config/LMSDirectoryConfigTest.kt | 63 +++++ .../DocumentLmsDirectorySourceTest.kt | 143 ++++++++++ .../core/lmsdirectory/LmsImageSourceTest.kt | 60 +++++ default_config/dev/config.yaml | 16 ++ default_config/prod/config.yaml | 16 ++ default_config/stage/config.yaml | 16 ++ 47 files changed, 2174 insertions(+), 39 deletions(-) create mode 100644 Documentation/LMSDirectory.md create mode 100644 app/src/main/java/org/openedx/app/data/networking/BaseUrlOverrideInterceptor.kt create mode 100644 app/src/test/java/org/openedx/app/data/networking/BaseUrlOverrideInterceptorTest.kt create mode 100644 app/src/test/java/org/openedx/app/lmsdirectory/LmsDetailDtoTest.kt create mode 100644 auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionCallbacks.kt create mode 100644 auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionFragment.kt create mode 100644 auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionScreen.kt create mode 100644 auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionUIState.kt create mode 100644 auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionViewModel.kt create mode 100644 auth/src/test/java/org/openedx/auth/presentation/lmsselection/SiteSelectionViewModelTest.kt create mode 100644 core/src/main/java/org/openedx/core/config/LMSDirectoryConfig.kt create mode 100644 core/src/main/java/org/openedx/core/lmsdirectory/DirectoryModels.kt create mode 100644 core/src/main/java/org/openedx/core/lmsdirectory/LmsDetailDto.kt create mode 100644 core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectoryModule.kt create mode 100644 core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectoryRepository.kt create mode 100644 core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectorySource.kt create mode 100644 core/src/main/java/org/openedx/core/lmsdirectory/LmsImageSource.kt create mode 100644 core/src/main/java/org/openedx/core/lmsdirectory/LmsThemeController.kt create mode 100644 core/src/main/java/org/openedx/core/ui/LmsHeaderImage.kt create mode 100644 core/src/test/java/org/openedx/core/config/LMSDirectoryConfigTest.kt create mode 100644 core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt create mode 100644 core/src/test/java/org/openedx/core/lmsdirectory/LmsImageSourceTest.kt diff --git a/Documentation/LMSDirectory.md b/Documentation/LMSDirectory.md new file mode 100644 index 000000000..77d08dc7e --- /dev/null +++ b/Documentation/LMSDirectory.md @@ -0,0 +1,139 @@ +# The LMS Directory + +A build of this app normally talks to one Open edX site, named in +`config.yaml`. With the LMS Directory on, it instead shows a list of platforms, +lets the learner pick one, re-themes to it and signs in against it. + +Off by default. With `ENABLED: false` nothing in this document applies and the +app behaves exactly as it always has. + +## Where the list comes from + +Two ways, and the config decides which: + +```yaml +LMS_DIRECTORY: + ENABLED: true + DIRECTORY_URL: "https://example.com/lms_directory.json" # a document, on the web + DIRECTORY_FILE: "" +``` + +```yaml +LMS_DIRECTORY: + ENABLED: true + DIRECTORY_URL: "" + DIRECTORY_FILE: "lms_directory.json" # a document, in the app +``` + +`DIRECTORY_URL` is fetched once, and whatever comes back is the document — the +address can be anything you can serve a file from. If both `DIRECTORY_URL` and +`DIRECTORY_FILE` are set the file wins: a build that ships its own copy has +deliberately opted out of the network, and quietly preferring a remote list would +undo that. + +## What a document looks like + +One JSON file. This is the whole format: + +```json +{ + "version": 1, + "provider": { + "name": "Northwind Education Group", + "tagline": "Five campuses, one app", + "logo_url": null + }, + "platforms": [ + { + "id": "1", + "title": "Northwind College", + "description": "The main campus, offering undergraduate programmes.", + "short_description": "Main campus", + "base_url": "https://learn.northwind.edu", + "logo_url": "https://cdn.northwind.edu/logo.png", + "accent_color": "#002545", + "visibility": "public", + "featured": false, + "api": { + "host_url": "https://learn.northwind.edu", + "feedback_email": "support@northwind.edu", + "oauth_client_id": "PASTE_THE_MOBILE_OAUTH_CLIENT_ID" + }, + "feature_flags": { + "pre_login_discovery": false, + "unknown_units_mode": "webview" + }, + "theme": { + "accent_color_dark": "#4989bf", + "login_background_url": "https://cdn.northwind.edu/signin.png", + "logo_upload_url": null + }, + "ui_components": { + "course_unit_progress_enabled": true, + "course_dropdown_navigation_enabled": true, + "pre_login_experience_enabled": false + }, + "dashboard": { "type": "list" } + } + ] +} +``` + +### Required + +| field | what it is | +| --- | --- | +| `version` | `1`. The only version there is. | +| `platforms[]` | At least one. An empty list gives the learner nothing to pick. | +| `id` | Unique within the file. A string, even when it looks like a number. | +| `title` | Shown in the list and on the sign-in screen. | +| `description` / `short_description` | Long and one-line blurbs. | +| `base_url` | The Open edX site. Must be `https` in a shipped build. | +| `api.host_url` | Usually the same as `base_url`. | +| `api.oauth_client_id` | The site's **mobile** OAuth client id. Sign-in fails without the right one. | +| `api.feedback_email` | May be `""`. | + +### Optional + +Everything else. Omit a key and the app uses its own default, so the smallest +useful entry is `id`, `title`, `description`, `short_description`, `base_url` +and `api`. `provider` is optional too; its `name` is shown above the list. + +`visibility` and `featured` are accepted and ignored — every platform in the +file is shown, in the order the file lists them. + +## Images + +Every image field takes either of two things, and the value itself says which: + +- something starting with `http://` or `https://` is downloaded; +- anything else is the **name of a file shipped with the app**. + +So `"logo_url": "https://cdn.northwind.edu/logo.png"` is fetched, and +`"logo_url": "northwind-logo.png"` is read from `assets/` — Coil resolves it as +`file:///android_asset/…` natively. That is what makes a fully offline build possible: put the images next to the document, refer to them +by name, and the app never asks the network for a picture. + +## Shipping the document inside the app + +1. Put the document and its images in `app/src/main/assets/`. +2. That is all — assets need no registration. + +Then set `DIRECTORY_FILE` to the file name and leave `DIRECTORY_URL` empty. The +app now works on a device that has never been online. + +## Where to get a document + +**Write it by hand.** For a handful of platforms this is the honest answer — +it is one JSON file, and the example above is a working template. + +**Or edit it somewhere.** Any tool that emits the shape above will do, and one +that exists today is : a form for adding +platforms and uploading their logos and sign-in artwork, which publishes the +document at a URL and also exports a `.zip` of the document with its image +fields already rewritten to file names, plus the images themselves — the bundle +the offline case needs. + +That is somebody's **unofficial** tool. It is not part of Open edX, not +maintained by this project, and nothing here depends on it. The app reads a +document; where the document came from is not its business. diff --git a/app/src/main/java/org/openedx/app/AppActivity.kt b/app/src/main/java/org/openedx/app/AppActivity.kt index f0a71f713..5bdc5bd30 100644 --- a/app/src/main/java/org/openedx/app/AppActivity.kt +++ b/app/src/main/java/org/openedx/app/AppActivity.kt @@ -21,6 +21,7 @@ import org.koin.android.ext.android.inject import org.koin.androidx.viewmodel.ext.android.viewModel import org.openedx.app.databinding.ActivityAppBinding import org.openedx.app.deeplink.DeepLink +import org.openedx.auth.presentation.lmsselection.SiteSelectionFragment import org.openedx.auth.presentation.logistration.LogistrationFragment import org.openedx.auth.presentation.signin.SignInFragment import org.openedx.core.data.storage.CorePreferences @@ -158,10 +159,10 @@ class AppActivity : AppCompatActivity(), InsetHolder, WindowSizeHolder { if (savedInstanceState == null) { when { corePreferencesManager.user == null -> { - val fragment = if (viewModel.isLogistrationEnabled && authCode == null) { - LogistrationFragment() - } else { - SignInFragment.newInstance(null, null) + val fragment = when { + viewModel.isLmsSelectionRequired && authCode == null -> SiteSelectionFragment() + viewModel.isLogistrationEnabled && authCode == null -> LogistrationFragment() + else -> SignInFragment.newInstance(null, null) } addFragment(fragment) } diff --git a/app/src/main/java/org/openedx/app/AppRouter.kt b/app/src/main/java/org/openedx/app/AppRouter.kt index a511dc839..e3869cf21 100644 --- a/app/src/main/java/org/openedx/app/AppRouter.kt +++ b/app/src/main/java/org/openedx/app/AppRouter.kt @@ -5,12 +5,16 @@ import androidx.fragment.app.FragmentManager import androidx.fragment.app.FragmentTransaction import org.openedx.app.deeplink.HomeTab import org.openedx.auth.presentation.AuthRouter +import org.openedx.auth.presentation.lmsselection.SiteSelectionFragment import org.openedx.auth.presentation.logistration.LogistrationFragment import org.openedx.auth.presentation.restore.RestorePasswordFragment import org.openedx.auth.presentation.signin.SignInFragment import org.openedx.auth.presentation.signup.SignUpFragment import org.openedx.core.CalendarRouter import org.openedx.core.FragmentViewType +import org.openedx.core.config.Config +import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.lmsdirectory.LmsThemeController import org.openedx.core.presentation.global.appupgrade.AppUpgradeRouter import org.openedx.core.presentation.global.appupgrade.UpgradeRequiredFragment import org.openedx.core.presentation.global.webview.SSOWebContentFragment @@ -61,7 +65,10 @@ import org.openedx.profile.presentation.video.VideoSettingsFragment import org.openedx.whatsnew.WhatsNewRouter import org.openedx.whatsnew.presentation.whatsnew.WhatsNewFragment -class AppRouter : +class AppRouter( + private val config: Config, + private val corePreferences: CorePreferences, +) : AuthRouter, DiscoveryRouter, DashboardRouter, @@ -103,6 +110,10 @@ class AppRouter : replaceFragmentWithBackStack(fm, LogistrationFragment.newInstance(courseId)) } + override fun navigateToLmsSelection(fm: FragmentManager) { + replaceFragmentWithBackStack(fm, SiteSelectionFragment()) + } + override fun navigateToDownloadQueue(fm: FragmentManager, descendants: List) { replaceFragmentWithBackStack(fm, DownloadQueueFragment.newInstance(descendants)) } @@ -406,10 +417,20 @@ class AppRouter : override fun restartApp(fm: FragmentManager, isLogistrationEnabled: Boolean) { fm.apply { clearBackStack(this) - if (isLogistrationEnabled) { - replaceFragment(fm, LogistrationFragment()) - } else { - replaceFragment(fm, SignInFragment.newInstance(null, null)) + when { + // LMS Directory: after logout the selection is cleared (see + // clearCorePreferences), so when the feature is reachable return to the + // platform picker instead of the stock sign-in — matches the app-launch + // path in AppActivity.setupInitialFragment and the iOS behavior. Reset the + // in-memory accent so the neutral landing isn't tinted by the old LMS. + config.getLMSDirectoryConfig().isReachable && + corePreferences.selectedBaseUrl.isNullOrBlank() -> { + LmsThemeController.clear() + replaceFragment(fm, SiteSelectionFragment()) + } + + isLogistrationEnabled -> replaceFragment(fm, LogistrationFragment()) + else -> replaceFragment(fm, SignInFragment.newInstance(null, null)) } } } diff --git a/app/src/main/java/org/openedx/app/AppViewModel.kt b/app/src/main/java/org/openedx/app/AppViewModel.kt index bafddb19b..74939976c 100644 --- a/app/src/main/java/org/openedx/app/AppViewModel.kt +++ b/app/src/main/java/org/openedx/app/AppViewModel.kt @@ -57,6 +57,13 @@ class AppViewModel( val isLogistrationEnabled get() = config.isPreLoginExperienceEnabled() + /** + * LMS Directory: on first launch (before sign-in) the learner must pick a platform. + * True only when the feature is on and nothing is selected yet. + */ + val isLmsSelectionRequired: Boolean + get() = config.getLMSDirectoryConfig().isReachable && preferencesManager.selectedBaseUrl.isNullOrBlank() + private var logoutHandledAt: Long = 0 val isBranchEnabled get() = config.getBranchConfig().enabled diff --git a/app/src/main/java/org/openedx/app/MainFragment.kt b/app/src/main/java/org/openedx/app/MainFragment.kt index 397216b74..c96697a57 100644 --- a/app/src/main/java/org/openedx/app/MainFragment.kt +++ b/app/src/main/java/org/openedx/app/MainFragment.kt @@ -1,5 +1,6 @@ package org.openedx.app +import android.content.res.ColorStateList import android.os.Bundle import android.view.Menu import android.view.View @@ -8,6 +9,8 @@ import androidx.compose.runtime.getValue import androidx.compose.runtime.livedata.observeAsState import androidx.compose.runtime.remember import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.toArgb +import androidx.core.content.ContextCompat import androidx.core.os.bundleOf import androidx.core.view.forEach import androidx.fragment.app.Fragment @@ -22,6 +25,7 @@ import org.openedx.app.deeplink.HomeTab import org.openedx.core.AppUpdateState import org.openedx.core.AppUpdateState.wasUpgradeDialogClosed import org.openedx.core.adapter.NavigationFragmentAdapter +import org.openedx.core.lmsdirectory.LmsThemeController import org.openedx.core.presentation.dialog.appupgrade.AppUpgradeDialogFragment import org.openedx.core.presentation.global.appupgrade.AppUpgradeRecommendedBox import org.openedx.core.presentation.global.appupgrade.UpgradeRequiredFragment @@ -84,10 +88,31 @@ class MainFragment : Fragment(R.layout.fragment_main) { val tabList = createTabList(openTabArg) addMenuItems(menu, tabList) setupBottomNavListener(tabList) + applyLmsAccentTint() requireArguments().remove(ARG_OPEN_TAB) } + /** + * LMS Directory: the bottom bar is a View-based [BottomNavigationView], so the Compose + * accent theme doesn't reach it — its selected color stays the baked-in stock blue. + * When a platform is selected, tint the checked item with the LMS accent so the tab bar + * matches the rest of the re-themed app (and iOS). Unchecked keeps the stock grey. + */ + private fun applyLmsAccentTint() { + val accent = LmsThemeController.accentColor ?: return + val unchecked = ContextCompat.getColor(requireContext(), org.openedx.core.R.color.unchecked_tab_item) + val tint = ColorStateList( + arrayOf( + intArrayOf(android.R.attr.state_checked), + intArrayOf(-android.R.attr.state_checked), + ), + intArrayOf(accent.toArgb(), unchecked), + ) + binding.bottomNavView.itemIconTintList = tint + binding.bottomNavView.itemTextColor = tint + } + private fun createTabList(openTabArg: String): List Fragment>> { val learnFragmentFactory = { LearnFragment.newInstance( diff --git a/app/src/main/java/org/openedx/app/OpenEdXApp.kt b/app/src/main/java/org/openedx/app/OpenEdXApp.kt index 6524cde5d..ad9dcb816 100644 --- a/app/src/main/java/org/openedx/app/OpenEdXApp.kt +++ b/app/src/main/java/org/openedx/app/OpenEdXApp.kt @@ -14,11 +14,15 @@ import org.openedx.app.di.appModule import org.openedx.app.di.networkingModule import org.openedx.app.di.screenModule import org.openedx.core.config.Config +import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.lmsdirectory.LmsThemeController +import org.openedx.core.lmsdirectory.lmsDirectoryModule import org.openedx.firebase.OEXFirebaseAnalytics class OpenEdXApp : Application() { private val config by inject() + private val corePreferences by inject() private val pluginManager by inject() override fun onCreate() { @@ -28,9 +32,16 @@ class OpenEdXApp : Application() { modules( appModule, networkingModule, - screenModule + screenModule, + lmsDirectoryModule ) } + // LMS Directory: re-apply the selected platform's brand color on cold start so + // the whole app is themed before the first screen composes. No-op when off. + if (config.getLMSDirectoryConfig().isReachable) { + LmsThemeController.apply(corePreferences.selectedLmsAccentColor) + LmsThemeController.applyBackground(corePreferences.selectedLmsLoginBackgroundUrl) + } if (config.getFirebaseConfig().enabled) { FirebaseApp.initializeApp(this) } diff --git a/app/src/main/java/org/openedx/app/data/networking/BaseUrlOverrideInterceptor.kt b/app/src/main/java/org/openedx/app/data/networking/BaseUrlOverrideInterceptor.kt new file mode 100644 index 000000000..301bfb8e3 --- /dev/null +++ b/app/src/main/java/org/openedx/app/data/networking/BaseUrlOverrideInterceptor.kt @@ -0,0 +1,50 @@ +package org.openedx.app.data.networking + +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull +import okhttp3.Interceptor +import okhttp3.Response +import org.openedx.core.data.storage.CorePreferences + +/** + * LMS Directory: routes every API request to the platform the learner selected. + * + * The Retrofit client is built once with the config host, but the selected LMS can + * differ (and is chosen after the client exists). This rewrites each request's + * scheme/host/port to [CorePreferences.selectedBaseUrl] on the fly. No selection + * (feature off, or a stock build) → requests pass through untouched. + */ +class BaseUrlOverrideInterceptor( + private val corePreferences: CorePreferences, +) : Interceptor { + + override fun intercept(chain: Interceptor.Chain): Response { + val override = corePreferences.selectedBaseUrl + val original = chain.request() + + if (override.isNullOrBlank()) { + return chain.proceed(original) + } + + val baseUrl = override.toHttpUrlOrNull() + val originalUrl = original.url + + val needsUpdate = baseUrl != null && ( + originalUrl.host != baseUrl.host || + originalUrl.port != baseUrl.port || + originalUrl.scheme != baseUrl.scheme + ) + + val requestToProcess = if (needsUpdate && baseUrl != null) { + val updatedUrl = originalUrl.newBuilder() + .scheme(baseUrl.scheme) + .host(baseUrl.host) + .port(baseUrl.port) + .build() + original.newBuilder().url(updatedUrl).build() + } else { + original + } + + return chain.proceed(requestToProcess) + } +} diff --git a/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt b/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt index 48b0d58a1..212adecd9 100644 --- a/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt +++ b/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt @@ -85,6 +85,14 @@ class PreferencesManager( val LAST_WHATS_NEW_VERSION = stringPreferencesKey("last_whats_new_version") val LAST_REVIEW_VERSION = stringPreferencesKey("last_review_version") val WAS_POSITIVE_RATED = booleanPreferencesKey("app_was_positive_rated") + val SELECTED_BASE_URL = stringPreferencesKey("selected_base_url") + val SELECTED_LMS_ACCENT_COLOR = stringPreferencesKey("selected_lms_accent_color") + val SELECTED_OAUTH_CLIENT_ID = stringPreferencesKey("selected_oauth_client_id") + val SELECTED_FEEDBACK_EMAIL = stringPreferencesKey("selected_feedback_email") + val SELECTED_LMS_LOGO_URL = stringPreferencesKey("selected_lms_logo_url") + val SELECTED_LMS_LOGIN_BACKGROUND = + stringPreferencesKey("selected_lms_login_background") + val SELECTED_LMS_TITLE = stringPreferencesKey("selected_lms_title") fun calendarSyncDialogShown(courseName: String) = booleanPreferencesKey("calendar_sync_dialog_${courseName.replaceSpace("_")}") @@ -128,6 +136,16 @@ class PreferencesManager( prefs.remove(Keys.EXPIRES_IN) prefs.remove(Keys.USER) prefs.remove(Keys.ACCOUNT) + // LMS Directory: drop the selected platform on logout so the app returns to + // the platform picker (matches iOS) instead of silently reusing the previous + // LMS's host/branding for the next user. No-op for single-tenant builds. + prefs.remove(Keys.SELECTED_BASE_URL) + prefs.remove(Keys.SELECTED_LMS_ACCENT_COLOR) + prefs.remove(Keys.SELECTED_OAUTH_CLIENT_ID) + prefs.remove(Keys.SELECTED_FEEDBACK_EMAIL) + prefs.remove(Keys.SELECTED_LMS_LOGO_URL) + prefs.remove(Keys.SELECTED_LMS_LOGIN_BACKGROUND) + prefs.remove(Keys.SELECTED_LMS_TITLE) } } @@ -201,6 +219,34 @@ class PreferencesManager( get() = getValue(Keys.IS_RELATIVE_DATES_ENABLED, true) set(value) = setValue(Keys.IS_RELATIVE_DATES_ENABLED, value) + override var selectedBaseUrl: String? + get() = getValue(Keys.SELECTED_BASE_URL, "").ifEmpty { null } + set(value) = setValue(Keys.SELECTED_BASE_URL, value.orEmpty()) + + override var selectedLmsAccentColor: String? + get() = getValue(Keys.SELECTED_LMS_ACCENT_COLOR, "").ifEmpty { null } + set(value) = setValue(Keys.SELECTED_LMS_ACCENT_COLOR, value.orEmpty()) + + override var selectedOAuthClientId: String? + get() = getValue(Keys.SELECTED_OAUTH_CLIENT_ID, "").ifEmpty { null } + set(value) = setValue(Keys.SELECTED_OAUTH_CLIENT_ID, value.orEmpty()) + + override var selectedFeedbackEmail: String? + get() = getValue(Keys.SELECTED_FEEDBACK_EMAIL, "").ifEmpty { null } + set(value) = setValue(Keys.SELECTED_FEEDBACK_EMAIL, value.orEmpty()) + + override var selectedLmsLogoUrl: String? + get() = getValue(Keys.SELECTED_LMS_LOGO_URL, "").ifEmpty { null } + set(value) = setValue(Keys.SELECTED_LMS_LOGO_URL, value.orEmpty()) + + override var selectedLmsLoginBackgroundUrl: String? + get() = getValue(Keys.SELECTED_LMS_LOGIN_BACKGROUND, "").ifEmpty { null } + set(value) = setValue(Keys.SELECTED_LMS_LOGIN_BACKGROUND, value.orEmpty()) + + override var selectedLmsTitle: String? + get() = getValue(Keys.SELECTED_LMS_TITLE, "").ifEmpty { null } + set(value) = setValue(Keys.SELECTED_LMS_TITLE, value.orEmpty()) + override var profile: Account? get() { val json = getEncryptedString(Keys.ACCOUNT, "") diff --git a/app/src/main/java/org/openedx/app/di/AppModule.kt b/app/src/main/java/org/openedx/app/di/AppModule.kt index 267b73432..fcee60ab9 100644 --- a/app/src/main/java/org/openedx/app/di/AppModule.kt +++ b/app/src/main/java/org/openedx/app/di/AppModule.kt @@ -86,7 +86,7 @@ import org.openedx.core.DatabaseManager as IDatabaseManager val appModule = module { - single { Config(get()) } + single { Config(context = get(), corePreferences = get()) } single { PreferencesManager(get(), get()) } single { get() } single { get() } @@ -120,7 +120,7 @@ val appModule = module { single { DiscoveryNotifier() } single { CalendarNotifier() } - single { AppRouter() } + single { AppRouter(get(), get()) } single { get() } single { get() } single { get() } diff --git a/app/src/main/java/org/openedx/app/di/NetworkingModule.kt b/app/src/main/java/org/openedx/app/di/NetworkingModule.kt index 6360e7fba..6c09ac913 100644 --- a/app/src/main/java/org/openedx/app/di/NetworkingModule.kt +++ b/app/src/main/java/org/openedx/app/di/NetworkingModule.kt @@ -5,6 +5,7 @@ import okhttp3.logging.HttpLoggingInterceptor import org.koin.dsl.module import org.openedx.app.data.api.NotificationsApi import org.openedx.app.data.networking.AppUpgradeInterceptor +import org.openedx.app.data.networking.BaseUrlOverrideInterceptor import org.openedx.app.data.networking.HandleErrorInterceptor import org.openedx.app.data.networking.HeadersInterceptor import org.openedx.app.data.networking.OauthRefreshTokenAuthenticator @@ -29,6 +30,8 @@ val networkingModule = module { writeTimeout(60, TimeUnit.SECONDS) readTimeout(60, TimeUnit.SECONDS) addInterceptor(HeadersInterceptor(get(), get(), get())) + // LMS Directory: redirect requests to the selected platform (no-op when off). + addInterceptor(BaseUrlOverrideInterceptor(get())) if (BuildConfig.DEBUG) { addNetworkInterceptor(HttpLoggingInterceptor().setLevel(HttpLoggingInterceptor.Level.BODY)) } diff --git a/app/src/test/java/org/openedx/app/data/networking/BaseUrlOverrideInterceptorTest.kt b/app/src/test/java/org/openedx/app/data/networking/BaseUrlOverrideInterceptorTest.kt new file mode 100644 index 000000000..66392b2f1 --- /dev/null +++ b/app/src/test/java/org/openedx/app/data/networking/BaseUrlOverrideInterceptorTest.kt @@ -0,0 +1,63 @@ +package org.openedx.app.data.networking + +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import okhttp3.Interceptor +import okhttp3.Request +import okhttp3.Response +import org.junit.Assert.assertEquals +import org.junit.Test +import org.openedx.core.data.storage.CorePreferences + +/** + * Regression coverage for the LMS Directory login fix: with a platform selected, + * every request must be routed to that host (the Retrofit client is built once, + * before selection). No selection → the request is untouched. + */ +class BaseUrlOverrideInterceptorTest { + + private val corePreferences = mockk() + + private fun proceededRequest(selected: String?, requestUrl: String): Request { + every { corePreferences.selectedBaseUrl } returns selected + val original = Request.Builder().url(requestUrl).build() + val captured = slot() + val chain = mockk() + every { chain.request() } returns original + every { chain.proceed(capture(captured)) } returns mockk(relaxed = true) + + BaseUrlOverrideInterceptor(corePreferences).intercept(chain) + return captured.captured + } + + @Test + fun `rewrites host to the selected LMS`() { + val request = proceededRequest( + selected = "https://sandbox.openedx.org/", + requestUrl = "http://localhost:8000/oauth2/access_token", + ) + assertEquals("sandbox.openedx.org", request.url.host) + assertEquals("https", request.url.scheme) + assertEquals("/oauth2/access_token", request.url.encodedPath) + } + + @Test + fun `passes request through when nothing is selected`() { + val request = proceededRequest( + selected = null, + requestUrl = "http://localhost:8000/oauth2/access_token", + ) + assertEquals("localhost", request.url.host) + assertEquals(8000, request.url.port) + } + + @Test + fun `passes request through when selection is blank`() { + val request = proceededRequest( + selected = "", + requestUrl = "https://config-host.example.com/api/v1/x", + ) + assertEquals("config-host.example.com", request.url.host) + } +} diff --git a/app/src/test/java/org/openedx/app/lmsdirectory/LmsDetailDtoTest.kt b/app/src/test/java/org/openedx/app/lmsdirectory/LmsDetailDtoTest.kt new file mode 100644 index 000000000..3f009e393 --- /dev/null +++ b/app/src/test/java/org/openedx/app/lmsdirectory/LmsDetailDtoTest.kt @@ -0,0 +1,63 @@ +package org.openedx.app.lmsdirectory + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test +import org.openedx.core.lmsdirectory.LmsDetailDto + +/** + * The catalog summary can't log you in — only the detail carries the per-LMS OAuth + * client id and feedback email. This verifies the mapping the selection flow relies on. + */ +class LmsDetailDtoTest { + + @Test + fun `maps api fields to domain`() { + val detail = LmsDetailDto( + id = "4", + title = "Sandbox Env", + baseUrl = "https://sandbox.openedx.org", + logoUrl = "https://cdn.example.com/logo.png", + accentColor = "#6a2e7b", + api = LmsDetailDto.ApiDto( + hostUrl = "https://sandbox.openedx.org", + oauthClientId = "android", + feedbackEmail = "team@example.com", + ), + ).toDomain() + + assertEquals("android", detail.oauthClientId) + assertEquals("team@example.com", detail.feedbackEmail) + assertEquals("https://sandbox.openedx.org", detail.baseUrl) + assertEquals("#6a2e7b", detail.accentColor) + assertEquals("https://cdn.example.com/logo.png", detail.logoUrl) + } + + @Test + fun `blank api values fall back to null and base_url`() { + val detail = LmsDetailDto( + id = "1", + title = "Fallback", + baseUrl = "https://fallback.example.com", + api = LmsDetailDto.ApiDto(hostUrl = "", oauthClientId = "", feedbackEmail = null), + ).toDomain() + + // Blank host_url → the top-level base_url is used. + assertEquals("https://fallback.example.com", detail.baseUrl) + assertNull(detail.oauthClientId) + assertNull(detail.feedbackEmail) + } + + @Test + fun `null api yields base_url and null credentials`() { + val detail = LmsDetailDto( + id = "2", + title = "No API block", + baseUrl = "https://noapi.example.com", + api = null, + ).toDomain() + + assertEquals("https://noapi.example.com", detail.baseUrl) + assertNull(detail.oauthClientId) + } +} diff --git a/auth/src/main/java/org/openedx/auth/presentation/AuthRouter.kt b/auth/src/main/java/org/openedx/auth/presentation/AuthRouter.kt index ac657271f..65b40a3a4 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/AuthRouter.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/AuthRouter.kt @@ -15,6 +15,9 @@ interface AuthRouter { fun navigateToLogistration(fm: FragmentManager, courseId: String?) + /** LMS Directory: open the "Find my LMS" browse/search screen. */ + fun navigateToLmsSelection(fm: FragmentManager) + fun navigateToSignUp(fm: FragmentManager, courseId: String?, infoType: String?) fun navigateToRestorePassword(fm: FragmentManager) diff --git a/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionCallbacks.kt b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionCallbacks.kt new file mode 100644 index 000000000..49d217ff7 --- /dev/null +++ b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionCallbacks.kt @@ -0,0 +1,9 @@ +package org.openedx.auth.presentation.lmsselection + +import org.openedx.core.lmsdirectory.LmsSummary + +/** UI callbacks for [SiteSelectionScreen]. */ +class SiteSelectionCallbacks( + val onPlatformSelected: (LmsSummary) -> Unit, + val onRetry: () -> Unit, +) diff --git a/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionFragment.kt b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionFragment.kt new file mode 100644 index 000000000..bb290bb44 --- /dev/null +++ b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionFragment.kt @@ -0,0 +1,74 @@ +package org.openedx.auth.presentation.lmsselection + +import android.os.Bundle +import android.view.LayoutInflater +import android.view.ViewGroup +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue +import androidx.compose.ui.platform.ComposeView +import androidx.compose.ui.platform.ViewCompositionStrategy +import androidx.fragment.app.Fragment +import org.koin.android.ext.android.inject +import org.koin.androidx.viewmodel.ext.android.viewModel +import org.openedx.auth.presentation.AuthRouter +import org.openedx.core.config.Config +import org.openedx.core.ui.theme.OpenEdXTheme + +/** + * The platform picker, shown before sign-in when the directory is configured and + * no platform has been chosen yet. Picking one re-themes the app to it and + * continues into the normal sign-in flow. + */ +class SiteSelectionFragment : Fragment() { + + private val viewModel: SiteSelectionViewModel by viewModel() + private val router: AuthRouter by inject() + private val config: Config by inject() + + override fun onCreateView( + inflater: LayoutInflater, + container: ViewGroup?, + savedInstanceState: Bundle?, + ) = ComposeView(requireContext()).apply { + setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed) + setContent { + OpenEdXTheme { + val state by viewModel.uiState.collectAsState() + + LaunchedEffect(Unit) { + viewModel.actions.collect { action -> + when (action) { + is SiteSelectionViewModel.SiteSelectionAction.Success -> + continueAfterSelection(action.preLoginDiscovery) + } + } + } + + SiteSelectionScreen( + state = state, + callbacks = SiteSelectionCallbacks( + onPlatformSelected = viewModel::onPlatformSelected, + onRetry = viewModel::retry, + ) + ) + } + } + } + + private fun continueAfterSelection(preLoginDiscovery: Boolean) { + val fm = requireActivity().supportFragmentManager + when { + // The chosen platform starts on course Discovery — open that instead of + // sign-in (native or webview per config), matching iOS. + preLoginDiscovery -> if (config.getDiscoveryConfig().isViewTypeWebView()) { + router.navigateToWebDiscoverCourses(fm, querySearch = "") + } else { + router.navigateToNativeDiscoverCourses(fm, querySearch = "") + } + + config.isPreLoginExperienceEnabled() -> router.navigateToLogistration(fm, courseId = null) + else -> router.navigateToSignIn(fm, courseId = null, infoType = null) + } + } +} diff --git a/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionScreen.kt b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionScreen.kt new file mode 100644 index 000000000..5c3854577 --- /dev/null +++ b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionScreen.kt @@ -0,0 +1,246 @@ +package org.openedx.auth.presentation.lmsselection + +import androidx.compose.foundation.BorderStroke +import androidx.compose.foundation.background +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.navigationBarsPadding +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.width +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.foundation.verticalScroll +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.automirrored.filled.KeyboardArrowRight +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Scaffold +import androidx.compose.material3.Surface +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.layout.ContentScale +import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.unit.dp +import coil.compose.AsyncImage +import coil.request.ImageRequest +import org.openedx.auth.R +import org.openedx.core.lmsdirectory.LmsImageSource +import org.openedx.core.lmsdirectory.LmsSummary +import org.openedx.core.lmsdirectory.LmsThemeController +import org.openedx.core.ui.theme.appColors +import org.openedx.core.ui.theme.appShapes +import org.openedx.core.ui.theme.appTypography + +@Composable +internal fun SiteSelectionScreen( + state: SiteSelectionUIState, + callbacks: SiteSelectionCallbacks, +) { + val scrollState = rememberScrollState() + + Scaffold( + modifier = Modifier + .fillMaxSize() + .navigationBarsPadding(), + containerColor = MaterialTheme.appColors.background, + topBar = { + Surface(color = MaterialTheme.appColors.background) { + Box( + modifier = Modifier + .fillMaxWidth() + .padding(vertical = 12.dp), + contentAlignment = Alignment.Center, + ) { + Text( + text = stringResource(id = R.string.auth_lms_curated_title), + style = MaterialTheme.appTypography.titleMedium, + color = MaterialTheme.appColors.textPrimary, + ) + } + } + } + ) { padding -> + Column( + modifier = Modifier + .padding(padding) + .padding(horizontal = 24.dp, vertical = 16.dp) + .verticalScroll(scrollState), + verticalArrangement = Arrangement.spacedBy(16.dp), + ) { + val context = LocalContext.current + LaunchedEffect(state.imageReferences) { + // Decoding these now is the whole reason the branded sign-in appears + // whole instead of assembling itself after the platform is tapped. + LmsImageSource.prefetch(context, state.imageReferences) + } + + if (state.providerName.isNotBlank()) { + Text( + text = stringResource(id = R.string.auth_lms_provider_subtitle, state.providerName), + style = MaterialTheme.appTypography.labelLarge, + color = MaterialTheme.appColors.textFieldHint, + ) + } + + when (val catalog = state.catalog) { + is CatalogState.Loading -> LoadingRow() + + is CatalogState.Loaded -> state.platforms.forEach { item -> + CatalogRow(item) { callbacks.onPlatformSelected(item) } + } + + is CatalogState.Empty -> Message(stringResource(id = R.string.auth_lms_empty)) + + is CatalogState.Error -> { + Message(catalog.message) + TextButton(onClick = callbacks.onRetry) { + Text( + text = stringResource(id = R.string.auth_lms_retry), + style = MaterialTheme.appTypography.labelLarge, + color = MaterialTheme.appColors.primary, + ) + } + } + } + } + } +} + +@Composable +private fun Message(text: String) { + Text( + text = text, + style = MaterialTheme.appTypography.bodyMedium, + color = MaterialTheme.appColors.textFieldHint, + ) +} + +@Composable +private fun LoadingRow() { + Box( + modifier = Modifier.fillMaxWidth().padding(vertical = 24.dp), + contentAlignment = Alignment.Center, + ) { + CircularProgressIndicator(color = MaterialTheme.appColors.primary) + } +} + +@Composable +private fun CatalogRow(item: LmsSummary, onSelect: () -> Unit) { + CatalogRow( + title = item.title, + shortDescription = item.shortDescription, + baseUrl = item.baseUrl, + logoUrl = item.logoUrl, + accentColor = item.accentColor, + onSelect = onSelect, + ) +} + +@Composable +private fun CatalogRow( + title: String, + shortDescription: String, + baseUrl: String, + logoUrl: String?, + accentColor: String?, + onSelect: () -> Unit, +) { + Surface( + modifier = Modifier + .fillMaxWidth() + .clickable { onSelect() }, + shape = MaterialTheme.appShapes.textFieldShape, + color = MaterialTheme.appColors.background, + border = BorderStroke(1.dp, MaterialTheme.appColors.textFieldBorder.copy(alpha = 0.5f)), + ) { + Row( + modifier = Modifier.padding(start = 12.dp, top = 10.dp, bottom = 10.dp, end = 4.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + LmsRowLogo(logoUrl = logoUrl, title = title, accentColor = accentColor) + Spacer(modifier = Modifier.width(12.dp)) + Column(modifier = Modifier.weight(1f)) { + Text( + text = title, + maxLines = 1, + style = MaterialTheme.appTypography.titleSmall, + color = MaterialTheme.appColors.textPrimary, + ) + if (shortDescription.isNotBlank()) { + Text( + text = shortDescription, + maxLines = 1, + style = MaterialTheme.appTypography.bodyMedium, + color = MaterialTheme.appColors.textSecondary, + ) + } + Text( + text = hostOf(baseUrl), + maxLines = 1, + style = MaterialTheme.appTypography.labelMedium, + color = MaterialTheme.appColors.textSecondary, + ) + } + Icon( + imageVector = Icons.AutoMirrored.Filled.KeyboardArrowRight, + contentDescription = null, + tint = MaterialTheme.appColors.primary, + ) + } + } +} + +/** + * Platform logo for a catalog row. Loads the LMS's logo when available; otherwise + * falls back to a colored initial badge tinted with the platform's accent color — + * mirroring the iOS directory rows. + */ +@Composable +private fun LmsRowLogo(logoUrl: String?, title: String, accentColor: String?) { + val logoModifier = Modifier + .size(48.dp) + .clip(RoundedCornerShape(10.dp)) + val logoModel = LmsImageSource.model(logoUrl) + if (logoModel != null) { + AsyncImage( + model = ImageRequest.Builder(LocalContext.current) + .data(logoModel) + .crossfade(true) + .build(), + contentDescription = null, + contentScale = ContentScale.Fit, + modifier = logoModifier, + ) + } else { + val accent = LmsThemeController.parseHexColor(accentColor) ?: MaterialTheme.appColors.primary + Box( + modifier = logoModifier.background(accent.copy(alpha = 0.15f)), + contentAlignment = Alignment.Center, + ) { + Text( + text = title.trim().take(1).uppercase(), + style = MaterialTheme.appTypography.titleMedium, + color = accent, + ) + } + } +} + +private fun hostOf(url: String): String { + return url.removePrefix("https://").removePrefix("http://").trimEnd('/') +} diff --git a/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionUIState.kt b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionUIState.kt new file mode 100644 index 000000000..e253444af --- /dev/null +++ b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionUIState.kt @@ -0,0 +1,24 @@ +package org.openedx.auth.presentation.lmsselection + +import org.openedx.core.lmsdirectory.LmsSummary + +data class SiteSelectionUIState( + /** The publisher's own name, shown above the list. Blank when they gave none. */ + val providerName: String = "", + val platforms: List = emptyList(), + val catalog: CatalogState = CatalogState.Loading, + + /** + * Every image the directory will ask for. The whole list arrives at once, so + * these are known before a platform is picked; the screen warms them so the + * branded sign-in does not assemble itself in front of the learner. + */ + val imageReferences: List = emptyList(), +) + +sealed interface CatalogState { + data object Loading : CatalogState + data object Loaded : CatalogState + data object Empty : CatalogState + data class Error(val message: String) : CatalogState +} diff --git a/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionViewModel.kt b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionViewModel.kt new file mode 100644 index 000000000..e500981dc --- /dev/null +++ b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionViewModel.kt @@ -0,0 +1,142 @@ +package org.openedx.auth.presentation.lmsselection + +import androidx.lifecycle.viewModelScope +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharedFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asSharedFlow +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch +import okhttp3.HttpUrl +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull +import org.openedx.auth.R +import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.lmsdirectory.LmsDirectoryRepository +import org.openedx.core.lmsdirectory.LmsSummary +import org.openedx.core.lmsdirectory.LmsThemeController +import org.openedx.foundation.presentation.BaseViewModel +import org.openedx.foundation.system.ResourceManager + +/** + * Drives the platform picker: show what the directory lists, and make the chosen + * platform the one the app talks to — its host, its OAuth client, its branding — + * then signal the fragment to continue to sign-in. + */ +class SiteSelectionViewModel( + private val corePreferences: CorePreferences, + private val resourceManager: ResourceManager, + private val directoryRepository: LmsDirectoryRepository, +) : BaseViewModel(resourceManager) { + + private val _uiState = MutableStateFlow(SiteSelectionUIState()) + val uiState: StateFlow = _uiState + + private val _actions = MutableSharedFlow() + val actions: SharedFlow = _actions.asSharedFlow() + + init { + loadPlatforms() + } + + fun retry() = loadPlatforms() + + private fun loadPlatforms() { + _uiState.update { it.copy(catalog = CatalogState.Loading) } + viewModelScope.launch { + directoryRepository.platforms() + .onSuccess { items -> + _uiState.update { + it.copy( + platforms = items, + providerName = directoryRepository.providerName(), + catalog = if (items.isEmpty()) CatalogState.Empty else CatalogState.Loaded, + // The whole list is in hand, so every logo and sign-in + // background is known before anything is tapped. + imageReferences = directoryRepository.imageReferences(), + ) + } + } + .onFailure { + _uiState.update { state -> + state.copy( + catalog = CatalogState.Error( + resourceManager.getString(R.string.auth_lms_error_catalog) + ) + ) + } + } + } + } + + fun onPlatformSelected(item: LmsSummary) { + viewModelScope.launch { + // The summary carries no OAuth client id, and sign-in needs the + // platform's own registered mobile client to work. + val detail = directoryRepository.detail(item.id).getOrNull() + val normalized = normalizeUrl(detail?.baseUrl ?: item.baseUrl) + if (normalized == null) { + _uiState.update { + it.copy( + catalog = CatalogState.Error( + resourceManager.getString(R.string.auth_lms_error_invalid_url) + ) + ) + } + return@launch + } + selectLms( + baseUrl = normalized.newBuilder().encodedPath("/").build().toString(), + accentColor = detail?.accentColor ?: item.accentColor, + oauthClientId = detail?.oauthClientId, + feedbackEmail = detail?.feedbackEmail, + logoUrl = detail?.logoUrl ?: item.logoUrl, + title = detail?.title ?: item.title, + loginBackgroundUrl = detail?.loginBackgroundUrl, + ) + _actions.emit(SiteSelectionAction.Success(detail?.preLoginDiscovery ?: false)) + } + } + + /** + * Make this platform the one the app talks to. + * + * Everything the rest of the app needs about the chosen platform is written + * here, because from this point on nothing else knows a directory existed. + */ + private fun selectLms( + baseUrl: String, + accentColor: String?, + oauthClientId: String?, + feedbackEmail: String?, + logoUrl: String?, + title: String?, + loginBackgroundUrl: String?, + ) { + corePreferences.selectedBaseUrl = baseUrl + corePreferences.selectedLmsAccentColor = accentColor + corePreferences.selectedOAuthClientId = oauthClientId + corePreferences.selectedFeedbackEmail = feedbackEmail + corePreferences.selectedLmsLogoUrl = logoUrl + corePreferences.selectedLmsTitle = title + corePreferences.selectedLmsLoginBackgroundUrl = loginBackgroundUrl + LmsThemeController.apply(accentColor) + LmsThemeController.applyBackground(loginBackgroundUrl) + } + + sealed interface SiteSelectionAction { + /** [preLoginDiscovery] true -> open the pre-login catalog instead of sign-in. */ + data class Success(val preLoginDiscovery: Boolean) : SiteSelectionAction + } + + private fun normalizeUrl(text: String): HttpUrl? { + val trimmed = text.trim() + if (trimmed.isEmpty()) return null + val withScheme = if (trimmed.startsWith("http://") || trimmed.startsWith("https://")) { + trimmed + } else { + "https://$trimmed" + } + return withScheme.toHttpUrlOrNull() + } +} diff --git a/auth/src/main/java/org/openedx/auth/presentation/restore/RestorePasswordFragment.kt b/auth/src/main/java/org/openedx/auth/presentation/restore/RestorePasswordFragment.kt index beebf4eaa..a3cbb0c9e 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/restore/RestorePasswordFragment.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/restore/RestorePasswordFragment.kt @@ -4,7 +4,6 @@ import android.content.res.Configuration import android.os.Bundle import android.view.LayoutInflater import android.view.ViewGroup -import androidx.compose.foundation.Image import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -41,7 +40,6 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color -import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.ComposeView import androidx.compose.ui.platform.LocalSoftwareKeyboardController import androidx.compose.ui.platform.ViewCompositionStrategy @@ -64,6 +62,7 @@ import org.openedx.core.R import org.openedx.core.presentation.global.appupgrade.AppUpgradeRequiredScreen import org.openedx.core.ui.BackBtn import org.openedx.core.ui.HandleUIMessage +import org.openedx.core.ui.LmsHeaderImage import org.openedx.core.ui.OpenEdXButton import org.openedx.core.ui.displayCutoutForLandscape import org.openedx.core.ui.statusBarsInset @@ -186,13 +185,10 @@ private fun RestorePasswordScreen( ) } - Image( + LmsHeaderImage( modifier = Modifier .fillMaxWidth() - .height(200.dp), - painter = painterResource(id = R.drawable.core_top_header), - contentScale = ContentScale.FillBounds, - contentDescription = null + .height(200.dp) ) HandleUIMessage(uiMessage = uiMessage, snackbarHostState = snackbarHostState) diff --git a/auth/src/main/java/org/openedx/auth/presentation/signin/SignInFragment.kt b/auth/src/main/java/org/openedx/auth/presentation/signin/SignInFragment.kt index fc72523a8..2921d0e1f 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/signin/SignInFragment.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/signin/SignInFragment.kt @@ -71,6 +71,12 @@ class SignInFragment : Fragment() { requireActivity().supportFragmentManager.popBackStackImmediate() } + AuthEvent.ChangeLmsClick -> { + viewModel.navigateToLmsSelection( + requireActivity().supportFragmentManager + ) + } + is AuthEvent.OpenLink -> viewModel.openLink( parentFragmentManager, event.links, @@ -124,4 +130,5 @@ internal sealed interface AuthEvent { object RegisterClick : AuthEvent object ForgotPasswordClick : AuthEvent object BackClick : AuthEvent + object ChangeLmsClick : AuthEvent } diff --git a/auth/src/main/java/org/openedx/auth/presentation/signin/SignInUIState.kt b/auth/src/main/java/org/openedx/auth/presentation/signin/SignInUIState.kt index f7b56084c..95337b51c 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/signin/SignInUIState.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/signin/SignInUIState.kt @@ -26,4 +26,9 @@ internal data class SignInUIState( val showProgress: Boolean = false, val loginSuccess: Boolean = false, val agreement: RegistrationField? = null, + // LMS Directory: branding of the platform the learner picked (null when the + // feature is off or nothing selected — sign-in then shows the app's own logo). + val selectedLmsTitle: String? = null, + val selectedLmsLogoUrl: String? = null, + val selectedLmsLoginBackgroundUrl: String? = null, ) diff --git a/auth/src/main/java/org/openedx/auth/presentation/signin/SignInViewModel.kt b/auth/src/main/java/org/openedx/auth/presentation/signin/SignInViewModel.kt index e7053f2c6..8e22114c8 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/signin/SignInViewModel.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/signin/SignInViewModel.kt @@ -74,6 +74,21 @@ class SignInViewModel( isLogistrationEnabled = config.isPreLoginExperienceEnabled(), isRegistrationEnabled = config.isRegistrationEnabled(), agreement = agreementProvider.getAgreement(isSignIn = true)?.createHonorCodeField(), + selectedLmsTitle = if (config.getLMSDirectoryConfig().isReachable) { + preferencesManager.selectedLmsTitle + } else { + null + }, + selectedLmsLogoUrl = if (config.getLMSDirectoryConfig().isReachable) { + preferencesManager.selectedLmsLogoUrl + } else { + null + }, + selectedLmsLoginBackgroundUrl = if (config.getLMSDirectoryConfig().isReachable) { + preferencesManager.selectedLmsLoginBackgroundUrl + } else { + null + }, ) ) internal val uiState: StateFlow = _uiState @@ -203,6 +218,10 @@ class SignInViewModel( logEvent(AuthAnalyticsEvent.REGISTER_CLICKED) } + fun navigateToLmsSelection(parentFragmentManager: FragmentManager) { + router.navigateToLmsSelection(parentFragmentManager) + } + fun navigateToForgotPassword(parentFragmentManager: FragmentManager) { router.navigateToRestorePassword(parentFragmentManager) logEvent(AuthAnalyticsEvent.FORGOT_PASSWORD_CLICKED) diff --git a/auth/src/main/java/org/openedx/auth/presentation/signin/compose/SignInView.kt b/auth/src/main/java/org/openedx/auth/presentation/signin/compose/SignInView.kt index f5b9bc867..80b1c7123 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/signin/compose/SignInView.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/signin/compose/SignInView.kt @@ -2,8 +2,10 @@ package org.openedx.auth.presentation.signin.compose import android.content.res.Configuration.UI_MODE_NIGHT_NO import android.content.res.Configuration.UI_MODE_NIGHT_YES +import androidx.compose.foundation.BorderStroke import androidx.compose.foundation.Image import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row @@ -13,6 +15,7 @@ import androidx.compose.foundation.layout.fillMaxHeight import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.navigationBarsPadding import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.statusBarsPadding @@ -41,6 +44,7 @@ import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.ContentScale +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalFocusManager import androidx.compose.ui.platform.LocalSoftwareKeyboardController import androidx.compose.ui.platform.testTag @@ -59,6 +63,8 @@ import androidx.compose.ui.tooling.preview.Devices import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp +import coil.compose.AsyncImage +import coil.request.ImageRequest import org.openedx.auth.R import org.openedx.auth.presentation.signin.AuthEvent import org.openedx.auth.presentation.signin.SignInUIState @@ -133,15 +139,30 @@ internal fun LoginScreen( ) } - Image( - modifier = - Modifier + // LMS Directory: brand the header background with the selected platform's own + // login background image, falling back to the default gradient header. + if (!state.selectedLmsLoginBackgroundUrl.isNullOrBlank()) { + AsyncImage( + model = ImageRequest.Builder(LocalContext.current) + .data(state.selectedLmsLoginBackgroundUrl) + .crossfade(true) + .build(), + modifier = Modifier .fillMaxWidth() .fillMaxHeight(fraction = 0.3f), - painter = painterResource(id = coreR.drawable.core_top_header), - contentScale = ContentScale.FillBounds, - contentDescription = null, - ) + contentScale = ContentScale.FillBounds, + contentDescription = null + ) + } else { + Image( + modifier = Modifier + .fillMaxWidth() + .fillMaxHeight(fraction = 0.3f), + painter = painterResource(id = coreR.drawable.core_top_header), + contentScale = ContentScale.FillBounds, + contentDescription = null + ) + } HandleUIMessage(uiMessage = uiMessage, snackbarHostState = snackbarHostState) if (state.isLogistrationEnabled) { Box( @@ -163,7 +184,29 @@ internal fun LoginScreen( Modifier.padding(it), horizontalAlignment = Alignment.CenterHorizontally, ) { - SignInLogoView() + // LMS Directory: brand the header with the selected platform's logo. + if (!state.selectedLmsLogoUrl.isNullOrBlank()) { + Box( + modifier = Modifier + .fillMaxWidth() + .fillMaxHeight(fraction = 0.2f), + contentAlignment = Alignment.Center + ) { + AsyncImage( + model = ImageRequest.Builder(LocalContext.current) + .data(state.selectedLmsLogoUrl) + .crossfade(true) + .build(), + contentDescription = null, + contentScale = ContentScale.Fit, + modifier = Modifier + .padding(top = 20.dp, start = 24.dp, end = 24.dp) + .heightIn(max = 80.dp) + ) + } + } else { + SignInLogoView() + } Surface( color = MaterialTheme.appColors.background, shape = MaterialTheme.appShapes.screenBackgroundShape, @@ -197,7 +240,13 @@ internal fun LoginScreen( style = MaterialTheme.appTypography.titleSmall, ) } - + if (!state.selectedLmsTitle.isNullOrBlank()) { + Spacer(modifier = Modifier.height(16.dp)) + SelectedLmsBanner( + title = state.selectedLmsTitle, + onChange = { onEvent(AuthEvent.ChangeLmsClick) }, + ) + } Spacer(modifier = Modifier.height(24.dp)) AuthForm( buttonWidth, @@ -533,3 +582,51 @@ private fun SignInScreenTabletPreview() { ) } } + +@Composable +private fun SelectedLmsBanner( + title: String, + onChange: () -> Unit, +) { + Surface( + modifier = Modifier + .fillMaxWidth() + .testTag("selected_lms_container"), + shape = MaterialTheme.appShapes.textFieldShape, + color = MaterialTheme.appColors.background, + border = BorderStroke(1.dp, MaterialTheme.appColors.textFieldBorder.copy(alpha = 0.5f)), + ) { + Row( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp, vertical = 12.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Column( + modifier = Modifier.weight(1f), + verticalArrangement = Arrangement.spacedBy(4.dp), + ) { + Text( + text = stringResource(id = R.string.auth_lms_selected_label), + style = MaterialTheme.appTypography.labelMedium, + color = MaterialTheme.appColors.textSecondary, + ) + Text( + text = title, + maxLines = 1, + style = MaterialTheme.appTypography.bodyLarge, + color = MaterialTheme.appColors.textPrimary, + ) + } + Text( + modifier = Modifier + .noRippleClickable { onChange() } + .padding(start = 12.dp) + .testTag("change_lms_button"), + text = stringResource(id = R.string.auth_lms_change), + style = MaterialTheme.appTypography.labelLarge, + color = MaterialTheme.appColors.primary, + ) + } + } +} diff --git a/auth/src/main/java/org/openedx/auth/presentation/signup/compose/SignUpView.kt b/auth/src/main/java/org/openedx/auth/presentation/signup/compose/SignUpView.kt index 5354a081a..1ca3026c8 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/signup/compose/SignUpView.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/signup/compose/SignUpView.kt @@ -3,7 +3,6 @@ package org.openedx.auth.presentation.signup.compose import android.content.res.Configuration import androidx.compose.animation.AnimatedVisibility import androidx.compose.animation.core.tween -import androidx.compose.foundation.Image import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -45,12 +44,10 @@ import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.hapticfeedback.HapticFeedbackType -import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalFocusManager import androidx.compose.ui.platform.LocalHapticFeedback import androidx.compose.ui.platform.LocalSoftwareKeyboardController import androidx.compose.ui.platform.testTag -import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.semantics import androidx.compose.ui.semantics.testTagsAsResourceId @@ -73,6 +70,7 @@ import org.openedx.core.domain.model.RegistrationField import org.openedx.core.domain.model.RegistrationFieldType import org.openedx.core.ui.BackBtn import org.openedx.core.ui.HandleUIMessage +import org.openedx.core.ui.LmsHeaderImage import org.openedx.core.ui.OpenEdXButton import org.openedx.core.ui.SheetContent import org.openedx.core.ui.displayCutoutForLandscape @@ -234,13 +232,10 @@ internal fun SignUpView( } } - Image( + LmsHeaderImage( modifier = Modifier .fillMaxWidth() - .fillMaxHeight(fraction = 0.3f), - painter = painterResource(id = coreR.drawable.core_top_header), - contentScale = ContentScale.FillBounds, - contentDescription = null + .fillMaxHeight(fraction = 0.3f) ) HandleUIMessage(uiMessage = uiMessage, snackbarHostState = snackbarHostState) Column( diff --git a/auth/src/main/res/values/strings.xml b/auth/src/main/res/values/strings.xml index 9838dab6b..d270e7af7 100644 --- a/auth/src/main/res/values/strings.xml +++ b/auth/src/main/res/values/strings.xml @@ -44,4 +44,14 @@ %2$s]]> Show password Hide password + + + Choose your platform + Enter a valid platform address. + Platforms published by %1$s + This directory lists no platforms yet. + Try again + We couldn\'t load the list of platforms. + Selected LMS + Change diff --git a/auth/src/test/java/org/openedx/auth/presentation/lmsselection/SiteSelectionViewModelTest.kt b/auth/src/test/java/org/openedx/auth/presentation/lmsselection/SiteSelectionViewModelTest.kt new file mode 100644 index 000000000..5233ae49f --- /dev/null +++ b/auth/src/test/java/org/openedx/auth/presentation/lmsselection/SiteSelectionViewModelTest.kt @@ -0,0 +1,143 @@ +package org.openedx.auth.presentation.lmsselection + +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.toList +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.lmsdirectory.LmsDetail +import org.openedx.core.lmsdirectory.LmsDirectoryRepository +import org.openedx.core.lmsdirectory.LmsSummary +import org.openedx.core.lmsdirectory.LmsThemeController +import org.openedx.foundation.system.ResourceManager + +/** + * The picker lists what the document holds, and choosing one platform makes the + * app talk to it: its host, its OAuth client, its brand. Everything downstream + * reads those, so what this writes is the whole point of the screen. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class SiteSelectionViewModelTest { + + private val dispatcher = StandardTestDispatcher() + private val corePreferences = mockk(relaxed = true) + private val resourceManager = mockk(relaxed = true) + private val repository = mockk(relaxed = true) + + private val summary = LmsSummary( + id = "4", + title = "Sandbox Env", + shortDescription = "", + baseUrl = "https://sandbox.openedx.org", + logoUrl = null, + accentColor = "#6a2e7b", + ) + + private fun detail(preLoginDiscovery: Boolean = false) = LmsDetail( + id = "4", + title = "Sandbox Env", + shortDescription = "", + baseUrl = "https://sandbox.openedx.org", + logoUrl = null, + accentColor = "#6a2e7b", + oauthClientId = "client-id", + feedbackEmail = null, + loginBackgroundUrl = null, + preLoginDiscovery = preLoginDiscovery, + ) + + @Before + fun setUp() { + Dispatchers.setMain(dispatcher) + coEvery { repository.platforms() } returns Result.success(listOf(summary)) + coEvery { repository.providerName() } returns "Northwind" + coEvery { repository.imageReferences() } returns emptyList() + } + + @After + fun tearDown() { + Dispatchers.resetMain() + LmsThemeController.clear() + } + + @Test + fun `the directory is listed as the document orders it`() = runTest(dispatcher) { + val viewModel = SiteSelectionViewModel(corePreferences, resourceManager, repository) + advanceUntilIdle() + + assertEquals(CatalogState.Loaded, viewModel.uiState.value.catalog) + assertEquals(listOf("4"), viewModel.uiState.value.platforms.map { it.id }) + assertEquals("Northwind", viewModel.uiState.value.providerName) + } + + @Test + fun `a document with no platforms says so rather than looking broken`() = runTest(dispatcher) { + coEvery { repository.platforms() } returns Result.success(emptyList()) + val viewModel = SiteSelectionViewModel(corePreferences, resourceManager, repository) + advanceUntilIdle() + + assertEquals(CatalogState.Empty, viewModel.uiState.value.catalog) + } + + @Test + fun `a document that cannot be read surfaces as an error`() = runTest(dispatcher) { + coEvery { repository.platforms() } returns Result.failure(IllegalStateException("nope")) + val viewModel = SiteSelectionViewModel(corePreferences, resourceManager, repository) + advanceUntilIdle() + + assertTrue(viewModel.uiState.value.catalog is CatalogState.Error) + } + + @Test + fun `choosing a platform makes the app talk to it`() = runTest(dispatcher) { + val (viewModel, actions) = select(preLoginDiscovery = false) + + // The summary carries no OAuth client id, so the full record has to be read. + coVerify { repository.detail("4") } + verify { corePreferences.selectedBaseUrl = "https://sandbox.openedx.org/" } + verify { corePreferences.selectedOAuthClientId = "client-id" } + verify { corePreferences.selectedLmsTitle = "Sandbox Env" } + assertEquals(1, actions.size) + assertTrue(!(actions.first() as SiteSelectionViewModel.SiteSelectionAction.Success).preLoginDiscovery) + assertEquals(CatalogState.Loaded, viewModel.uiState.value.catalog) + } + + @Test + fun `a platform that opens on discovery routes there instead of sign-in`() = runTest(dispatcher) { + val (_, actions) = select(preLoginDiscovery = true) + + val success = actions.first() as SiteSelectionViewModel.SiteSelectionAction.Success + assertTrue("Discovery platform must route to pre-login Discovery", success.preLoginDiscovery) + } + + private fun kotlinx.coroutines.test.TestScope.select( + preLoginDiscovery: Boolean, + ): Pair> { + coEvery { repository.detail("4") } returns Result.success(detail(preLoginDiscovery)) + val viewModel = SiteSelectionViewModel(corePreferences, resourceManager, repository) + val actions = mutableListOf() + backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) { + viewModel.actions.toList(actions) + } + advanceUntilIdle() + + viewModel.onPlatformSelected(summary) + advanceUntilIdle() + return viewModel to actions + } +} diff --git a/auth/src/test/java/org/openedx/auth/presentation/signin/SignInViewModelTest.kt b/auth/src/test/java/org/openedx/auth/presentation/signin/SignInViewModelTest.kt index ce6044eaf..0ee2a1a9f 100644 --- a/auth/src/test/java/org/openedx/auth/presentation/signin/SignInViewModelTest.kt +++ b/auth/src/test/java/org/openedx/auth/presentation/signin/SignInViewModelTest.kt @@ -32,6 +32,7 @@ import org.openedx.core.Validator import org.openedx.core.config.Config import org.openedx.core.config.FacebookConfig import org.openedx.core.config.GoogleConfig +import org.openedx.core.config.LMSDirectoryConfig import org.openedx.core.config.MicrosoftConfig import org.openedx.core.data.storage.CalendarPreferences import org.openedx.core.data.storage.CorePreferences @@ -91,6 +92,7 @@ class SignInViewModelTest { every { appNotifier.notifier } returns emptyFlow() every { agreementProvider.getAgreement(true) } returns null every { config.isPreLoginExperienceEnabled() } returns false + every { config.getLMSDirectoryConfig() } returns LMSDirectoryConfig() every { config.isSocialAuthEnabled() } returns false every { config.getFacebookConfig() } returns FacebookConfig() every { config.getGoogleConfig() } returns GoogleConfig() diff --git a/core/build.gradle b/core/build.gradle index 19be1f57a..67fc46cf7 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -124,6 +124,7 @@ dependencies { debugApi "androidx.compose.ui:ui-tooling:$compose_ui_tooling" testImplementation "junit:junit:$junit_version" + testImplementation "org.jetbrains.kotlinx:kotlinx-coroutines-test:$kotlinx_coroutines_test_version" androidTestImplementation "androidx.test.ext:junit:$test_ext_version" androidTestImplementation "androidx.test.espresso:espresso-core:$espresso_version" } diff --git a/core/src/main/java/org/openedx/core/config/Config.kt b/core/src/main/java/org/openedx/core/config/Config.kt index 1f07f43ad..f81ede613 100644 --- a/core/src/main/java/org/openedx/core/config/Config.kt +++ b/core/src/main/java/org/openedx/core/config/Config.kt @@ -5,11 +5,15 @@ import com.google.gson.Gson import com.google.gson.JsonElement import com.google.gson.JsonObject import com.google.gson.JsonParser +import org.openedx.core.data.storage.CorePreferences import org.openedx.core.domain.model.AgreementUrls import java.io.InputStreamReader @Suppress("TooManyFunctions") -class Config(context: Context) { +class Config( + context: Context, + private val corePreferences: CorePreferences? = null, +) { private var configProperties: JsonObject = try { val inputStream = context.assets.open("config/config.json") @@ -24,10 +28,25 @@ class Config(context: Context) { return getString(APPLICATION_ID, "") } + /** + * The LMS the app talks to. With the LMS Directory feature on and a platform + * picked, that selection wins over the baked-in host; otherwise the config value + * is used. Off (default) → always the config value, i.e. stock behaviour. + */ fun getApiHostURL(): String { + if (getLMSDirectoryConfig().isReachable) { + val selected = corePreferences?.selectedBaseUrl + if (!selected.isNullOrBlank()) { + return selected + } + } return getString(API_HOST_URL) } + fun getLMSDirectoryConfig(): LMSDirectoryConfig { + return getObjectOrNewInstance(LMS_DIRECTORY, LMSDirectoryConfig::class.java) + } + fun getSSOURL(): String { return getString(SSO_URL, "") } @@ -40,6 +59,14 @@ class Config(context: Context) { } fun getOAuthClientId(): String { + // LMS Directory: sign in with the selected platform's own registered mobile + // OAuth client. Off (default) or no selection → the config value. + if (getLMSDirectoryConfig().isReachable) { + val selected = corePreferences?.selectedOAuthClientId + if (!selected.isNullOrBlank()) { + return selected + } + } return getString(OAUTH_CLIENT_ID) } @@ -52,6 +79,12 @@ class Config(context: Context) { } fun getFeedbackEmailAddress(): String { + if (getLMSDirectoryConfig().isReachable) { + val selected = corePreferences?.selectedFeedbackEmail + if (!selected.isNullOrBlank()) { + return selected + } + } return getString(FEEDBACK_EMAIL_ADDRESS) } @@ -217,6 +250,7 @@ class Config(context: Context) { private const val BRANCH = "BRANCH" private const val UI_COMPONENTS = "UI_COMPONENTS" private const val PLATFORM_NAME = "PLATFORM_NAME" + private const val LMS_DIRECTORY = "LMS_DIRECTORY" } enum class ViewType { diff --git a/core/src/main/java/org/openedx/core/config/LMSDirectoryConfig.kt b/core/src/main/java/org/openedx/core/config/LMSDirectoryConfig.kt new file mode 100644 index 000000000..145a18ff8 --- /dev/null +++ b/core/src/main/java/org/openedx/core/config/LMSDirectoryConfig.kt @@ -0,0 +1,56 @@ +package org.openedx.core.config + +import com.google.gson.annotations.SerializedName + +/** + * Feature flag for the multi-tenant LMS Directory: a build that lets a learner + * choose which Open edX platform to sign in to. Off by default — the app then + * behaves as a stock single-tenant build. + */ +data class LMSDirectoryConfig( + @SerializedName("ENABLED") + val enabled: Boolean = false, + + /** Address of a JSON document listing the platforms this build offers. */ + @SerializedName("DIRECTORY_URL") + val directoryUrl: String = "", + + /** + * The same document, in the app's assets, e.g. "lms_directory.json". Set it + * and the app reads its platform list from there and never asks the network. + */ + @SerializedName("DIRECTORY_FILE") + val directoryFile: String = "", +) { + + /** + * Where the document comes from. + * + * A bundled file wins over a URL: a build that ships its own copy has opted out + * of the network, and quietly preferring a remote list would undo that. + */ + sealed interface Source { + /** Read from the app's assets. Never touches the network. */ + data class BundledDocument(val fileName: String) : Source + + /** Fetched once, from anywhere the publisher chose to put it. */ + data class Document(val url: String) : Source + } + + val source: Source? + get() { + if (!enabled) return null + val file = directoryFile.trim() + if (file.isNotEmpty()) return Source.BundledDocument(file) + val url = directoryUrl.trim() + return if (url.isEmpty()) null else Source.Document(url) + } + + /** + * The single gate for activating any LMS Directory behaviour. An ENABLED:true + * build with nothing configured stays fully single-tenant rather than starting + * a feature that has no list to show. + */ + val isReachable: Boolean + get() = enabled && (directoryFile.isNotBlank() || directoryUrl.isNotBlank()) +} diff --git a/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt b/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt index 9e42a5273..0e85da5bc 100644 --- a/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt +++ b/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt @@ -15,5 +15,30 @@ interface CorePreferences { var canResetAppDirectory: Boolean var isRelativeDatesEnabled: Boolean + /** + * Base URL of the LMS the learner picked in the LMS Directory, or null when + * none is selected (or the feature is off). When set, [org.openedx.core.config.Config.getApiHostURL] + * returns this instead of the baked-in host. + */ + var selectedBaseUrl: String? + + /** Accent color (hex, e.g. "#f15d49") of the selected LMS, used to re-theme the app. */ + var selectedLmsAccentColor: String? + + /** OAuth mobile client id of the selected LMS. Sign-in uses this instead of the config value. */ + var selectedOAuthClientId: String? + + /** Feedback email of the selected LMS. */ + var selectedFeedbackEmail: String? + + /** Logo URL of the selected LMS, shown on the sign-in screen. */ + var selectedLmsLogoUrl: String? + + /** Login background image URL of the selected LMS, shown behind the sign-in header. */ + var selectedLmsLoginBackgroundUrl: String? + + /** Human title of the selected LMS, shown in the sign-in "Change" banner. */ + var selectedLmsTitle: String? + suspend fun clearCorePreferences() } diff --git a/core/src/main/java/org/openedx/core/lmsdirectory/DirectoryModels.kt b/core/src/main/java/org/openedx/core/lmsdirectory/DirectoryModels.kt new file mode 100644 index 000000000..aa3446920 --- /dev/null +++ b/core/src/main/java/org/openedx/core/lmsdirectory/DirectoryModels.kt @@ -0,0 +1,33 @@ +package org.openedx.core.lmsdirectory + +/** + * What the app knows about a platform: enough to list it, and enough to become it. + */ + +data class LmsSummary( + val id: String, + val title: String, + val shortDescription: String, + val baseUrl: String, + val logoUrl: String?, + val accentColor: String?, +) + +/** + * Full record for one platform, fetched when the learner picks it. Carries the + * per-LMS OAuth client id and feedback email needed to actually sign in against it, + * plus branding (logo, accent) — the catalog summary alone can't log you in. + */ +data class LmsDetail( + val id: String, + val title: String, + val shortDescription: String = "", + val baseUrl: String, + val logoUrl: String?, + val accentColor: String?, + val oauthClientId: String?, + val feedbackEmail: String?, + val loginBackgroundUrl: String?, + /** When true, the app opens the pre-login course Discovery screen instead of sign-in. */ + val preLoginDiscovery: Boolean = false, +) diff --git a/core/src/main/java/org/openedx/core/lmsdirectory/LmsDetailDto.kt b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDetailDto.kt new file mode 100644 index 000000000..a15d179f4 --- /dev/null +++ b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDetailDto.kt @@ -0,0 +1,44 @@ +package org.openedx.core.lmsdirectory + +import com.google.gson.annotations.SerializedName + +/** Wire format of one platform inside the directory document. */ + +data class LmsDetailDto( + @SerializedName("id") val id: String, + @SerializedName("title") val title: String, + @SerializedName("short_description") val shortDescription: String? = null, + @SerializedName("base_url") val baseUrl: String, + @SerializedName("logo_url") val logoUrl: String? = null, + @SerializedName("accent_color") val accentColor: String? = null, + @SerializedName("api") val api: ApiDto? = null, + @SerializedName("theme") val theme: ThemeDto? = null, + @SerializedName("feature_flags") val featureFlags: FeatureFlagsDto? = null, +) { + data class ApiDto( + @SerializedName("host_url") val hostUrl: String? = null, + @SerializedName("oauth_client_id") val oauthClientId: String? = null, + @SerializedName("feedback_email") val feedbackEmail: String? = null, + ) + + data class ThemeDto( + @SerializedName("login_background_url") val loginBackgroundUrl: String? = null, + ) + + data class FeatureFlagsDto( + @SerializedName("pre_login_discovery") val preLoginDiscovery: Boolean = false, + ) + + fun toDomain() = LmsDetail( + id = id, + title = title, + shortDescription = shortDescription.orEmpty(), + baseUrl = api?.hostUrl?.ifBlank { null } ?: baseUrl, + logoUrl = logoUrl, + accentColor = accentColor, + oauthClientId = api?.oauthClientId?.ifBlank { null }, + feedbackEmail = api?.feedbackEmail?.ifBlank { null }, + loginBackgroundUrl = theme?.loginBackgroundUrl?.ifBlank { null }, + preLoginDiscovery = featureFlags?.preLoginDiscovery ?: false, + ) +} diff --git a/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectoryModule.kt b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectoryModule.kt new file mode 100644 index 000000000..c46911c9c --- /dev/null +++ b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectoryModule.kt @@ -0,0 +1,45 @@ +package org.openedx.core.lmsdirectory + +import android.content.Context +import okhttp3.OkHttpClient +import org.koin.core.qualifier.named +import org.koin.dsl.module +import org.openedx.core.config.Config +import org.openedx.core.config.LMSDirectoryConfig +import java.util.concurrent.TimeUnit + +/** + * Koin module for the LMS directory. + * + * The list comes from whichever source the config names: a document to fetch, or + * one shipped in the app's assets. Nothing resolves this unless the feature is + * configured — see [LMSDirectoryConfig.isReachable]. + */ +val lmsDirectoryModule = module { + + single(qualifier = named("LmsDirectory")) { + OkHttpClient.Builder() + .connectTimeout(DIRECTORY_TIMEOUT_SECONDS, TimeUnit.SECONDS) + .readTimeout(DIRECTORY_TIMEOUT_SECONDS, TimeUnit.SECONDS) + .build() + } + + single { + when (val source = get().getLMSDirectoryConfig().source) { + is LMSDirectoryConfig.Source.BundledDocument -> + DocumentLmsDirectorySource.fromAsset(get(), source.fileName) + + is LMSDirectoryConfig.Source.Document -> + DocumentLmsDirectorySource.fromUrl( + get(qualifier = named("LmsDirectory")), + source.url + ) + + null -> error("The LMS directory has no configured source") + } + } + + single { LmsDirectoryRepository(source = get()) } +} + +private const val DIRECTORY_TIMEOUT_SECONDS = 20L diff --git a/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectoryRepository.kt b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectoryRepository.kt new file mode 100644 index 000000000..91f3ff790 --- /dev/null +++ b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectoryRepository.kt @@ -0,0 +1,38 @@ +package org.openedx.core.lmsdirectory + +import android.util.Log + +/** + * Supplies the platform list. Calls return [Result] so a screen can say what went + * wrong instead of showing an empty list and hoping. + * + * [source] decides where the list comes from — a document, hosted or shipped in + * the app. Whoever builds the app decides that; nothing here is told about it. + */ +class LmsDirectoryRepository(private val source: LmsDirectorySource) { + + companion object { + private const val TAG = "LmsDirectory" + } + + /** The publisher's own name, shown above the list. Blank when they gave none. */ + suspend fun providerName(): String = + runCatching { source.providerName() } + .onFailure { Log.w(TAG, "Provider name unavailable: ${it.message}") } + .getOrDefault("") + + suspend fun platforms(): Result> = runCatching { + source.platforms() + }.onFailure { Log.w(TAG, "Could not read the directory: ${it.message}") } + + /** Full record for one platform, including the OAuth client id sign-in needs. */ + suspend fun detail(id: String): Result = runCatching { + source.detail(id) + }.onFailure { Log.w(TAG, "Could not read platform $id: ${it.message}") } + + /** Images the list will need, for warming before the screens that show them. */ + suspend fun imageReferences(): List = + runCatching { source.imageReferences() } + .onFailure { Log.w(TAG, "Could not list directory images: ${it.message}") } + .getOrDefault(emptyList()) +} diff --git a/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectorySource.kt b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectorySource.kt new file mode 100644 index 000000000..0b8b4ea0c --- /dev/null +++ b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectorySource.kt @@ -0,0 +1,138 @@ +package org.openedx.core.lmsdirectory + +import android.content.Context +import android.util.Log +import com.google.gson.Gson +import com.google.gson.annotations.SerializedName +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import okhttp3.OkHttpClient +import okhttp3.Request + +/** + * Where the platform list comes from. + * + * One implementation today — a JSON document, hosted or shipped with the app — + * behind an interface so the screens do not care which of the two they got. + */ +interface LmsDirectorySource { + /** The publisher's own name, shown above the list. Blank when they gave none. */ + suspend fun providerName(): String + + /** Every platform in the directory, in the order the document lists them. */ + suspend fun platforms(): List + + /** Everything needed to re-theme the app and sign in to one of them. */ + suspend fun detail(id: String): LmsDetail + + /** + * Every image the list will ask for, so a caller can warm them before the + * screens that show them are built. + */ + suspend fun imageReferences(): List = emptyList() +} + +/** + * A single JSON document, fetched from a URL or read out of the app's assets. + * + * Read once and kept: the picker, the theming and the image prefetch all work + * from the same copy rather than parsing it three times. Because everything + * arrives together, a platform's sign-in background is known before the learner + * has picked anything, which is what lets the artwork be warmed in advance. + */ +class DocumentLmsDirectorySource( + private val loader: DocumentLoader, + private val gson: Gson = Gson(), +) : LmsDirectorySource { + + /** How the bytes are obtained. Kept separate so tests need no network or app. */ + fun interface DocumentLoader { + suspend fun load(): String + } + + private var cached: DirectoryDocumentDto? = null + + override suspend fun providerName(): String = document().provider?.name.orEmpty() + + override suspend fun platforms(): List = document().platforms.map { it.toSummary() } + + override suspend fun detail(id: String): LmsDetail = + document().platforms.firstOrNull { it.id == id }?.toDomain() + ?: throw NoSuchElementException("No platform with id $id in the directory document") + + override suspend fun imageReferences(): List = + document().platforms.flatMap { + listOfNotNull(it.logoUrl, it.theme?.loginBackgroundUrl) + }.filter { it.isNotBlank() } + + private suspend fun document(): DirectoryDocumentDto { + cached?.let { return it } + val raw = loader.load() + val parsed = gson.fromJson(raw, DirectoryDocumentDto::class.java) + checkNotNull(parsed) { "Directory document is empty" } + if (parsed.platforms.isEmpty()) { + Log.w(TAG, "Directory document parsed but lists no platforms") + } + cached = parsed + return parsed + } + + companion object { + private const val TAG = "LmsDirectory" + + /** Reads a document shipped in the app's assets. Never touches the network. */ + fun fromAsset(context: Context, fileName: String): DocumentLmsDirectorySource = + DocumentLmsDirectorySource( + loader = { + withContext(Dispatchers.IO) { + context.assets.open(fileName).bufferedReader().use { it.readText() } + } + } + ) + + /** Fetches a document over HTTP, once. */ + fun fromUrl(client: OkHttpClient, url: String): DocumentLmsDirectorySource = + DocumentLmsDirectorySource( + loader = { + withContext(Dispatchers.IO) { + client.newCall(Request.Builder().url(url).build()).execute().use { response -> + check(response.isSuccessful) { + "Directory document returned ${response.code}" + } + checkNotNull(response.body?.string()) { + "Directory document had no body" + } + } + } + } + ) + } +} + +/** + * Wire format of the directory document. + * + * `platforms` entries are the same objects `/api/v1/directory/{id}` returns, so + * [LmsDetailDto] is reused rather than duplicated — a document and a live service + * describe a platform identically, and keeping one parser is what guarantees it. + */ +data class DirectoryDocumentDto( + @SerializedName("version") val version: Int = 1, + @SerializedName("provider") val provider: ProviderDto? = null, + @SerializedName("platforms") val platforms: List = emptyList(), +) { + data class ProviderDto( + @SerializedName("name") val name: String? = null, + @SerializedName("tagline") val tagline: String? = null, + @SerializedName("logo_url") val logoUrl: String? = null, + ) +} + +private fun LmsDetailDto.toSummary(): LmsSummary = LmsSummary( + id = id, + title = title, + shortDescription = shortDescription.orEmpty(), + baseUrl = api?.hostUrl?.ifBlank { null } ?: baseUrl, + logoUrl = logoUrl, + accentColor = accentColor, +) diff --git a/core/src/main/java/org/openedx/core/lmsdirectory/LmsImageSource.kt b/core/src/main/java/org/openedx/core/lmsdirectory/LmsImageSource.kt new file mode 100644 index 000000000..2b233c93d --- /dev/null +++ b/core/src/main/java/org/openedx/core/lmsdirectory/LmsImageSource.kt @@ -0,0 +1,51 @@ +package org.openedx.core.lmsdirectory + +import android.content.Context +import coil.ImageLoader +import coil.request.ImageRequest + +/** + * Turns a directory image field into something Coil can load. + * + * The document carries image fields as plain strings. A value that looks like a + * web address is downloaded; anything else is the name of a file shipped in the + * app's assets. That one rule is what lets the same document serve an operator + * who hosts their images and one who bundles them, with no second set of fields + * to keep in step. + */ +object LmsImageSource { + + private const val ASSET_SCHEME = "file:///android_asset/" + + /** + * A Coil model for [value], or null when there is nothing to show. + * + * Coil reads `file:///android_asset/…` natively, so a bundled image needs no + * special case anywhere it is rendered — only here. + */ + fun model(value: String?): String? { + val trimmed = value?.trim().orEmpty() + if (trimmed.isEmpty()) return null + return if (isRemote(trimmed)) trimmed else ASSET_SCHEME + trimmed.trimStart('/') + } + + fun isRemote(value: String): Boolean = + value.startsWith("http://", ignoreCase = true) || + value.startsWith("https://", ignoreCase = true) + + /** + * Warm images before the screens that show them are built. + * + * Worth doing only because the whole directory arrives at once: a platform's + * sign-in background is known while the learner is still choosing, so by the + * time they pick one it is already decoded and the branded screen does not + * visibly assemble itself. + */ + fun prefetch(context: Context, values: List, loader: ImageLoader? = null) { + val imageLoader = loader ?: coil.Coil.imageLoader(context) + values.asSequence() + .mapNotNull { model(it) } + .distinct() + .forEach { imageLoader.enqueue(ImageRequest.Builder(context).data(it).build()) } + } +} diff --git a/core/src/main/java/org/openedx/core/lmsdirectory/LmsThemeController.kt b/core/src/main/java/org/openedx/core/lmsdirectory/LmsThemeController.kt new file mode 100644 index 000000000..5de895521 --- /dev/null +++ b/core/src/main/java/org/openedx/core/lmsdirectory/LmsThemeController.kt @@ -0,0 +1,54 @@ +package org.openedx.core.lmsdirectory + +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.ui.graphics.Color + +/** + * Holds the accent color the app re-themes to when a learner picks an LMS from the + * directory. [OpenEdXTheme][org.openedx.core.ui.theme.OpenEdXTheme] reads [accentColor] + * and, when present, tints the palette's accent-driven surfaces (buttons, primary). + * + * Backed by [mutableStateOf] so setting it recomposes the theme. The app seeds it at + * launch from the persisted selection and updates it the moment a platform is chosen. + */ +object LmsThemeController { + + var accentColor by mutableStateOf(null) + private set + + /** + * The selected platform's login background image URL. Drives the branded header on + * the auth (sign-in/register/reset) and settings screens, mirroring iOS's + * `LmsHeaderBackground`. Null (default build / no custom image) keeps the stock header. + */ + var loginBackgroundUrl by mutableStateOf(null) + private set + + /** Apply a hex color like "#f15d49". Invalid or blank input clears the override. */ + fun apply(hex: String?) { + accentColor = parseHexColor(hex) + } + + /** Apply the selected LMS's login background image URL (blank/null clears it). */ + fun applyBackground(url: String?) { + loginBackgroundUrl = url?.takeIf { it.isNotBlank() } + } + + fun clear() { + accentColor = null + loginBackgroundUrl = null + } + + @Suppress("MagicNumber", "ReturnCount") + fun parseHexColor(hex: String?): Color? { + val raw = hex?.trim()?.removePrefix("#") ?: return null + if (raw.length != 6 && raw.length != 8) return null + val value = raw.toLongOrNull(16) ?: return null + return when (raw.length) { + 6 -> Color(0xFF000000 or value) + else -> Color(value) + } + } +} diff --git a/core/src/main/java/org/openedx/core/ui/ComposeExtensions.kt b/core/src/main/java/org/openedx/core/ui/ComposeExtensions.kt index 1351662eb..77e17b7f2 100644 --- a/core/src/main/java/org/openedx/core/ui/ComposeExtensions.kt +++ b/core/src/main/java/org/openedx/core/ui/ComposeExtensions.kt @@ -37,7 +37,10 @@ import androidx.compose.ui.platform.LocalInspectionMode import androidx.compose.ui.res.painterResource import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp +import coil.compose.rememberAsyncImagePainter +import coil.request.ImageRequest import org.openedx.core.R +import org.openedx.core.lmsdirectory.LmsThemeController import org.openedx.core.presentation.global.InsetHolder const val KEYBOARD_VISIBILITY_THRESHOLD = 0.15f @@ -172,9 +175,24 @@ fun PagerState.calculateCurrentOffsetForPage(page: Int): Float { } fun Modifier.settingsHeaderBackground(): Modifier = composed { + // LMS Directory: brand the header with the selected platform's login background image, + // falling back to the stock gradient header (matches iOS's LmsHeaderBackground). + val backgroundUrl = LmsThemeController.loginBackgroundUrl + val painter = if (!backgroundUrl.isNullOrBlank()) { + rememberAsyncImagePainter( + model = ImageRequest.Builder(LocalContext.current) + .data(backgroundUrl) + .placeholder(R.drawable.core_top_header) + .error(R.drawable.core_top_header) + .crossfade(true) + .build() + ) + } else { + painterResource(id = R.drawable.core_top_header) + } return@composed this .paint( - painter = painterResource(id = R.drawable.core_top_header), + painter = painter, contentScale = ContentScale.FillWidth, alignment = Alignment.TopCenter ) diff --git a/core/src/main/java/org/openedx/core/ui/LmsHeaderImage.kt b/core/src/main/java/org/openedx/core/ui/LmsHeaderImage.kt new file mode 100644 index 000000000..e2f64ea97 --- /dev/null +++ b/core/src/main/java/org/openedx/core/ui/LmsHeaderImage.kt @@ -0,0 +1,48 @@ +package org.openedx.core.ui + +import androidx.compose.foundation.Image +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.layout.ContentScale +import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.res.painterResource +import coil.compose.AsyncImage +import coil.request.ImageRequest +import org.openedx.core.R +import org.openedx.core.lmsdirectory.LmsImageSource +import org.openedx.core.lmsdirectory.LmsThemeController + +/** + * Header image for the auth screens (sign-in / register / reset password). When the LMS + * Directory feature has a selected platform with a custom login background, shows that + * image; otherwise the stock gradient header. Mirrors iOS's `LmsHeaderBackground`, so a + * branded platform looks the same across sign-in, register, reset and the settings screens. + */ +@Composable +fun LmsHeaderImage(modifier: Modifier = Modifier) { + val background = LmsImageSource.model(LmsThemeController.loginBackgroundUrl) + if (background != null) { + AsyncImage( + model = ImageRequest.Builder(LocalContext.current) + .data(background) + .placeholder(R.drawable.core_top_header) + .error(R.drawable.core_top_header) + // No crossfade: the image is prefetched while the learner is still + // choosing a platform, so it is already decoded by the time this is + // built. Fading it in would put back the appearing-image effect that + // prefetching exists to remove. + .crossfade(false) + .build(), + modifier = modifier, + contentScale = ContentScale.FillBounds, + contentDescription = null, + ) + } else { + Image( + modifier = modifier, + painter = painterResource(id = R.drawable.core_top_header), + contentScale = ContentScale.FillBounds, + contentDescription = null, + ) + } +} diff --git a/core/src/main/java/org/openedx/core/ui/theme/Theme.kt b/core/src/main/java/org/openedx/core/ui/theme/Theme.kt index ec7997c72..fce872875 100644 --- a/core/src/main/java/org/openedx/core/ui/theme/Theme.kt +++ b/core/src/main/java/org/openedx/core/ui/theme/Theme.kt @@ -10,6 +10,8 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.ReadOnlyComposable import androidx.compose.runtime.staticCompositionLocalOf +import androidx.compose.ui.graphics.Color +import org.openedx.core.lmsdirectory.LmsThemeController internal val LocalAppColors = staticCompositionLocalOf { error("No AppColors provided") @@ -223,11 +225,14 @@ val MaterialTheme.appColors: AppColors @OptIn(ExperimentalFoundationApi::class) @Composable fun OpenEdXTheme(darkTheme: Boolean = isSystemInDarkTheme(), content: @Composable () -> Unit) { - val colors = if (darkTheme) { + val basePalette = if (darkTheme) { DarkColorPalette } else { LightColorPalette } + // LMS Directory: re-tint accent surfaces to the selected platform's brand color. + // Null (default / stock build) leaves the baked-in palette untouched. + val colors = LmsThemeController.accentColor?.let { basePalette.withAccent(it) } ?: basePalette MaterialTheme( colorScheme = colors.material3, @@ -240,3 +245,33 @@ fun OpenEdXTheme(darkTheme: Boolean = isSystemInDarkTheme(), content: @Composabl ) } } + +/** + * Returns a copy of this palette with the accent-driven surfaces re-tinted to + * [accent] — the primary/secondary buttons, the Material3 primary/tertiary roles, + * the accent text, and every link/interactive accent role (mirroring iOS's + * LMSThemeApplier: accentColor/infoColor, the outlined secondary-button text & + * border, and the toggle switch). This drives the SignIn "Change" / "Register" + * (primary) and "Forgot password" (infoVariant) text links, plus page indicators + * and toggles, so the whole app adopts the platform's brand color. Everything else + * (backgrounds, body text, on-button text, borders) is preserved so the app keeps + * its light/dark identity and text-on-surface readability. + */ +private fun AppColors.withAccent(accent: Color): AppColors { + return copy( + material3 = material3.copy( + primary = accent, + tertiary = accent, + surfaceTint = accent, + ), + textAccent = accent, + primaryButtonBackground = accent, + secondaryButtonBackground = accent, + secondaryButtonBorder = accent, + secondaryButtonBorderedText = accent, + bottomSheetToggle = accent, + info = accent, + infoVariant = accent, + progressBarColor = accent, + ) +} diff --git a/core/src/test/java/org/openedx/core/config/LMSDirectoryConfigTest.kt b/core/src/test/java/org/openedx/core/config/LMSDirectoryConfigTest.kt new file mode 100644 index 000000000..e431e7ee1 --- /dev/null +++ b/core/src/test/java/org/openedx/core/config/LMSDirectoryConfigTest.kt @@ -0,0 +1,63 @@ +package org.openedx.core.config + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Which source a build reads its platform list from is decided entirely by the + * config file, so the rule has to be exactly what the comments in that file say. + */ +class LMSDirectoryConfigTest { + + @Test + fun `an address is fetched as a document, whatever it ends in`() { + assertEquals( + LMSDirectoryConfig.Source.Document("https://example.com/directory.json"), + LMSDirectoryConfig(enabled = true, directoryUrl = "https://example.com/directory.json").source + ) + assertEquals( + LMSDirectoryConfig.Source.Document("https://example.com/directory"), + LMSDirectoryConfig(enabled = true, directoryUrl = "https://example.com/directory").source + ) + } + + @Test + fun `whitespace is not a configured source`() { + assertNull(LMSDirectoryConfig(enabled = true, directoryUrl = " ", directoryFile = " ").source) + } + + @Test + fun `a bundled file wins over an address`() { + // A build shipping its own copy has opted out of the network; quietly + // preferring a remote list would undo that. + assertEquals( + LMSDirectoryConfig.Source.BundledDocument("lms_directory.json"), + LMSDirectoryConfig( + enabled = true, + directoryUrl = "https://example.com/directory.json", + directoryFile = "lms_directory.json", + ).source + ) + } + + @Test + fun `nothing configured means no source and nothing reachable`() { + assertNull(LMSDirectoryConfig(enabled = true).source) + assertFalse(LMSDirectoryConfig(enabled = true).isReachable) + // Off is off, whatever else is filled in. + assertNull( + LMSDirectoryConfig(enabled = false, directoryUrl = "https://example.com/d.json").source + ) + assertFalse( + LMSDirectoryConfig(enabled = false, directoryUrl = "https://example.com/d.json").isReachable + ) + } + + @Test + fun `a bundled file alone is enough to be reachable`() { + assertTrue(LMSDirectoryConfig(enabled = true, directoryFile = "lms_directory.json").isReachable) + } +} diff --git a/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt b/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt new file mode 100644 index 000000000..e61bd70bc --- /dev/null +++ b/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt @@ -0,0 +1,143 @@ +package org.openedx.core.lmsdirectory + +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * A directory read from a single JSON document — hosted or shipped in the app — + * has to behave exactly like one read from a live service, and keep behaving + * that way with no network at all. That is the promise the document makes. + */ +class DocumentLmsDirectorySourceTest { + + private val document = """ + { + "version": 1, + "provider": { "name": "Northwind", "tagline": "Five campuses, one app" }, + "platforms": [ + { + "id": "1", + "title": "Alpha", + "short_description": "Alpha", + "base_url": "https://alpha.example.edu", + "logo_url": "https://cdn.example.com/alpha.png", + "accent_color": "#112233", + "api": { + "host_url": "https://alpha.example.edu", + "oauth_client_id": "alpha-client", + "feedback_email": "support@example.edu" + }, + "feature_flags": { "pre_login_discovery": true }, + "theme": { "login_background_url": "alpha-bg.png" } + }, + { + "id": "2", + "title": "Beta", + "short_description": "Beta", + "base_url": "https://beta.example.edu", + "logo_url": "beta-logo.png", + "api": { + "host_url": "https://beta.example.edu", + "oauth_client_id": "beta-client", + "feedback_email": "" + }, + "feature_flags": { "pre_login_discovery": false } + } + ] + } + """.trimIndent() + + private fun source(payload: String = document, onLoad: () -> Unit = {}) = + DocumentLmsDirectorySource( + loader = { + onLoad() + payload + } + ) + + @Test + fun `every platform is listed in document order`() = runTest { + assertEquals(listOf("Alpha", "Beta"), source().platforms().map { it.title }) + } + + @Test + fun `detail comes from the same copy without loading again`() = runTest { + var loads = 0 + val source = source(onLoad = { loads++ }) + source.platforms() + val detail = source.detail("2") + + assertEquals("Beta", detail.title) + assertEquals("beta-client", detail.oauthClientId) + // Read once and kept: the picker, the theming and the prefetch all work + // from one copy rather than fetching it three times. + assertEquals(1, loads) + } + + @Test + fun `an unknown id is an error rather than a silent empty result`() = runTest { + val source = source() + try { + source.detail("nope") + throw AssertionError("Expected a failure for an unknown id") + } catch (e: NoSuchElementException) { + assertTrue(e.message!!.contains("nope")) + } + } + + @Test + fun `the provider name comes from the document`() = runTest { + assertEquals("Northwind", source().providerName()) + } + + @Test + fun `image references cover logos and sign-in backgrounds`() = runTest { + val refs = source().imageReferences() + assertTrue(refs.contains("https://cdn.example.com/alpha.png")) + assertTrue(refs.contains("alpha-bg.png")) + assertTrue(refs.contains("beta-logo.png")) + } + + @Test + fun `a minimal hand-written document is accepted`() = runTest { + // The smallest document a person could reasonably write. The same file + // has to work on iOS, so anything omitted here must have a default on + // both platforms — not just on this one, where Gson is forgiving. + val minimal = """ + { + "version": 1, + "platforms": [ + { + "id": "1", + "title": "Alpha", + "description": "Alpha campus", + "short_description": "Alpha", + "base_url": "https://alpha.example.edu", + "api": { + "host_url": "https://alpha.example.edu", + "oauth_client_id": "alpha-client", + "feedback_email": "support@example.edu" + } + } + ] + } + """.trimIndent() + + val detail = source(payload = minimal).detail("1") + + assertEquals("Alpha", detail.title) + assertEquals("alpha-client", detail.oauthClientId) + } + + @Test + fun `a document that cannot be parsed fails instead of looking empty`() = runTest { + try { + source(payload = "not json at all").platforms() + throw AssertionError("Expected a parse failure") + } catch (e: Exception) { + assertTrue(e !is AssertionError) + } + } +} diff --git a/core/src/test/java/org/openedx/core/lmsdirectory/LmsImageSourceTest.kt b/core/src/test/java/org/openedx/core/lmsdirectory/LmsImageSourceTest.kt new file mode 100644 index 000000000..3d2ad44fa --- /dev/null +++ b/core/src/test/java/org/openedx/core/lmsdirectory/LmsImageSourceTest.kt @@ -0,0 +1,60 @@ +package org.openedx.core.lmsdirectory + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * One field decides whether an image is downloaded or read out of the app. The + * rule is simple enough to state in a sentence, which is why it needs tests: an + * operator editing the document by hand will lean on it. + */ +class LmsImageSourceTest { + + @Test + fun `web addresses are passed through untouched`() { + assertEquals( + "https://cdn.example.com/logo.png", + LmsImageSource.model("https://cdn.example.com/logo.png") + ) + assertEquals( + "http://cdn.example.com/logo.png", + LmsImageSource.model("http://cdn.example.com/logo.png") + ) + } + + @Test + fun `anything else becomes an asset in the app`() { + assertEquals("file:///android_asset/acme.png", LmsImageSource.model("acme.png")) + assertEquals("file:///android_asset/logos/acme.png", LmsImageSource.model("logos/acme.png")) + // A leading slash is a habit from the hosted form; it must not produce a + // double slash that fails to resolve. + assertEquals("file:///android_asset/acme.png", LmsImageSource.model("/acme.png")) + } + + @Test + fun `surrounding whitespace does not change the answer`() { + assertEquals( + "https://cdn.example.com/logo.png", + LmsImageSource.model(" https://cdn.example.com/logo.png ") + ) + assertEquals("file:///android_asset/acme.png", LmsImageSource.model(" acme.png ")) + } + + @Test + fun `empty and missing values produce nothing`() { + assertNull(LmsImageSource.model(null)) + assertNull(LmsImageSource.model("")) + assertNull(LmsImageSource.model(" ")) + } + + @Test + fun `remote is decided by scheme, not by looking like a URL`() { + assertTrue(LmsImageSource.isRemote("https://a.test/x.png")) + assertTrue(LmsImageSource.isRemote("HTTPS://a.test/x.png")) + assertFalse(LmsImageSource.isRemote("a.test/x.png")) + assertFalse(LmsImageSource.isRemote("x.png")) + } +} diff --git a/default_config/dev/config.yaml b/default_config/dev/config.yaml index f2868eb78..aa6c743df 100644 --- a/default_config/dev/config.yaml +++ b/default_config/dev/config.yaml @@ -94,3 +94,19 @@ UI_COMPONENTS: COURSE_DROPDOWN_NAVIGATION_ENABLED: false COURSE_UNIT_PROGRESS_ENABLED: false COURSE_DOWNLOAD_QUEUE_SCREEN: false +# Multi-tenant: let a learner choose which Open edX platform to sign in to. +# Off by default — with ENABLED false the app is a stock single-tenant build. +# +# The list of platforms is one JSON document. Give it either way, not both: +# +# DIRECTORY_URL: "https://example.com/lms_directory.json" fetched once +# DIRECTORY_FILE: "lms_directory.json" in app/src/main/assets +# +# A bundled file wins over an address: a build that ships its own copy has +# deliberately opted out of the network. Image fields inside the document are +# either web addresses or names of files shipped with the app. +# See Documentation/LMSDirectory.md for the format. +LMS_DIRECTORY: + ENABLED: false + DIRECTORY_URL: "" + DIRECTORY_FILE: "" diff --git a/default_config/prod/config.yaml b/default_config/prod/config.yaml index f2868eb78..aa6c743df 100644 --- a/default_config/prod/config.yaml +++ b/default_config/prod/config.yaml @@ -94,3 +94,19 @@ UI_COMPONENTS: COURSE_DROPDOWN_NAVIGATION_ENABLED: false COURSE_UNIT_PROGRESS_ENABLED: false COURSE_DOWNLOAD_QUEUE_SCREEN: false +# Multi-tenant: let a learner choose which Open edX platform to sign in to. +# Off by default — with ENABLED false the app is a stock single-tenant build. +# +# The list of platforms is one JSON document. Give it either way, not both: +# +# DIRECTORY_URL: "https://example.com/lms_directory.json" fetched once +# DIRECTORY_FILE: "lms_directory.json" in app/src/main/assets +# +# A bundled file wins over an address: a build that ships its own copy has +# deliberately opted out of the network. Image fields inside the document are +# either web addresses or names of files shipped with the app. +# See Documentation/LMSDirectory.md for the format. +LMS_DIRECTORY: + ENABLED: false + DIRECTORY_URL: "" + DIRECTORY_FILE: "" diff --git a/default_config/stage/config.yaml b/default_config/stage/config.yaml index f2868eb78..aa6c743df 100644 --- a/default_config/stage/config.yaml +++ b/default_config/stage/config.yaml @@ -94,3 +94,19 @@ UI_COMPONENTS: COURSE_DROPDOWN_NAVIGATION_ENABLED: false COURSE_UNIT_PROGRESS_ENABLED: false COURSE_DOWNLOAD_QUEUE_SCREEN: false +# Multi-tenant: let a learner choose which Open edX platform to sign in to. +# Off by default — with ENABLED false the app is a stock single-tenant build. +# +# The list of platforms is one JSON document. Give it either way, not both: +# +# DIRECTORY_URL: "https://example.com/lms_directory.json" fetched once +# DIRECTORY_FILE: "lms_directory.json" in app/src/main/assets +# +# A bundled file wins over an address: a build that ships its own copy has +# deliberately opted out of the network. Image fields inside the document are +# either web addresses or names of files shipped with the app. +# See Documentation/LMSDirectory.md for the format. +LMS_DIRECTORY: + ENABLED: false + DIRECTORY_URL: "" + DIRECTORY_FILE: "" From 3303358d856868b97bbf3e6074ca531aaeff3e1f Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Thu, 20 Aug 2026 21:39:02 +0300 Subject: [PATCH 2/5] fix: register the picker's view model, and match the iOS screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The app crashed on launch with the directory configured: Koin had no definition for SiteSelectionViewModel, so the very first screen threw. Unit tests and the build were both green — it took running the app to see it. The rest is design parity, from putting the two screens side by side: - the title sat under the status bar, so the screen needed statusBarsPadding; - secondary text used appColors.textSecondary, which on Android is a light grey (#B3B3B3) while the token of that name on iOS is the slate #3D4964. The colour iOS actually draws is textPrimaryVariant here, so the subtitle, the host, the provider line and the chevron all use that; - the chevron was the accent colour, where iOS draws it in the same secondary ink as the text; - rows padded 12/10/10/4 against iOS's even 16, and the platform name used a title style where iOS uses bodyLarge. --- .../main/java/org/openedx/app/di/ScreenModule.kt | 3 +++ .../lmsselection/SiteSelectionScreen.kt | 16 +++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/openedx/app/di/ScreenModule.kt b/app/src/main/java/org/openedx/app/di/ScreenModule.kt index 1799dafc6..d3710181b 100644 --- a/app/src/main/java/org/openedx/app/di/ScreenModule.kt +++ b/app/src/main/java/org/openedx/app/di/ScreenModule.kt @@ -8,6 +8,7 @@ import org.openedx.app.AppViewModel import org.openedx.app.MainViewModel import org.openedx.auth.data.repository.AuthRepository import org.openedx.auth.domain.interactor.AuthInteractor +import org.openedx.auth.presentation.lmsselection.SiteSelectionViewModel import org.openedx.auth.presentation.logistration.LogistrationViewModel import org.openedx.auth.presentation.restore.RestorePasswordViewModel import org.openedx.auth.presentation.signin.SignInViewModel @@ -107,6 +108,8 @@ val screenModule = module { factory { AuthInteractor(get()) } factory { Validator() } + viewModel { SiteSelectionViewModel(get(), get(), get()) } + viewModel { (courseId: String) -> LogistrationViewModel( courseId, diff --git a/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionScreen.kt b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionScreen.kt index 5c3854577..112f533f0 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionScreen.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionScreen.kt @@ -13,6 +13,7 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.navigationBarsPadding import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.statusBarsPadding import androidx.compose.foundation.layout.width import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.RoundedCornerShape @@ -62,6 +63,7 @@ internal fun SiteSelectionScreen( Box( modifier = Modifier .fillMaxWidth() + .statusBarsPadding() .padding(vertical = 12.dp), contentAlignment = Alignment.Center, ) { @@ -92,7 +94,7 @@ internal fun SiteSelectionScreen( Text( text = stringResource(id = R.string.auth_lms_provider_subtitle, state.providerName), style = MaterialTheme.appTypography.labelLarge, - color = MaterialTheme.appColors.textFieldHint, + color = MaterialTheme.appColors.textPrimaryVariant, ) } @@ -125,7 +127,7 @@ private fun Message(text: String) { Text( text = text, style = MaterialTheme.appTypography.bodyMedium, - color = MaterialTheme.appColors.textFieldHint, + color = MaterialTheme.appColors.textPrimaryVariant, ) } @@ -169,7 +171,7 @@ private fun CatalogRow( border = BorderStroke(1.dp, MaterialTheme.appColors.textFieldBorder.copy(alpha = 0.5f)), ) { Row( - modifier = Modifier.padding(start = 12.dp, top = 10.dp, bottom = 10.dp, end = 4.dp), + modifier = Modifier.padding(16.dp), verticalAlignment = Alignment.CenterVertically, ) { LmsRowLogo(logoUrl = logoUrl, title = title, accentColor = accentColor) @@ -178,7 +180,7 @@ private fun CatalogRow( Text( text = title, maxLines = 1, - style = MaterialTheme.appTypography.titleSmall, + style = MaterialTheme.appTypography.bodyLarge, color = MaterialTheme.appColors.textPrimary, ) if (shortDescription.isNotBlank()) { @@ -186,20 +188,20 @@ private fun CatalogRow( text = shortDescription, maxLines = 1, style = MaterialTheme.appTypography.bodyMedium, - color = MaterialTheme.appColors.textSecondary, + color = MaterialTheme.appColors.textPrimaryVariant, ) } Text( text = hostOf(baseUrl), maxLines = 1, style = MaterialTheme.appTypography.labelMedium, - color = MaterialTheme.appColors.textSecondary, + color = MaterialTheme.appColors.textPrimaryVariant, ) } Icon( imageVector = Icons.AutoMirrored.Filled.KeyboardArrowRight, contentDescription = null, - tint = MaterialTheme.appColors.primary, + tint = MaterialTheme.appColors.textPrimaryVariant, ) } } From 847b7e0de6eb9ef7cc3b8e945fdc0f486a6638e9 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:49:09 +0300 Subject: [PATCH 3/5] test: pin that a platform need not carry its own OAuth client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wire format here already allowed it — api and oauth_client_id are nullable, and Config falls back to the app's own client id. Nothing to change, but the mobile working group settled that this is the model rather than an accident, so it is worth a test that fails if someone tightens the DTO later. The matching iOS change makes the same document decode there. --- Documentation/LMSDirectory.md | 26 +++++++----- .../DocumentLmsDirectorySourceTest.kt | 40 +++++++++++++++---- 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/Documentation/LMSDirectory.md b/Documentation/LMSDirectory.md index 77d08dc7e..e58025cfb 100644 --- a/Documentation/LMSDirectory.md +++ b/Documentation/LMSDirectory.md @@ -55,9 +55,7 @@ One JSON file. This is the whole format: "visibility": "public", "featured": false, "api": { - "host_url": "https://learn.northwind.edu", - "feedback_email": "support@northwind.edu", - "oauth_client_id": "PASTE_THE_MOBILE_OAUTH_CLIENT_ID" + "feedback_email": "support@northwind.edu" }, "feature_flags": { "pre_login_discovery": false, @@ -89,19 +87,29 @@ One JSON file. This is the whole format: | `title` | Shown in the list and on the sign-in screen. | | `description` / `short_description` | Long and one-line blurbs. | | `base_url` | The Open edX site. Must be `https` in a shipped build. | -| `api.host_url` | Usually the same as `base_url`. | -| `api.oauth_client_id` | The site's **mobile** OAuth client id. Sign-in fails without the right one. | -| `api.feedback_email` | May be `""`. | ### Optional -Everything else. Omit a key and the app uses its own default, so the smallest -useful entry is `id`, `title`, `description`, `short_description`, `base_url` -and `api`. `provider` is optional too; its `name` is shown above the list. +Everything else, `api` included. Omit a key and the app uses its own default, so +the smallest useful entry is `id`, `title`, `description`, `short_description` +and `base_url`. `provider` is optional too; its `name` is shown above the list. `visibility` and `featured` are accepted and ignored — every platform in the file is shown, in the order the file lists them. +### OAuth + +A multi-instance app carries **one** OAuth client id of its own — the one in +`config.yaml` — and each platform registers that id in its own OAuth +Applications table, ideally restricted to the app's redirect scheme. The +directory is not where per-platform credentials live, so `api` can be omitted +entirely and usually should be. + +`api` is still read when present: `host_url` for a platform whose API lives at a +different address than the one the learner picked, `oauth_client_id` for a +platform that insists on its own, and `feedback_email` for the support address. +A platform naming its own client id overrides the app's for that platform only. + ## Images Every image field takes either of two things, and the value itself says which: diff --git a/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt b/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt index e61bd70bc..234bcb932 100644 --- a/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt +++ b/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt @@ -2,6 +2,7 @@ package org.openedx.core.lmsdirectory import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test @@ -114,12 +115,7 @@ class DocumentLmsDirectorySourceTest { "title": "Alpha", "description": "Alpha campus", "short_description": "Alpha", - "base_url": "https://alpha.example.edu", - "api": { - "host_url": "https://alpha.example.edu", - "oauth_client_id": "alpha-client", - "feedback_email": "support@example.edu" - } + "base_url": "https://alpha.example.edu" } ] } @@ -128,7 +124,37 @@ class DocumentLmsDirectorySourceTest { val detail = source(payload = minimal).detail("1") assertEquals("Alpha", detail.title) - assertEquals("alpha-client", detail.oauthClientId) + // No "api" block at all: the platform is served from the address the + // learner picked, and the app signs in with its own OAuth client. + assertEquals("https://alpha.example.edu", detail.baseUrl) + assertNull(detail.oauthClientId) + } + + @Test + fun `a platform need not carry its own oauth client`() = runTest { + // A multi-instance app carries one client id of its own, which each + // backend registers. A directory that names none per platform is the + // normal case, and must not stop the file being read. + val payload = """ + { + "version": 1, + "platforms": [ + { + "id": "1", + "title": "Alpha", + "short_description": "Alpha", + "base_url": "https://alpha.example.edu", + "api": { "host_url": "https://api.alpha.example.edu" } + } + ] + } + """.trimIndent() + + val detail = source(payload = payload).detail("1") + + assertEquals("https://api.alpha.example.edu", detail.baseUrl) + assertNull(detail.oauthClientId) + assertNull(detail.feedbackEmail) } @Test From 7d2b2109cede6195d0eb98cd109f901c6b31cab7 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Tue, 25 Aug 2026 18:10:42 +0300 Subject: [PATCH 4/5] refactor: read the directory in the key names the working group agreed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. `id` becomes optional with it: a platform that names none is identified by its address, which is unique within a directory anyway. Also finishes the design parity pass. A platform with no logo drew one initial on a tinted square here and two white initials on a filled one on iOS; both now draw the filled badge, since that fallback only shows up for platforms that never uploaded a logo — exactly the ones nobody looked at while comparing. --- Documentation/LMSDirectory.md | 48 +++++++++---------- .../app/lmsdirectory/LmsDetailDtoTest.kt | 14 +++--- .../lmsselection/SiteSelectionScreen.kt | 19 +++++--- .../openedx/core/lmsdirectory/LmsDetailDto.kt | 25 +++++----- .../core/lmsdirectory/LmsDirectorySource.kt | 31 ++++++------ .../DocumentLmsDirectorySourceTest.kt | 43 ++++++++--------- 6 files changed, 92 insertions(+), 88 deletions(-) diff --git a/Documentation/LMSDirectory.md b/Documentation/LMSDirectory.md index e58025cfb..b4c083078 100644 --- a/Documentation/LMSDirectory.md +++ b/Documentation/LMSDirectory.md @@ -37,23 +37,21 @@ One JSON file. This is the whole format: ```json { - "version": 1, + "format": "v1", "provider": { "name": "Northwind Education Group", "tagline": "Five campuses, one app", - "logo_url": null + "logo": null }, - "platforms": [ + "include": [ { + "name": "Northwind College", + "description": "Main campus", + "url": "https://learn.northwind.edu", + "logo": "https://cdn.northwind.edu/logo.png", + "id": "1", - "title": "Northwind College", - "description": "The main campus, offering undergraduate programmes.", - "short_description": "Main campus", - "base_url": "https://learn.northwind.edu", - "logo_url": "https://cdn.northwind.edu/logo.png", "accent_color": "#002545", - "visibility": "public", - "featured": false, "api": { "feedback_email": "support@northwind.edu" }, @@ -63,8 +61,7 @@ One JSON file. This is the whole format: }, "theme": { "accent_color_dark": "#4989bf", - "login_background_url": "https://cdn.northwind.edu/signin.png", - "logo_upload_url": null + "login_background": "https://cdn.northwind.edu/signin.png" }, "ui_components": { "course_unit_progress_enabled": true, @@ -81,21 +78,22 @@ One JSON file. This is the whole format: | field | what it is | | --- | --- | -| `version` | `1`. The only version there is. | -| `platforms[]` | At least one. An empty list gives the learner nothing to pick. | -| `id` | Unique within the file. A string, even when it looks like a number. | -| `title` | Shown in the list and on the sign-in screen. | -| `description` / `short_description` | Long and one-line blurbs. | -| `base_url` | The Open edX site. Must be `https` in a shipped build. | +| `format` | `"v1"`. The only version there is. | +| `include[]` | At least one platform. An empty list gives the learner nothing to pick. | +| `name` | Shown in the list and on the sign-in screen. | +| `url` | The Open edX site. Must be `https` in a shipped build. | ### Optional -Everything else, `api` included. Omit a key and the app uses its own default, so -the smallest useful entry is `id`, `title`, `description`, `short_description` -and `base_url`. `provider` is optional too; its `name` is shown above the list. +Everything else, `api` and `id` included. Omit a key and the app uses its own +default, so the smallest useful entry is `name` and `url`. A platform that names +no `id` is identified by its address, which is unique in a directory anyway. +`provider` is optional too; its `name` is shown above the list. -`visibility` and `featured` are accepted and ignored — every platform in the -file is shown, in the order the file lists them. +The key names follow the schema the Open edX mobile working group is settling +on, so a file written by hand and a file exported from a registry are the same +shape. Unknown keys are ignored, which is what lets a newer file stay readable +by an older build. ### OAuth @@ -117,8 +115,8 @@ Every image field takes either of two things, and the value itself says which: - something starting with `http://` or `https://` is downloaded; - anything else is the **name of a file shipped with the app**. -So `"logo_url": "https://cdn.northwind.edu/logo.png"` is fetched, and -`"logo_url": "northwind-logo.png"` is read from `assets/` — Coil resolves it as +So `"logo": "https://cdn.northwind.edu/logo.png"` is fetched, and +`"logo": "northwind-logo.png"` is read from `assets/` — Coil resolves it as `file:///android_asset/…` natively. That is what makes a fully offline build possible: put the images next to the document, refer to them by name, and the app never asks the network for a picture. diff --git a/app/src/test/java/org/openedx/app/lmsdirectory/LmsDetailDtoTest.kt b/app/src/test/java/org/openedx/app/lmsdirectory/LmsDetailDtoTest.kt index 3f009e393..8db2484a8 100644 --- a/app/src/test/java/org/openedx/app/lmsdirectory/LmsDetailDtoTest.kt +++ b/app/src/test/java/org/openedx/app/lmsdirectory/LmsDetailDtoTest.kt @@ -15,9 +15,9 @@ class LmsDetailDtoTest { fun `maps api fields to domain`() { val detail = LmsDetailDto( id = "4", - title = "Sandbox Env", - baseUrl = "https://sandbox.openedx.org", - logoUrl = "https://cdn.example.com/logo.png", + name = "Sandbox Env", + url = "https://sandbox.openedx.org", + logo = "https://cdn.example.com/logo.png", accentColor = "#6a2e7b", api = LmsDetailDto.ApiDto( hostUrl = "https://sandbox.openedx.org", @@ -37,8 +37,8 @@ class LmsDetailDtoTest { fun `blank api values fall back to null and base_url`() { val detail = LmsDetailDto( id = "1", - title = "Fallback", - baseUrl = "https://fallback.example.com", + name = "Fallback", + url = "https://fallback.example.com", api = LmsDetailDto.ApiDto(hostUrl = "", oauthClientId = "", feedbackEmail = null), ).toDomain() @@ -52,8 +52,8 @@ class LmsDetailDtoTest { fun `null api yields base_url and null credentials`() { val detail = LmsDetailDto( id = "2", - title = "No API block", - baseUrl = "https://noapi.example.com", + name = "No API block", + url = "https://noapi.example.com", api = null, ).toDomain() diff --git a/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionScreen.kt b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionScreen.kt index 112f533f0..84f8aea11 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionScreen.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/lmsselection/SiteSelectionScreen.kt @@ -215,8 +215,8 @@ private fun CatalogRow( @Composable private fun LmsRowLogo(logoUrl: String?, title: String, accentColor: String?) { val logoModifier = Modifier - .size(48.dp) - .clip(RoundedCornerShape(10.dp)) + .size(44.dp) + .clip(RoundedCornerShape(8.dp)) val logoModel = LmsImageSource.model(logoUrl) if (logoModel != null) { AsyncImage( @@ -229,15 +229,22 @@ private fun LmsRowLogo(logoUrl: String?, title: String, accentColor: String?) { modifier = logoModifier, ) } else { + // A platform with no logo gets its initials on its own colour, drawn the + // same way iOS draws them: filled badge, white letters, up to two. val accent = LmsThemeController.parseHexColor(accentColor) ?: MaterialTheme.appColors.primary + val initials = title.trim().split(" ") + .take(2) + .mapNotNull { it.firstOrNull()?.uppercase() } + .joinToString("") + .ifEmpty { title.take(1).uppercase() } Box( - modifier = logoModifier.background(accent.copy(alpha = 0.15f)), + modifier = logoModifier.background(accent), contentAlignment = Alignment.Center, ) { Text( - text = title.trim().take(1).uppercase(), - style = MaterialTheme.appTypography.titleMedium, - color = accent, + text = initials, + style = MaterialTheme.appTypography.titleSmall, + color = MaterialTheme.appColors.background, ) } } diff --git a/core/src/main/java/org/openedx/core/lmsdirectory/LmsDetailDto.kt b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDetailDto.kt index a15d179f4..57dafd94a 100644 --- a/core/src/main/java/org/openedx/core/lmsdirectory/LmsDetailDto.kt +++ b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDetailDto.kt @@ -5,11 +5,12 @@ import com.google.gson.annotations.SerializedName /** Wire format of one platform inside the directory document. */ data class LmsDetailDto( - @SerializedName("id") val id: String, - @SerializedName("title") val title: String, - @SerializedName("short_description") val shortDescription: String? = null, - @SerializedName("base_url") val baseUrl: String, - @SerializedName("logo_url") val logoUrl: String? = null, + /** Optional: a file that names no id is identified by its address. */ + @SerializedName("id") val id: String? = null, + @SerializedName("name") val name: String, + @SerializedName("description") val description: String? = null, + @SerializedName("url") val url: String, + @SerializedName("logo") val logo: String? = null, @SerializedName("accent_color") val accentColor: String? = null, @SerializedName("api") val api: ApiDto? = null, @SerializedName("theme") val theme: ThemeDto? = null, @@ -22,7 +23,7 @@ data class LmsDetailDto( ) data class ThemeDto( - @SerializedName("login_background_url") val loginBackgroundUrl: String? = null, + @SerializedName("login_background") val loginBackground: String? = null, ) data class FeatureFlagsDto( @@ -30,15 +31,15 @@ data class LmsDetailDto( ) fun toDomain() = LmsDetail( - id = id, - title = title, - shortDescription = shortDescription.orEmpty(), - baseUrl = api?.hostUrl?.ifBlank { null } ?: baseUrl, - logoUrl = logoUrl, + id = id ?: url, + title = name, + shortDescription = description.orEmpty(), + baseUrl = api?.hostUrl?.ifBlank { null } ?: url, + logoUrl = logo, accentColor = accentColor, oauthClientId = api?.oauthClientId?.ifBlank { null }, feedbackEmail = api?.feedbackEmail?.ifBlank { null }, - loginBackgroundUrl = theme?.loginBackgroundUrl?.ifBlank { null }, + loginBackgroundUrl = theme?.loginBackground?.ifBlank { null }, preLoginDiscovery = featureFlags?.preLoginDiscovery ?: false, ) } diff --git a/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectorySource.kt b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectorySource.kt index 0b8b4ea0c..d58772e66 100644 --- a/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectorySource.kt +++ b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectorySource.kt @@ -54,15 +54,15 @@ class DocumentLmsDirectorySource( override suspend fun providerName(): String = document().provider?.name.orEmpty() - override suspend fun platforms(): List = document().platforms.map { it.toSummary() } + override suspend fun platforms(): List = document().include.map { it.toSummary() } override suspend fun detail(id: String): LmsDetail = - document().platforms.firstOrNull { it.id == id }?.toDomain() + document().include.firstOrNull { (it.id ?: it.url) == id }?.toDomain() ?: throw NoSuchElementException("No platform with id $id in the directory document") override suspend fun imageReferences(): List = - document().platforms.flatMap { - listOfNotNull(it.logoUrl, it.theme?.loginBackgroundUrl) + document().include.flatMap { + listOfNotNull(it.logo, it.theme?.loginBackground) }.filter { it.isNotBlank() } private suspend fun document(): DirectoryDocumentDto { @@ -70,7 +70,7 @@ class DocumentLmsDirectorySource( val raw = loader.load() val parsed = gson.fromJson(raw, DirectoryDocumentDto::class.java) checkNotNull(parsed) { "Directory document is empty" } - if (parsed.platforms.isEmpty()) { + if (parsed.include.isEmpty()) { Log.w(TAG, "Directory document parsed but lists no platforms") } cached = parsed @@ -112,27 +112,26 @@ class DocumentLmsDirectorySource( /** * Wire format of the directory document. * - * `platforms` entries are the same objects `/api/v1/directory/{id}` returns, so - * [LmsDetailDto] is reused rather than duplicated — a document and a live service - * describe a platform identically, and keeping one parser is what guarantees it. + * The key names are the ones the Open edX mobile working group settled on, so a + * file written by hand and a file exported from a registry are the same shape. */ data class DirectoryDocumentDto( - @SerializedName("version") val version: Int = 1, + @SerializedName("format") val format: String? = null, @SerializedName("provider") val provider: ProviderDto? = null, - @SerializedName("platforms") val platforms: List = emptyList(), + @SerializedName("include") val include: List = emptyList(), ) { data class ProviderDto( @SerializedName("name") val name: String? = null, @SerializedName("tagline") val tagline: String? = null, - @SerializedName("logo_url") val logoUrl: String? = null, + @SerializedName("logo") val logo: String? = null, ) } private fun LmsDetailDto.toSummary(): LmsSummary = LmsSummary( - id = id, - title = title, - shortDescription = shortDescription.orEmpty(), - baseUrl = api?.hostUrl?.ifBlank { null } ?: baseUrl, - logoUrl = logoUrl, + id = id ?: url, + title = name, + shortDescription = description.orEmpty(), + baseUrl = api?.hostUrl?.ifBlank { null } ?: url, + logoUrl = logo, accentColor = accentColor, ) diff --git a/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt b/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt index 234bcb932..8e2a34e36 100644 --- a/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt +++ b/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt @@ -15,15 +15,15 @@ class DocumentLmsDirectorySourceTest { private val document = """ { - "version": 1, + "format": "v1", "provider": { "name": "Northwind", "tagline": "Five campuses, one app" }, - "platforms": [ + "include": [ { "id": "1", - "title": "Alpha", - "short_description": "Alpha", - "base_url": "https://alpha.example.edu", - "logo_url": "https://cdn.example.com/alpha.png", + "name": "Alpha", + "description": "Alpha", + "url": "https://alpha.example.edu", + "logo": "https://cdn.example.com/alpha.png", "accent_color": "#112233", "api": { "host_url": "https://alpha.example.edu", @@ -31,14 +31,14 @@ class DocumentLmsDirectorySourceTest { "feedback_email": "support@example.edu" }, "feature_flags": { "pre_login_discovery": true }, - "theme": { "login_background_url": "alpha-bg.png" } + "theme": { "login_background": "alpha-bg.png" } }, { "id": "2", - "title": "Beta", - "short_description": "Beta", - "base_url": "https://beta.example.edu", - "logo_url": "beta-logo.png", + "name": "Beta", + "description": "Beta", + "url": "https://beta.example.edu", + "logo": "beta-logo.png", "api": { "host_url": "https://beta.example.edu", "oauth_client_id": "beta-client", @@ -108,14 +108,13 @@ class DocumentLmsDirectorySourceTest { // both platforms — not just on this one, where Gson is forgiving. val minimal = """ { - "version": 1, - "platforms": [ + "format": "v1", + "include": [ { "id": "1", - "title": "Alpha", - "description": "Alpha campus", - "short_description": "Alpha", - "base_url": "https://alpha.example.edu" + "name": "Alpha", + "description": "Alpha", + "url": "https://alpha.example.edu" } ] } @@ -137,13 +136,13 @@ class DocumentLmsDirectorySourceTest { // normal case, and must not stop the file being read. val payload = """ { - "version": 1, - "platforms": [ + "format": "v1", + "include": [ { "id": "1", - "title": "Alpha", - "short_description": "Alpha", - "base_url": "https://alpha.example.edu", + "name": "Alpha", + "description": "Alpha", + "url": "https://alpha.example.edu", "api": { "host_url": "https://api.alpha.example.edu" } } ] From 59de9c930ced8146ba22ac49a07328715e8a895b Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Thu, 27 Aug 2026 19:47:29 +0300 Subject: [PATCH 5/5] refactor: drop the platform id, and identify a platform by its position MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 identifying by URL merges them, so opening one row would give another row's settings. Compose survived that by accident, because the list is keyed by index; iOS did not, which is how it was found. A test holds a two-entry document that shares one address, so neither platform can regress into merging them. --- Documentation/LMSDirectory.md | 7 ++-- .../app/lmsdirectory/LmsDetailDtoTest.kt | 9 ++--- .../SiteSelectionViewModelTest.kt | 10 ++--- .../openedx/core/lmsdirectory/LmsDetailDto.kt | 13 ++++-- .../core/lmsdirectory/LmsDirectorySource.kt | 11 ++--- .../DocumentLmsDirectorySourceTest.kt | 40 ++++++++++++++----- 6 files changed, 57 insertions(+), 33 deletions(-) diff --git a/Documentation/LMSDirectory.md b/Documentation/LMSDirectory.md index b4c083078..b25f61047 100644 --- a/Documentation/LMSDirectory.md +++ b/Documentation/LMSDirectory.md @@ -50,7 +50,6 @@ One JSON file. This is the whole format: "url": "https://learn.northwind.edu", "logo": "https://cdn.northwind.edu/logo.png", - "id": "1", "accent_color": "#002545", "api": { "feedback_email": "support@northwind.edu" @@ -85,9 +84,9 @@ One JSON file. This is the whole format: ### Optional -Everything else, `api` and `id` included. Omit a key and the app uses its own -default, so the smallest useful entry is `name` and `url`. A platform that names -no `id` is identified by its address, which is unique in a directory anyway. +Everything else, `api` included. Omit a key and the app uses its own default, so +the smallest useful entry is `name` and `url`. A platform is identified by its +address, so there is no separate id to keep in step with anything. `provider` is optional too; its `name` is shown above the list. The key names follow the schema the Open edX mobile working group is settling diff --git a/app/src/test/java/org/openedx/app/lmsdirectory/LmsDetailDtoTest.kt b/app/src/test/java/org/openedx/app/lmsdirectory/LmsDetailDtoTest.kt index 8db2484a8..97e0034f8 100644 --- a/app/src/test/java/org/openedx/app/lmsdirectory/LmsDetailDtoTest.kt +++ b/app/src/test/java/org/openedx/app/lmsdirectory/LmsDetailDtoTest.kt @@ -14,7 +14,6 @@ class LmsDetailDtoTest { @Test fun `maps api fields to domain`() { val detail = LmsDetailDto( - id = "4", name = "Sandbox Env", url = "https://sandbox.openedx.org", logo = "https://cdn.example.com/logo.png", @@ -24,7 +23,7 @@ class LmsDetailDtoTest { oauthClientId = "android", feedbackEmail = "team@example.com", ), - ).toDomain() + ).toDomain("0") assertEquals("android", detail.oauthClientId) assertEquals("team@example.com", detail.feedbackEmail) @@ -36,11 +35,10 @@ class LmsDetailDtoTest { @Test fun `blank api values fall back to null and base_url`() { val detail = LmsDetailDto( - id = "1", name = "Fallback", url = "https://fallback.example.com", api = LmsDetailDto.ApiDto(hostUrl = "", oauthClientId = "", feedbackEmail = null), - ).toDomain() + ).toDomain("0") // Blank host_url → the top-level base_url is used. assertEquals("https://fallback.example.com", detail.baseUrl) @@ -51,11 +49,10 @@ class LmsDetailDtoTest { @Test fun `null api yields base_url and null credentials`() { val detail = LmsDetailDto( - id = "2", name = "No API block", url = "https://noapi.example.com", api = null, - ).toDomain() + ).toDomain("0") assertEquals("https://noapi.example.com", detail.baseUrl) assertNull(detail.oauthClientId) diff --git a/auth/src/test/java/org/openedx/auth/presentation/lmsselection/SiteSelectionViewModelTest.kt b/auth/src/test/java/org/openedx/auth/presentation/lmsselection/SiteSelectionViewModelTest.kt index 5233ae49f..ba1a97f40 100644 --- a/auth/src/test/java/org/openedx/auth/presentation/lmsselection/SiteSelectionViewModelTest.kt +++ b/auth/src/test/java/org/openedx/auth/presentation/lmsselection/SiteSelectionViewModelTest.kt @@ -40,7 +40,7 @@ class SiteSelectionViewModelTest { private val repository = mockk(relaxed = true) private val summary = LmsSummary( - id = "4", + id = "https://sandbox.openedx.org", title = "Sandbox Env", shortDescription = "", baseUrl = "https://sandbox.openedx.org", @@ -49,7 +49,7 @@ class SiteSelectionViewModelTest { ) private fun detail(preLoginDiscovery: Boolean = false) = LmsDetail( - id = "4", + id = "https://sandbox.openedx.org", title = "Sandbox Env", shortDescription = "", baseUrl = "https://sandbox.openedx.org", @@ -81,7 +81,7 @@ class SiteSelectionViewModelTest { advanceUntilIdle() assertEquals(CatalogState.Loaded, viewModel.uiState.value.catalog) - assertEquals(listOf("4"), viewModel.uiState.value.platforms.map { it.id }) + assertEquals(listOf("https://sandbox.openedx.org"), viewModel.uiState.value.platforms.map { it.id }) assertEquals("Northwind", viewModel.uiState.value.providerName) } @@ -108,7 +108,7 @@ class SiteSelectionViewModelTest { val (viewModel, actions) = select(preLoginDiscovery = false) // The summary carries no OAuth client id, so the full record has to be read. - coVerify { repository.detail("4") } + coVerify { repository.detail("https://sandbox.openedx.org") } verify { corePreferences.selectedBaseUrl = "https://sandbox.openedx.org/" } verify { corePreferences.selectedOAuthClientId = "client-id" } verify { corePreferences.selectedLmsTitle = "Sandbox Env" } @@ -128,7 +128,7 @@ class SiteSelectionViewModelTest { private fun kotlinx.coroutines.test.TestScope.select( preLoginDiscovery: Boolean, ): Pair> { - coEvery { repository.detail("4") } returns Result.success(detail(preLoginDiscovery)) + coEvery { repository.detail("https://sandbox.openedx.org") } returns Result.success(detail(preLoginDiscovery)) val viewModel = SiteSelectionViewModel(corePreferences, resourceManager, repository) val actions = mutableListOf() backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) { diff --git a/core/src/main/java/org/openedx/core/lmsdirectory/LmsDetailDto.kt b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDetailDto.kt index 57dafd94a..dddc5febd 100644 --- a/core/src/main/java/org/openedx/core/lmsdirectory/LmsDetailDto.kt +++ b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDetailDto.kt @@ -5,8 +5,6 @@ import com.google.gson.annotations.SerializedName /** Wire format of one platform inside the directory document. */ data class LmsDetailDto( - /** Optional: a file that names no id is identified by its address. */ - @SerializedName("id") val id: String? = null, @SerializedName("name") val name: String, @SerializedName("description") val description: String? = null, @SerializedName("url") val url: String, @@ -30,8 +28,15 @@ data class LmsDetailDto( @SerializedName("pre_login_discovery") val preLoginDiscovery: Boolean = false, ) - fun toDomain() = LmsDetail( - id = id ?: url, + /** + * The platform, identified by where it sits in the document. + * + * Position is the only thing guaranteed unique. Two entries may legitimately + * share an address — the same LMS listed twice under different branding — + * and identifying them by URL silently merges them. + */ + fun toDomain(id: String) = LmsDetail( + id = id, title = name, shortDescription = description.orEmpty(), baseUrl = api?.hostUrl?.ifBlank { null } ?: url, diff --git a/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectorySource.kt b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectorySource.kt index d58772e66..501cba6bc 100644 --- a/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectorySource.kt +++ b/core/src/main/java/org/openedx/core/lmsdirectory/LmsDirectorySource.kt @@ -54,11 +54,12 @@ class DocumentLmsDirectorySource( override suspend fun providerName(): String = document().provider?.name.orEmpty() - override suspend fun platforms(): List = document().include.map { it.toSummary() } + override suspend fun platforms(): List = + document().include.mapIndexed { index, entry -> entry.toSummary(index.toString()) } override suspend fun detail(id: String): LmsDetail = - document().include.firstOrNull { (it.id ?: it.url) == id }?.toDomain() - ?: throw NoSuchElementException("No platform with id $id in the directory document") + document().include.getOrNull(id.toIntOrNull() ?: -1)?.toDomain(id) + ?: throw NoSuchElementException("No platform at position $id in the directory document") override suspend fun imageReferences(): List = document().include.flatMap { @@ -127,8 +128,8 @@ data class DirectoryDocumentDto( ) } -private fun LmsDetailDto.toSummary(): LmsSummary = LmsSummary( - id = id ?: url, +private fun LmsDetailDto.toSummary(id: String): LmsSummary = LmsSummary( + id = id, title = name, shortDescription = description.orEmpty(), baseUrl = api?.hostUrl?.ifBlank { null } ?: url, diff --git a/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt b/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt index 8e2a34e36..28769f94b 100644 --- a/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt +++ b/core/src/test/java/org/openedx/core/lmsdirectory/DocumentLmsDirectorySourceTest.kt @@ -19,7 +19,6 @@ class DocumentLmsDirectorySourceTest { "provider": { "name": "Northwind", "tagline": "Five campuses, one app" }, "include": [ { - "id": "1", "name": "Alpha", "description": "Alpha", "url": "https://alpha.example.edu", @@ -34,7 +33,6 @@ class DocumentLmsDirectorySourceTest { "theme": { "login_background": "alpha-bg.png" } }, { - "id": "2", "name": "Beta", "description": "Beta", "url": "https://beta.example.edu", @@ -68,7 +66,7 @@ class DocumentLmsDirectorySourceTest { var loads = 0 val source = source(onLoad = { loads++ }) source.platforms() - val detail = source.detail("2") + val detail = source.detail("1") assertEquals("Beta", detail.title) assertEquals("beta-client", detail.oauthClientId) @@ -77,14 +75,40 @@ class DocumentLmsDirectorySourceTest { assertEquals(1, loads) } + @Test + fun `two platforms may share an address without merging`() = runTest { + // Identifying a platform by its URL merges these two: the list draws one + // and opening it gives the other one's settings. + val payload = """ + { + "format": "v1", + "include": [ + { "name": "Alpha", "description": "Alpha", "url": "https://shared.example.edu", + "accent_color": "#111111" }, + { "name": "Beta", "description": "Beta", "url": "https://shared.example.edu", + "accent_color": "#222222" } + ] + } + """.trimIndent() + val source = source(payload = payload) + + val items = source.platforms() + assertEquals(listOf("Alpha", "Beta"), items.map { it.title }) + assertEquals(2, items.map { it.id }.toSet().size) + + val second = source.detail(items[1].id) + assertEquals("Beta", second.title) + assertEquals("#222222", second.accentColor) + } + @Test fun `an unknown id is an error rather than a silent empty result`() = runTest { val source = source() try { - source.detail("nope") + source.detail("9") throw AssertionError("Expected a failure for an unknown id") } catch (e: NoSuchElementException) { - assertTrue(e.message!!.contains("nope")) + assertTrue(e.message!!.contains("9")) } } @@ -111,7 +135,6 @@ class DocumentLmsDirectorySourceTest { "format": "v1", "include": [ { - "id": "1", "name": "Alpha", "description": "Alpha", "url": "https://alpha.example.edu" @@ -120,7 +143,7 @@ class DocumentLmsDirectorySourceTest { } """.trimIndent() - val detail = source(payload = minimal).detail("1") + val detail = source(payload = minimal).detail("0") assertEquals("Alpha", detail.title) // No "api" block at all: the platform is served from the address the @@ -139,7 +162,6 @@ class DocumentLmsDirectorySourceTest { "format": "v1", "include": [ { - "id": "1", "name": "Alpha", "description": "Alpha", "url": "https://alpha.example.edu", @@ -149,7 +171,7 @@ class DocumentLmsDirectorySourceTest { } """.trimIndent() - val detail = source(payload = payload).detail("1") + val detail = source(payload = payload).detail("0") assertEquals("https://api.alpha.example.edu", detail.baseUrl) assertNull(detail.oauthClientId)