Skip to content

Update VerifyExistingVault screen design and routing#4002

Merged
johnnyluo merged 7 commits into
mainfrom
aminsato/issue_3956
Apr 5, 2026
Merged

Update VerifyExistingVault screen design and routing#4002
johnnyluo merged 7 commits into
mainfrom
aminsato/issue_3956

Conversation

@aminsato
Copy link
Copy Markdown
Collaborator

@aminsato aminsato commented Apr 4, 2026

Summary

  • Added Route.VerifyExistingVault and wired it into NavGraph so vault settings routes single keygen through the verify existing vault screen
  • Updated VerifyExistingVaultScreen with dedicated string resources for email/password steps and simplified the button label logic
  • Adjusted InProgress step border width from 3dp to 1.5dp
  • Added localized strings (verify_existing_vault_email_description, verify_existing_vault_password_title, verify_existing_vault_password_description) across all 10 locale files

Closes #3956
Closes #3957

Test plan

  • Navigate to vault settings → single keygen and verify it routes through VerifyExistingVaultScreen
  • Confirm email step shows correct description text
  • Confirm password step shows correct title and description
  • Verify layout matches Figma design
  • Test all supported locales display correct translations

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added a two-step "Verify Existing Vault" flow (email then password) with inline validation, error messages, clear-input, password visibility toggle, Back/Next handling, and loading state.
    • New Compose screen and navigation route to enter/verify existing vault credentials and continue to key discovery on success.
  • Localization
    • Added localized UI text for the verification flow in multiple languages.

…ting vault keygen

Route vault settings single keygen through VerifyExistingVault screen with dedicated
string resources for email/password steps and UI refinements.

Co-Authored-By: aminsato <Amin.saradar@yahoo.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 4, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Introduces a two-step "Verify Existing Vault" flow: new route and serializable Route, Compose screen, Hilt ViewModel, UI state/events, repositories-driven password verification, navigation changes from VaultSettings, and localized strings for the new UI.

Changes

Cohort / File(s) Summary
New ViewModel & UI state
app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt
Adds VerifyExistingVaultViewModel, VerifyExistingVaultUiState, VerifyExistingVaultEvent, text-field states, step state mapping, event handling, password verification logic via repositories, and navigation calls.
New Compose screen
app/src/main/java/com/vultisig/wallet/ui/screens/keygen/VerifyExistingVaultScreen.kt
Adds two composable overloads for the two-step Email→Password UI, step indicator, input field wiring, previews, test tags, and password visibility handling.
Navigation additions
app/src/main/java/com/vultisig/wallet/ui/navigation/Navigation.kt, app/src/main/java/com/vultisig/wallet/ui/navigation/NavGraph.kt
Adds serializable Route.VerifyExistingVault(name, tssAction, vaultId) and a composable mapping in the SetupNavGraph to host the new screen.
VaultSettings navigation change
app/src/main/java/com/vultisig/wallet/ui/screens/vault_settings/VaultSettingsViewModel.kt
When hasFastSign is true, route changed to Route.VerifyExistingVault(...) (parameter name tssAction used).
Localization
app/src/main/res/values/.../strings.xml
app/src/main/res/values/strings.xml
Adds verify_existing_vault_email_description, verify_existing_vault_password_title, and verify_existing_vault_password_description across base and multiple locale string files (de, es, hr, it, ko, nl, pt, ru, zh-rCN).

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant UI as VerifyExistingVaultScreen
  participant VM as VerifyExistingVaultViewModel
  participant VR as VaultRepository
  participant VS as VultiSignerRepository
  participant Nav as Navigator

  User->>UI: input email -> Next
  UI->>VM: onEvent(Next)
  VM->>VM: validate email -> advance to Password step
  UI->>User: show password input
  User->>UI: input password -> Next
  UI->>VM: onEvent(Next)
  VM->>VR: loadVault(vaultId)
  VR-->>VM: vault (or null)
  VM->>VS: checkPassword(vault.pubKeyECDSA, password)
  VS-->>VM: PasswordCheckResult (Valid / Invalid / Error)
  alt Valid
    VM->>Nav: navigate(Route.Keygen.PeerDiscovery)
  else Invalid or Error
    VM->>UI: update password innerState & errorMessage
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • rkokhatskyi
  • johnnyluo

