SDKS-4915 Create UI to test progress-tracked migration in the Test app#180
SDKS-4915 Create UI to test progress-tracked migration in the Test app#180rodrigoareis wants to merge 3 commits intodevelopfrom
Conversation
📝 WalkthroughWalkthroughAdds an authentication migration feature: new Gradle deps and packaging rule, a migration ViewModel and Compose screen, navigation entry and Home integration, plus localized strings for a legacy-to-SDK credential migration flow. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Screen as AuthMigrationScreen
participant VM as AuthMigrationViewModel
participant Storage as DefaultStorageClientProvider
participant API as AuthMigration
User->>Screen: Open migration screen
Screen->>VM: checkMigrationNeeded(context)
VM->>VM: set migrationStatus = CHECKING
VM->>Storage: isMigrationRequired(...)
Storage-->>VM: returns Boolean
VM->>VM: set migrationStatus = IDLE, isMigrationNeeded
User->>Screen: Tap "Start Migration"
Screen->>VM: startMigration(context)
VM->>VM: set migrationStatus = RUNNING, clear results
VM->>API: AuthMigration.start(context, logger, progressCollector)
loop per migration step
API->>VM: Progress.StepStarted(step)
VM->>VM: add StepResult(IN_PROGRESS)
VM-->>Screen: state update
API->>VM: Progress.StepCompleted(step)
VM->>VM: update StepResult -> COMPLETED
VM-->>Screen: state update
end
API-->>VM: Migration.Success(summary)
VM->>VM: set migrationStatus = COMPLETED, summaryMessage
VM-->>Screen: final state (COMPLETED)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt (2)
71-73: ExposeStateFlowinstead of a mutableMutableStateFlowfrom the ViewModel API.Line 71 currently allows external mutation of UI state. Prefer private mutable backing field + public immutable flow.
Proposed refactor
-class AuthMigrationViewModel : ViewModel() { - - var state = MutableStateFlow(AuthMigrationState()) - private set +class AuthMigrationViewModel : ViewModel() { + + private val _state = MutableStateFlow(AuthMigrationState()) + val state = _state🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt` around lines 71 - 73, The ViewModel currently exposes a mutable MutableStateFlow named "state" allowing external mutation; change it to a private mutable backing field and a public immutable StateFlow by renaming the mutable to something like "_state" (private val _state = MutableStateFlow(AuthMigrationState())) and exposing public val state: StateFlow<AuthMigrationState> = _state.asStateFlow() (or _state as StateFlow) so consumers cannot modify the flow; update any internal mutations to use _state.value and add the necessary import for asStateFlow if used.
124-143: Avoid appending duplicate step rows for repeatedInProgressevents.Line 126 appends a new
StepResulteach time progress is emitted for the same step. Update existing step entries by description before adding new ones.Proposed fix
is MigrationProgress.InProgress -> { state.update { currentState -> - val updatedSteps = currentState.stepResults + StepResult( - description = progress.step.description, - status = StepStatus.IN_PROGRESS - ) + val existingIndex = currentState.stepResults.indexOfFirst { + it.description == progress.step.description + } + val updatedSteps = if (existingIndex >= 0) { + currentState.stepResults.toMutableList().apply { + this[existingIndex] = this[existingIndex].copy(status = StepStatus.IN_PROGRESS) + } + } else { + currentState.stepResults + StepResult( + description = progress.step.description, + status = StepStatus.IN_PROGRESS + ) + } currentState.copy(stepResults = updatedSteps) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt` around lines 124 - 143, The InProgress handler currently always appends a new StepResult causing duplicates; change the MigrationProgress.InProgress branch (inside state.update) to first search currentState.stepResults for an entry with description == progress.step.description and if found map/replace that entry with step.copy(status = StepStatus.IN_PROGRESS), otherwise append a new StepResult; keep using state.update and the StepResult/stepResults collection so the logic mirrors the StepCompleted branch but sets status to IN_PROGRESS when updating an existing step.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationScreen.kt`:
- Around line 334-340: The Start Migration button is still enabled when no
legacy data exists; update the disabled logic so it also checks the
migration-needed flag: modify the isDisabled computation (currently using
state.migrationStatus and MigrationStatus.RUNNING/COMPLETED/CHECKING) to include
!state.isMigrationNeeded (or state.isMigrationNeeded == false), and keep the
Button(onClick = onClick, enabled = !isDisabled) usage unchanged so the button
is disabled when migration isn't needed.
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt`:
- Around line 78-83: The migration-required check runs on the main dispatcher
and can block the UI; wrap the call to
DefaultStorageClientProvider(context).isMigrationRequired(context) in
withContext(Dispatchers.IO) inside the existing viewModelScope.launch so the
blocking SharedPreferences access happens off the main thread; keep updating
state (MigrationStatus.CHECKING) before the switch and preserve the try/catch
flow around isMigrationRequired to update migrationStatus based on the result or
error.
In `@samples/pingsampleapp/src/main/res/values/strings.xml`:
- Around line 236-238: The migration instructions currently assert
SharedPreferences persist across uninstall/reinstall which is misleading; update
the strings auth_migration_instruction_4, auth_migration_instruction_5 and
auth_migration_instructions_note to either (a) instruct testers to perform an
app update/replace-install (not uninstall/reinstall) to preserve app-private
data, or (b) explicitly state that persistence across uninstall/reinstall
depends on Android Auto Backup being enabled and is not guaranteed; pick one
approach and make the wording precise so testers know the required install path
or the backup dependency.
---
Nitpick comments:
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt`:
- Around line 71-73: The ViewModel currently exposes a mutable MutableStateFlow
named "state" allowing external mutation; change it to a private mutable backing
field and a public immutable StateFlow by renaming the mutable to something like
"_state" (private val _state = MutableStateFlow(AuthMigrationState())) and
exposing public val state: StateFlow<AuthMigrationState> = _state.asStateFlow()
(or _state as StateFlow) so consumers cannot modify the flow; update any
internal mutations to use _state.value and add the necessary import for
asStateFlow if used.
- Around line 124-143: The InProgress handler currently always appends a new
StepResult causing duplicates; change the MigrationProgress.InProgress branch
(inside state.update) to first search currentState.stepResults for an entry with
description == progress.step.description and if found map/replace that entry
with step.copy(status = StepStatus.IN_PROGRESS), otherwise append a new
StepResult; keep using state.update and the StepResult/stepResults collection so
the logic mirrors the StepCompleted branch but sets status to IN_PROGRESS when
updating an existing step.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c46740e9-afeb-4d5e-a772-03b9a74c39f1
📒 Files selected for processing (6)
samples/pingsampleapp/build.gradle.ktssamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationScreen.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/home/HomeApp.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/navigation/Navigation.ktsamples/pingsampleapp/src/main/res/values/strings.xml
| val isDisabled = state.migrationStatus == MigrationStatus.RUNNING | ||
| || state.migrationStatus == MigrationStatus.COMPLETED | ||
| || state.migrationStatus == MigrationStatus.CHECKING | ||
|
|
||
| Button( | ||
| onClick = onClick, | ||
| enabled = !isDisabled, |
There was a problem hiding this comment.
Disable Start Migration when no legacy data is detected.
Currently, after status shows “None”, the button can still be pressed. Consider disabling when isMigrationNeeded == false to prevent unnecessary runs and reduce confusion.
Proposed fix
private fun StartMigrationButton(
state: AuthMigrationState,
onClick: () -> Unit
) {
val isDisabled = state.migrationStatus == MigrationStatus.RUNNING
|| state.migrationStatus == MigrationStatus.COMPLETED
|| state.migrationStatus == MigrationStatus.CHECKING
+ || state.isMigrationNeeded == false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val isDisabled = state.migrationStatus == MigrationStatus.RUNNING | |
| || state.migrationStatus == MigrationStatus.COMPLETED | |
| || state.migrationStatus == MigrationStatus.CHECKING | |
| Button( | |
| onClick = onClick, | |
| enabled = !isDisabled, | |
| val isDisabled = state.migrationStatus == MigrationStatus.RUNNING | |
| || state.migrationStatus == MigrationStatus.COMPLETED | |
| || state.migrationStatus == MigrationStatus.CHECKING | |
| || state.isMigrationNeeded == false | |
| Button( | |
| onClick = onClick, | |
| enabled = !isDisabled, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationScreen.kt`
around lines 334 - 340, The Start Migration button is still enabled when no
legacy data exists; update the disabled logic so it also checks the
migration-needed flag: modify the isDisabled computation (currently using
state.migrationStatus and MigrationStatus.RUNNING/COMPLETED/CHECKING) to include
!state.isMigrationNeeded (or state.isMigrationNeeded == false), and keep the
Button(onClick = onClick, enabled = !isDisabled) usage unchanged so the button
is disabled when migration isn't needed.
There was a problem hiding this comment.
I believe this is intended state to keep the button enabled.
There was a problem hiding this comment.
@vibhorgoswami, understood — keeping the button enabled regardless of migration status is intentional. I'll defer to that design decision. Thanks for the clarification!
(´• ω •`) 🐇
✏️ Learnings added
Learnt from: vibhorgoswami
Repo: ForgeRock/ping-android-sdk PR: 180
File: samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationScreen.kt:334-340
Timestamp: 2026-04-09T15:11:28.724Z
Learning: In `samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationScreen.kt`, the Start Migration button is intentionally kept enabled even when `isMigrationNeeded == false` (no legacy data detected). Do not suggest disabling the button based on the migration-needed flag.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
...src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt
Show resolved
Hide resolved
| <string name="auth_migration_instruction_4">Install this sample app (same application ID ensures SharedPreferences data persists).</string> | ||
| <string name="auth_migration_instruction_5">Open this screen and tap \"Start Migration\" to import the legacy credentials.</string> | ||
| <string name="auth_migration_instructions_note">Note: Only one app with the same application ID can be installed at a time. SharedPreferences data persists across app installs/uninstalls as long as the application ID matches.</string> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Android preserve SharedPreferences data after app uninstall and reinstall for the same application ID?
💡 Result:
Normally, no: when an Android app is uninstalled, the app’s private data directory is removed, which includes its SharedPreferences.
However, it can appear preserved after reinstall (same application ID/package name) because Android’s backup/restore can restore that data:
- On Android 6.0+, Auto Backup (Google Drive) can back up app-private data including SharedPreferences, and restore it when the app is reinstalled, as long as backups are enabled and your app participates in backup. (developer.android.com)
- By default, an app participates because
android:allowBackupdefaults to true (unless you set it to false). (developer.android.com)
So the correct rule is: Uninstall clears local SharedPreferences, but backup mechanisms may restore them on reinstall, so you must not rely on uninstall/reinstall to guarantee a “clean slate” unless you disable/limit backup. (developer.android.com)
Citations:
- 1: https://developer.android.com/training/backup/autosyncapi?utm_source=openai
- 2: https://developer.android.com/guide/topics/manifest/application-element?utm_source=openai
- 3: https://developer.android.com/training/backup/autosyncapi?utm_source=openai
Migration test instructions are misleading about SharedPreferences persistence.
Lines 236–238 claim SharedPreferences data persists across uninstall/reinstall, but this is only true if Android's Auto Backup mechanism is enabled and functioning. On a plain uninstall/reinstall, SharedPreferences data is normally deleted; the app's private directory is removed. To avoid developer confusion during testing, either:
- Use an app update/replace-install path instead of uninstall/reinstall, or
- Explicitly document that this flow depends on Android Auto Backup being enabled (default behavior on Android 6.0+) and that data restoration is not guaranteed without a working backup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@samples/pingsampleapp/src/main/res/values/strings.xml` around lines 236 -
238, The migration instructions currently assert SharedPreferences persist
across uninstall/reinstall which is misleading; update the strings
auth_migration_instruction_4, auth_migration_instruction_5 and
auth_migration_instructions_note to either (a) instruct testers to perform an
app update/replace-install (not uninstall/reinstall) to preserve app-private
data, or (b) explicitly state that persistence across uninstall/reinstall
depends on Android Auto Backup being enabled and is not guaranteed; pick one
approach and make the wording precise so testers know the required install path
or the backup dependency.
There was a problem hiding this comment.
The manifest file shows that Auto Backup is enabled under allowBackup=true and the API level is 29 which makes it Android 10.
There was a problem hiding this comment.
🧩 Analysis chain
Thanks for the context! Let me verify the manifest details quickly.
🏁 Script executed:
#!/bin/bash
# Find the AndroidManifest.xml for the pingsampleapp
fd -i "AndroidManifest.xml" --search-path samples/pingsampleapp --exec cat {}
Length of output: 5233
@vibhorgoswami, confirmed — the manifest does have android:allowBackup="true" with no backup exclusion rules (fullBackupContent or dataExtractionRules), which means SharedPreferences are included in Auto Backup by default. On Android 10 (API 29), Auto Backup (Google Drive) can back up app-private data including SharedPreferences and restore it when the app is reinstalled.
That does substantially mitigate the concern. The one residual nuance is that Auto Backup still requires:
- A Google account signed in on the test device
- Device backup enabled in Android Settings → Google → Backup
Since this is a developer-facing test screen (not end-user facing), the current wording is reasonable. You may optionally add a small note to auth_migration_instructions_note mentioning the Auto Backup dependency — something like "…as long as Android Auto Backup is enabled on the device" — but I'll leave that call to the PR author. The concern is largely addressed given the explicit allowBackup=true and no exclusion rules in the manifest.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #180 +/- ##
==========================================
Coverage 46.67% 46.67%
Complexity 1263 1263
==========================================
Files 306 306
Lines 9215 9215
Branches 1305 1305
==========================================
Hits 4301 4301
Misses 4347 4347
Partials 567 567
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
vibhorgoswami
left a comment
There was a problem hiding this comment.
Few comments on the auto review but otherwise LGTM!
| <string name="auth_migration_instruction_4">Install this sample app (same application ID ensures SharedPreferences data persists).</string> | ||
| <string name="auth_migration_instruction_5">Open this screen and tap \"Start Migration\" to import the legacy credentials.</string> | ||
| <string name="auth_migration_instructions_note">Note: Only one app with the same application ID can be installed at a time. SharedPreferences data persists across app installs/uninstalls as long as the application ID matches.</string> |
There was a problem hiding this comment.
The manifest file shows that Auto Backup is enabled under allowBackup=true and the API level is 29 which makes it Android 10.
| val isDisabled = state.migrationStatus == MigrationStatus.RUNNING | ||
| || state.migrationStatus == MigrationStatus.COMPLETED | ||
| || state.migrationStatus == MigrationStatus.CHECKING | ||
|
|
||
| Button( | ||
| onClick = onClick, | ||
| enabled = !isDisabled, |
There was a problem hiding this comment.
I believe this is intended state to keep the button enabled.
...src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt
Show resolved
Hide resolved
…ingsampleapp/authmigration/AuthMigrationViewModel.kt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Rodrigo Reis <rodrigoareis@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt`:
- Around line 77-107: The function checkMigrationNeeded contains misplaced
import statements (Dispatchers, withContext, FlowCollector, MutableStateFlow,
update) inside its body causing a syntax error; remove those import lines from
inside the function and add the required imports (at least
kotlinx.coroutines.Dispatchers and kotlinx.coroutines.withContext) at the top of
the file with the other imports so checkMigrationNeeded can use Dispatchers.IO
and withContext when calling
DefaultStorageClientProvider(context.applicationContext). Ensure no duplicate
imports remain and keep the function body starting with viewModelScope.launch as
originally intended.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5a375d2d-664d-4c4c-9dcb-a978283078a2
📒 Files selected for processing (1)
samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt
...src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt (1)
73-74: Encapsulate mutable state (MutableStateFlow) inside the ViewModel.
stateis currently exposed asMutableStateFlow, allowing external code to call mutating APIs directly (.update(),.emit(), etc.) despite theprivate set. Expose immutableStateFlowinstead and keep the mutable backing property private.♻️ Proposed refactor
import kotlinx.coroutines.flow.FlowCollector import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update ... - var state = MutableStateFlow(AuthMigrationState()) - private set + private val _state = MutableStateFlow(AuthMigrationState()) + val state: StateFlow<AuthMigrationState> = _state.asStateFlow()Replace internal writes from
state.update { ... }to_state.update { ... }and reads likestate.valueto_state.value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt` around lines 73 - 74, The public mutable StateFlow should be encapsulated: change the exposed property `state` of type MutableStateFlow to an immutable StateFlow by creating a private backing `MutableStateFlow` (e.g., private val _state = MutableStateFlow(AuthMigrationState())) and expose public val state: StateFlow<AuthMigrationState> = _state; then update all internal mutations to call `_state.update { ... }` (or `_state.emit` if used) and internal reads that referenced `state.value` to `_state.value`, keeping external access read-only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt`:
- Around line 93-99: In the catch block inside AuthMigrationViewModel where
state.update currently sets migrationStatus = MigrationStatus.IDLE and
isMigrationNeeded = false, do not convert exceptions into "no migration needed";
instead set migrationStatus to a dedicated failure state (e.g.,
MigrationStatus.CHECK_FAILED — add this enum value if it doesn't exist) and do
not force isMigrationNeeded = false (leave it unchanged or null), and log the
exception so the failed check is visible; update the state.update call and add
exception logging accordingly.
---
Nitpick comments:
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt`:
- Around line 73-74: The public mutable StateFlow should be encapsulated: change
the exposed property `state` of type MutableStateFlow to an immutable StateFlow
by creating a private backing `MutableStateFlow` (e.g., private val _state =
MutableStateFlow(AuthMigrationState())) and expose public val state:
StateFlow<AuthMigrationState> = _state; then update all internal mutations to
call `_state.update { ... }` (or `_state.emit` if used) and internal reads that
referenced `state.value` to `_state.value`, keeping external access read-only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d396368-727f-450e-b820-081a8e072d2d
📒 Files selected for processing (1)
samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt
| } catch (_: Exception) { | ||
| state.update { | ||
| it.copy( | ||
| migrationStatus = MigrationStatus.IDLE, | ||
| isMigrationNeeded = false | ||
| ) | ||
| } |
There was a problem hiding this comment.
Don’t convert check failures into “no migration needed.”
On Line 97, exceptions are mapped to isMigrationNeeded = false, which hides real check failures and can mislead test results. Prefer surfacing this as a failed check state.
🐛 Proposed fix
- } catch (_: Exception) {
+ } catch (e: Exception) {
state.update {
it.copy(
- migrationStatus = MigrationStatus.IDLE,
- isMigrationNeeded = false
+ migrationStatus = MigrationStatus.FAILED,
+ isMigrationNeeded = null,
+ errorMessage = e.message ?: "Failed to check migration status."
)
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authmigration/AuthMigrationViewModel.kt`
around lines 93 - 99, In the catch block inside AuthMigrationViewModel where
state.update currently sets migrationStatus = MigrationStatus.IDLE and
isMigrationNeeded = false, do not convert exceptions into "no migration needed";
instead set migrationStatus to a dedicated failure state (e.g.,
MigrationStatus.CHECK_FAILED — add this enum value if it doesn't exist) and do
not force isMigrationNeeded = false (leave it unchanged or null), and log the
exception so the failed check is visible; update the state.update call and add
exception logging accordingly.
JIRA Ticket
SDKS-4915 Create Example UI demonstrating progress-tracked migration in the Test app
Description
Summary
pingsampleappthat allows developers to test migration of legacy ForgeRock Authenticator credentials (OATH and Push) to the modern Ping SDK storage formatAuthMigrationView/AuthMigrationViewModel) adapted for Android with Jetpack Compose and Material 3Changes
New Files
AuthMigrationScreen.kt— Composable UI with 4 card sections: Instructions (numbered steps), Migration Status (with dynamic badge), Progress (step list with status icons), and Result (success/error)AuthMigrationViewModel.kt— ViewModel that checks for legacy data viaDefaultStorageClientProvider.isMigrationRequired()and runsAuthMigration.start()with real-time progress tracking throughFlowCollector<MigrationProgress>Modified Files
build.gradle.kts— Addedmfa:auth-migrationandfoundation:migrationdependencies; addedpickFirstsrule forlibtool-file.soJNI conflict betweendevice-rootand transitiveforgerock-coreNavigation.kt— AddedAUTH_MIGRATIONroute and composable entry pointHomeApp.kt— AddedonAuthMigrationClickcallback andIconRowItemcard (SwapHoriz icon) in the MFA sectionstrings.xml— Added 22 string resources for the migration screenSummary by CodeRabbit
New Features
Chores