Poem

"A rabbit hopped to check a vault,
Two steps of care — email then vault password,
I nibbled code and tied each route,
Hilt gave me beans, Compose gave me oats,
Hooray — the vault is verified! 🐇🔑"

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR claims to close #3956 and #3957 but provides no evidence of changes to VerifyExistingVaultPasswordScreen or FastVaultEmailScreen files, and no before/after screenshots required by both issues' acceptance criteria are mentioned. Implement required changes to VerifyExistingVaultPasswordScreen and FastVaultEmailScreen to match Figma designs, and include before/after screenshots as specified in issue acceptance criteria.
Out of Scope Changes check ⚠️ Warning The PR introduces a new VerifyExistingVaultViewModel and VerifyExistingVaultScreen with full business logic implementation, which extends beyond the Figma design update scope stated in linked issues #3956 and #3957. Clarify whether the ViewModel implementation and business logic changes are intended, or separate this feature work from the design-matching PR to align with the stated issue objectives.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Update VerifyExistingVault screen design and routing' accurately summarizes the main changes, which introduce a new Route.VerifyExistingVault, wire it into NavGraph, update VerifyExistingVaultScreen with localized strings, and adjust the routing from VaultSettings.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch aminsato/issue_3956

Comment @coderabbitai help to get the list of available commands and usage tips.

Co-Authored-By: aminsato <Amin.saradar@yahoo.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt`:
- Around line 1-365: The file fails ktfmt formatting; run the Kotlin formatter
on VerifyExistingVaultViewModel.kt (e.g., run ktfmt or the project's gradle
ktfmtFormat/ktfmtApply task or use IDE Kotlin code style -> Reformat) and commit
the formatted file so class VerifyExistingVaultViewModel and related
declarations (functions like observeActiveStepState, collectStepStates,
validateCurrentStep, verifyPasswordAndNavigate) conform to the project's ktfmt
rules and the CI ktfmtCheckMain passes.
- Around line 169-203: collectStepStates recomputes the step map,
inputTextFieldState and hint but does not reset isNextButtonEnabled, so when
activeStep changes the new step can inherit the previous step's enabled state;
update the uiState.update call inside collectStepStates (the block combining
activeStep and steps) to set isNextButtonEnabled = false when writing the new
state (use the same uiState.update invocation that sets stepAndStates,
inputTextFieldState, and textFieldHint) so VerifyExistingVaultStepType
transitions (and associated emailTextFieldState/passwordTextFieldState) always
start with Next disabled.
- Around line 225-236: prev() currently allows changing steps while
verifyPasswordAndNavigate() is still running; add a guard so prev() returns
early if verification is in progress. Specifically, use the ViewModel's uiState
boolean flag that indicates ongoing work (e.g., isVerifying/isBusy) or add one
to the state, set it true at the start of verifyPasswordAndNavigate() and false
on completion/error, and in prev() check that flag before computing nextIndex;
if verification is running, do not navigate or change activeStep. Ensure
references to verifyPasswordAndNavigate(), prev(), uiState, activeStep,
stepAndStates, and navigator.navigate remain consistent.

In
`@app/src/main/java/com/vultisig/wallet/ui/screens/keygen/VerifyExistingVaultScreen.kt`:
- Around line 164-167: The UI currently applies
Modifier.shinedBottom(Theme.v2.colors.alerts.success) for every
non-VerifyExistingVaultStepState.Done state which causes inactive circles to
show the success shine; change the condition so the success shine is applied
only when this particular step is the active/in-progress step (e.g., compare the
step's identifier/index to the current active step or use an isActive boolean
exposed to VerifyExistingVaultScreen) rather than any state != Done, updating
the code that uses shinedBottom in VerifyExistingVaultScreen so only the active
step receives the green highlight.
- Around line 1-219: The file fails ktfmt formatting; run the project's ktfmt
formatter (or apply the project's Kotlin/IDE formatting rules) on
VerifyExistingVaultScreen.kt to fix whitespace, import ordering and line breaks
so ktfmtCheckMain passes; target the top-level Composables
VerifyExistingVaultScreen (both overloads), VerifyExistingVaultSteps,
VerifyExistingVaultStepIcon and the VerifyExistingVaultTags object when
verifying formatting changes. Ensure you commit the reformatted file without
changing behavior.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 86067abd-f769-43ff-b679-511ba9e3ecf6

📥 Commits

Reviewing files that changed from the base of the PR and between f7405fa and 38485f8.

📒 Files selected for processing (15)
  • app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt
  • app/src/main/java/com/vultisig/wallet/ui/navigation/NavGraph.kt
  • app/src/main/java/com/vultisig/wallet/ui/navigation/Navigation.kt
  • app/src/main/java/com/vultisig/wallet/ui/screens/keygen/VerifyExistingVaultScreen.kt
  • app/src/main/java/com/vultisig/wallet/ui/screens/vault_settings/VaultSettingsViewModel.kt
  • app/src/main/res/values-de/strings.xml
  • app/src/main/res/values-es/strings.xml
  • app/src/main/res/values-hr/strings.xml
  • app/src/main/res/values-it/strings.xml
  • app/src/main/res/values-ko/strings.xml
  • app/src/main/res/values-nl/strings.xml
  • app/src/main/res/values-pt/strings.xml
  • app/src/main/res/values-ru/strings.xml
  • app/src/main/res/values-zh-rCN/strings.xml
  • app/src/main/res/values/strings.xml

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt (2)

203-208: ⚠️ Potential issue | 🟡 Minor

Reset CTA enabled state when switching steps.

collectStepStates() updates the active field/hint but does not reset isNextButtonEnabled, so Password can inherit Email’s enabled state.

Suggested fix
                 uiState.update {
                     it.copy(
                         stepAndStates = map,
                         inputTextFieldState = textFieldState,
                         textFieldHint = hint,
+                        isNextButtonEnabled = false,
                     )
                 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt`
around lines 203 - 208, collectStepStates() updates stepAndStates,
inputTextFieldState and textFieldHint via uiState.update but doesn't reset
isNextButtonEnabled, causing a newly selected step (e.g., Password) to inherit
the previous step's enabled state; modify the uiState.update call in
VerifyExistingVaultViewModel.collectStepStates() to explicitly set
isNextButtonEnabled = false (or compute the correct default) whenever you swap
stepAndStates/inputTextFieldState/textFieldHint so the next-button state is
reset on step change.

234-245: ⚠️ Potential issue | 🟠 Major

Prevent back-step changes while verification is running.

prev() still allows step changes during verifyPasswordAndNavigate(), so successful verification can navigate forward from a stale UI step.

Suggested fix
     private fun prev() {
+        if (uiState.value.isLoading) return
         viewModelScope.launch {
             val currentStep = uiState.value.activeStep
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt`
around lines 234 - 245, The prev() function allows navigating back while a
verification is in progress; guard against this by checking a
verification-in-progress flag on uiState before changing steps (e.g., if
uiState.value.isVerifying return early), and ensure verifyPasswordAndNavigate()
sets and clears that flag on start/finish so the UIState (used by prev(),
uiState.update { ... }, and the verifyPasswordAndNavigate() coroutine) prevents
step changes during verification; add the boolean to the state, set it true at
the start of verifyPasswordAndNavigate() and false on all exits, and make prev()
return immediately when that flag is true.
🧹 Nitpick comments (1)
app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt (1)

87-98: Use sealed UI states instead of mixed flags in one data class.

This state model allows invalid combinations (isLoading + error + editable state together). For this ViewModel, a sealed UI state hierarchy would align with project rules and make transitions safer.

As per coding guidelines, "Use sealed classes for UI state (Loading, Success, Error)" and "use sealed state classes for error states."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt`
around lines 87 - 98, Replace the flat VerifyExistingVaultUiState data class
with a sealed UI-state hierarchy (sealed class VerifyExistingVaultUiState with
subclasses like Loading, Input, Success, Error) so invalid flag combinations
(e.g., isLoading + error + editable) cannot occur; move relevant fields into the
appropriate subclass variants (e.g., Loading contains no inputs, Input contains
inputTextFieldState, textFieldHint, innerState, isPasswordVisible,
isNextButtonEnabled, stepAndStates and activeStep belong where appropriate,
Error holds UiText errorMessage), update the ViewModel to emit the new sealed
subclasses and adjust any consumers to switch/when on VerifyExistingVaultUiState
instead of reading multiple boolean flags.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In
`@app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt`:
- Around line 203-208: collectStepStates() updates stepAndStates,
inputTextFieldState and textFieldHint via uiState.update but doesn't reset
isNextButtonEnabled, causing a newly selected step (e.g., Password) to inherit
the previous step's enabled state; modify the uiState.update call in
VerifyExistingVaultViewModel.collectStepStates() to explicitly set
isNextButtonEnabled = false (or compute the correct default) whenever you swap
stepAndStates/inputTextFieldState/textFieldHint so the next-button state is
reset on step change.
- Around line 234-245: The prev() function allows navigating back while a
verification is in progress; guard against this by checking a
verification-in-progress flag on uiState before changing steps (e.g., if
uiState.value.isVerifying return early), and ensure verifyPasswordAndNavigate()
sets and clears that flag on start/finish so the UIState (used by prev(),
uiState.update { ... }, and the verifyPasswordAndNavigate() coroutine) prevents
step changes during verification; add the boolean to the state, set it true at
the start of verifyPasswordAndNavigate() and false on all exits, and make prev()
return immediately when that flag is true.

---

Nitpick comments:
In
`@app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt`:
- Around line 87-98: Replace the flat VerifyExistingVaultUiState data class with
a sealed UI-state hierarchy (sealed class VerifyExistingVaultUiState with
subclasses like Loading, Input, Success, Error) so invalid flag combinations
(e.g., isLoading + error + editable) cannot occur; move relevant fields into the
appropriate subclass variants (e.g., Loading contains no inputs, Input contains
inputTextFieldState, textFieldHint, innerState, isPasswordVisible,
isNextButtonEnabled, stepAndStates and activeStep belong where appropriate,
Error holds UiText errorMessage), update the ViewModel to emit the new sealed
subclasses and adjust any consumers to switch/when on VerifyExistingVaultUiState
instead of reading multiple boolean flags.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 34df0556-bd58-4686-ac22-4b2a03eefab2

📥 Commits

Reviewing files that changed from the base of the PR and between 38485f8 and 2a99d5f.

📒 Files selected for processing (2)
  • app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt
  • app/src/main/java/com/vultisig/wallet/ui/screens/keygen/VerifyExistingVaultScreen.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/src/main/java/com/vultisig/wallet/ui/screens/keygen/VerifyExistingVaultScreen.kt

Reset isNextButtonEnabled when active step changes to prevent stale
button state, and block Back navigation while password verification
is in progress.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt (1)

145-150: Prefer an explicit step list over stepAndStates.keys for flow order.

Back/next currently depend on the map's iteration order. A dedicated step list makes the transition order obvious and keeps navigation independent from how the UI map is built.

♻️ Suggested refactor
+    private val steps =
+        listOf(
+            VerifyExistingVaultStepType.Email,
+            VerifyExistingVaultStepType.Password,
+        )
+
     private fun initStepStates() {
         val stepAndStates =
-            mapOf(
-                VerifyExistingVaultStepType.Email to VerifyExistingVaultStepState.InProgress,
-                VerifyExistingVaultStepType.Password to VerifyExistingVaultStepState.Inactive,
-            )
+            steps.associateWith { step ->
+                if (step == VerifyExistingVaultStepType.Email) {
+                    VerifyExistingVaultStepState.InProgress
+                } else {
+                    VerifyExistingVaultStepState.Inactive
+                }
+            }
         uiState.update { it.copy(stepAndStates = stepAndStates) }
     }
@@
     private fun collectStepStates() {
         uiState
             .map { it.activeStep }
             .distinctUntilChanged()
-            .combine(uiState.map { it.stepAndStates.keys }) { activeStep, steps ->
+            .onEach { activeStep ->
                 val map =
                     steps.associateWith { step ->
                         when {
                             step == activeStep -> VerifyExistingVaultStepState.InProgress
                             step.ordinal < activeStep.ordinal -> VerifyExistingVaultStepState.Done
@@
                 uiState.update {
                     it.copy(
                         stepAndStates = map,
                         inputTextFieldState = textFieldState,
                         textFieldHint = hint,
                         isNextButtonEnabled = isNextButtonEnabled,
                     )
                 }
             }
             .launchIn(viewModelScope)
     }
@@
-            val currentStepIndex = uiState.value.stepAndStates.keys.indexOf(currentStep)
+            val currentStepIndex = steps.indexOf(currentStep)
             val nextIndex = currentStepIndex - 1
@@
-            val newStep = uiState.value.stepAndStates.keys.elementAt(nextIndex)
+            val newStep = steps[nextIndex]
             uiState.update { it.copy(activeStep = newStep) }
@@
-            val currentStepIndex = uiState.value.stepAndStates.keys.indexOf(currentStep)
+            val currentStepIndex = steps.indexOf(currentStep)
             val nextIndex = currentStepIndex + 1
-            if (nextIndex >= uiState.value.stepAndStates.size) {
+            if (nextIndex >= steps.size) {
                 verifyPasswordAndNavigate()
                 return@launch
             }
-            val newStep = uiState.value.stepAndStates.keys.elementAt(nextIndex)
+            val newStep = steps[nextIndex]
             uiState.update { it.copy(activeStep = newStep) }

Also applies to: 175-219, 246-253, 264-271

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt`
around lines 145 - 150, The current navigation relies on the map iteration order
of stepAndStates (constructed from VerifyExistingVaultStepType keys), which is
brittle; introduce an explicit ordered list (e.g., orderedSteps or stepOrder:
List<VerifyExistingVaultStepType>) and store it in the UI state alongside
stepAndStates, then change all places that iterate stepAndStates.keys or assume
map order (including logic in methods handling back/next navigation and any
references around verify flow) to use this ordered list for determining
previous/next steps; update uiState.update to set both stepAndStates and the new
orderedSteps and refactor navigation helpers to use orderedSteps instead of map
keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt`:
- Around line 297-300: In VerifyExistingVaultViewModel's onError lambda, stop
using the raw exception text (e.message) for passwordErrorMessage; keep
Timber.e(e, ...) to log details, but map the UI error to a stable localized
fallback (e.g., set passwordErrorMessage to a UiText.StringResource pointing to
a "invalid password" / "unable to verify password" string ID) and retain
passwordInnerState.value = VsTextInputFieldInnerState.Error; ensure no exception
details are shown to the user and only logged via Timber.

---

Nitpick comments:
In
`@app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt`:
- Around line 145-150: The current navigation relies on the map iteration order
of stepAndStates (constructed from VerifyExistingVaultStepType keys), which is
brittle; introduce an explicit ordered list (e.g., orderedSteps or stepOrder:
List<VerifyExistingVaultStepType>) and store it in the UI state alongside
stepAndStates, then change all places that iterate stepAndStates.keys or assume
map order (including logic in methods handling back/next navigation and any
references around verify flow) to use this ordered list for determining
previous/next steps; update uiState.update to set both stepAndStates and the new
orderedSteps and refactor navigation helpers to use orderedSteps instead of map
keys.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: de2c2251-f97b-4c21-a933-73e0f313a04a

📥 Commits

Reviewing files that changed from the base of the PR and between 2a99d5f and a1696c1.

📒 Files selected for processing (1)
  • app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt

johnnyluo and others added 2 commits April 5, 2026 11:30
- Make uiState private MutableStateFlow, expose as StateFlow
- Restore @immutable on UiState by moving TextFieldState out
- Move VerifyExistingVaultStepType and StepState enums to Screen file
  to remove Compose UI types (Color, Dp) from ViewModel layer
- Apply shinedBottom only for InProgress step, not all non-Done steps
- Replace raw exception text with localized string in error handler
- Consolidate isNextButtonEnabled into single source of truth via combine

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt (1)

245-249: ⚠️ Potential issue | 🟡 Minor

Use one localized fallback for unexpected password-check failures.

onError currently shows the invalid-password copy for any thrown exception, while PasswordCheckResult.Error exposes result.message directly. Unexpected failures shouldn’t either masquerade as bad credentials or leak repository text; add one generic localized resource and use it in both branches.

🧩 Suggested fix
         viewModelScope.safeLaunch(
             onError = { e ->
                 Timber.e(e, "Failed to verify vault password")
                 passwordInnerState.value = VsTextInputFieldInnerState.Error
-                passwordErrorMessage.value = StringResource(R.string.fast_vault_invalid_password)
+                passwordErrorMessage.value =
+                    StringResource(R.string.verify_existing_vault_unable_to_verify_password)
                 _uiState.update { it.copy(isLoading = false) }
             }
         ) {
@@
                 is PasswordCheckResult.Error -> {
                     passwordInnerState.value = VsTextInputFieldInnerState.Error
-                    passwordErrorMessage.value = UiText.DynamicString(result.message)
+                    passwordErrorMessage.value =
+                        StringResource(R.string.verify_existing_vault_unable_to_verify_password)
                 }

Also applies to: 282-285

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt`
around lines 245 - 249, The onError lambda and the PasswordCheckResult.Error
branch in VerifyExistingVaultViewModel currently surface either the "invalid
password" string or the raw repository message; create a single generic
localized string (e.g., R.string.fast_vault_password_check_failed) and set
passwordErrorMessage.value to
StringResource(R.string.fast_vault_password_check_failed) for unexpected
failures: in onError replace the current invalid-password resource with the
generic resource, and in the PasswordCheckResult.Error branch use the generic
resource instead of result.message (or use it when result.message is
null/empty), leaving the explicit invalid-password resource only for the
PasswordCheckResult.InvalidPassword case; update usages of passwordErrorMessage
and any tests accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@app/src/main/java/com/vultisig/wallet/ui/screens/keygen/VerifyExistingVaultScreen.kt`:
- Around line 171-177: The Modifier.shinedBottom is currently only applied when
state == VerifyExistingVaultStepState.InProgress, but per spec it should also
apply for VerifyExistingVaultStepState.Inactive and be omitted only for
VerifyExistingVaultStepState.Done; update the conditional inside
VerifyExistingVaultStepIcon (the .then(...) block) to apply
Modifier.shinedBottom(color = Theme.v2.colors.alerts.success) when state is
InProgress OR Inactive and return plain Modifier only when state is Done,
keeping the same color and placement.

---

Duplicate comments:
In
`@app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt`:
- Around line 245-249: The onError lambda and the PasswordCheckResult.Error
branch in VerifyExistingVaultViewModel currently surface either the "invalid
password" string or the raw repository message; create a single generic
localized string (e.g., R.string.fast_vault_password_check_failed) and set
passwordErrorMessage.value to
StringResource(R.string.fast_vault_password_check_failed) for unexpected
failures: in onError replace the current invalid-password resource with the
generic resource, and in the PasswordCheckResult.Error branch use the generic
resource instead of result.message (or use it when result.message is
null/empty), leaving the explicit invalid-password resource only for the
PasswordCheckResult.InvalidPassword case; update usages of passwordErrorMessage
and any tests accordingly.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 763ff43b-ac4d-4d37-b1c3-1fc93f2125a4

📥 Commits

Reviewing files that changed from the base of the PR and between 8de8649 and c8c869d.

📒 Files selected for processing (2)
  • app/src/main/java/com/vultisig/wallet/ui/models/keygen/VerifyExistingVaultViewModel.kt
  • app/src/main/java/com/vultisig/wallet/ui/screens/keygen/VerifyExistingVaultScreen.kt

Copy link
Copy Markdown
Contributor

@johnnyluo johnnyluo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@johnnyluo johnnyluo enabled auto-merge April 5, 2026 09:31
@johnnyluo johnnyluo added this pull request to the merge queue Apr 5, 2026
Merged via the queue into main with commit 981a74c Apr 5, 2026
2 checks passed
@johnnyluo johnnyluo deleted the aminsato/issue_3956 branch April 5, 2026 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update FastVaultEmailScreen design to match Figma Update VerifyExistingVaultPasswordScreen design to match Figma

2 participants