Conversation
Ni3verma
added
the
ignore_for_release_notes
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds Hilt instrumentation support and broad Android testing coverage for authentication, records, backup and restore, settings, navigation, Compose screens, single-record behavior, migrations, scrolling, and visual transformations. ChangesSafeBox testing and application workflows
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (14)
app/src/androidTest/java/com/andryoga/safebox/e2e/AddNewRecordFlowsE2ETest.kt (1)
177-177: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUse a well-known test PAN instead of a novel-looking card number.
Static analysis flags this 16-digit literal as a possible real credit-card PAN. It's test fixture data, but using a recognized dummy value (e.g. the standard test Visa
4111111111111111) avoids tripping secret-scanning/CI gates on this and future PRs.🛡️ Suggested fix
- .performTextInput("4111222233334444") + .performTextInput("4111111111111111")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/e2e/AddNewRecordFlowsE2ETest.kt` at line 177, Replace the literal passed to performTextInput in the affected E2E test with the recognized dummy Visa PAN 4111111111111111, preserving the rest of the test flow unchanged.Source: Linters/SAST tools
app/src/androidTest/java/com/andryoga/safebox/e2e/SignupAndLoginE2ETest.kt (1)
83-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
E2ETestUtilsconstants/resources instead of duplicating literals.Lines 84/107 hardcode
"Qwerty@@123"and line 91 hardcodes"E2E hint"rather than reusingE2ETestUtils.TEST_MASTER_PASSWORD/TEST_MASTER_HINT. Also, line 83-84 matches the password field via a hardcoded"Password"literal instead ofcontext.getString(R.string.password), unlike the equivalent matcher at Line 131-133 in this same file. If the credential constant or string resource changes, these assertions silently diverge and can fail in confusing ways.♻️ Suggested fix
- composeTestRule.onNode(hasSetTextAction() and hasText("Password", substring = true)) - .performTextReplacement("Qwerty@@123") + composeTestRule.onNode( + hasSetTextAction() and hasText(context.getString(R.string.password), substring = true) + ) + .performTextReplacement(E2ETestUtils.TEST_MASTER_PASSWORD) composeTestRule.onNode( hasSetTextAction() and hasText( context.getString(R.string.hint), substring = true ) ) - .performTextReplacement("E2E hint") + .performTextReplacement(E2ETestUtils.TEST_MASTER_HINT)- val isPasswordCorrect = userDetailsRepository.checkPassword("Qwerty@@123") + val isPasswordCorrect = userDetailsRepository.checkPassword(E2ETestUtils.TEST_MASTER_PASSWORD)Also applies to: 91-91, 107-107
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/e2e/SignupAndLoginE2ETest.kt` around lines 83 - 84, Update SignupAndLoginE2ETest to reuse E2ETestUtils.TEST_MASTER_PASSWORD for both password replacements and E2ETestUtils.TEST_MASTER_HINT for the hint value. In the password-field matcher near the first replacement, replace the hardcoded “Password” text with context.getString(R.string.password), matching the existing equivalent matcher elsewhere in the test.app/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.kt (1)
47-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated wait-then-assert-then-act boilerplate.
The
waitUntil { runCatching { ... }.getOrDefault(false) }pattern is duplicated three times (welcome_back text, add-button content description, option text) with only the matcher and timeout differing.♻️ Suggested helper
private fun waitForNode( composeTestRule: ComposeTestRule, timeoutMillis: Long = 5000L, matcher: SemanticsMatcher ) { composeTestRule.waitUntil(timeoutMillis = timeoutMillis) { runCatching { composeTestRule.onAllNodes(matcher).fetchSemanticsNodes().isNotEmpty() }.getOrDefault(false) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.kt` around lines 47 - 98, Extract the duplicated waitUntil/runCatching node-detection logic into a private waitForNode helper accepting ComposeTestRule, SemanticsMatcher, and an optional timeoutMillis defaulting to 5000L. Update unlockApp to use it with the welcome_back matcher and 15000L timeout, and clickAddNewRecordOption to use it for the add-button content-description and option-text matchers, preserving the existing assertions and actions.app/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsE2ETest.kt (1)
84-458: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated setup/launch/unlock boilerplate across all 7 test methods.
Every test repeats the identical
E2ETestUtils.setupUnlockedHomeState(...)call followed byActivityScenario.launch(MainActivity::class.java).use { E2ETestUtils.unlockApp(...) }. Consider extracting a shared helper (e.g., inE2ETestUtils) that combines setup + launch + unlock into a single call returning the scenario, to reduce repetition as this suite grows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsE2ETest.kt` around lines 84 - 458, Extract the repeated setup, MainActivity launch, and unlock sequence from the seven test methods into a shared helper, preferably in E2ETestUtils, that returns or scopes the ActivityScenario. Replace each test’s duplicated setup and launch block with this helper while preserving the existing scenario lifecycle and unlocked-home behavior.app/src/androidTest/java/com/andryoga/safebox/e2e/RecordsScreenScrollAndAppBarTest.kt (2)
35-37: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest scrolls but never verifies the Top App Bar afterward, despite its name and class doc.
The class doc promises coverage of Top App Bar collapsing/visibility, and this test is named
...shouldKeepListAccessibleAndCheckTopBar. It only checks the add-button's visibility before scrolling (Line 125); afterperformScrollToIndex(40)(Line 128) there's no follow-up assertion on the app bar's state. As written, the test doesn't actually validate what its name/doc claim.Also applies to: 103-130
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/e2e/RecordsScreenScrollAndAppBarTest.kt` around lines 35 - 37, Update the test method shouldKeepListAccessibleAndCheckTopBar to assert the Top App Bar’s expected collapsed or visibility state after performScrollToIndex(40). Keep the existing pre-scroll add-button assertion and list accessibility checks, and add the follow-up assertion only after scrolling so the test validates its documented behavior.
133-169: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winScroll-to-end index relies on an undocumented off-by-one cancellation from the list's header item.
performScrollToIndex(combinedSorted.size)(Line 165) targets the last record correctly only becauseRecordsScreen.kt'sLazyColumnprepends aFilterRowas item 0 (per the providedRecordsScreen.ktsnippet), shifting every record's index up by one — socombinedSorted.sizehappens to equal the last record's list index. This is correct today, but silently breaks (pointing at the wrong index) if the header composition inRecordsScreen.ktever changes (e.g., an added header, or conditional header). Consider a comment documenting the+1header offset, or computing it explicitly (e.g., a namedheaderItemCountconstant) so the coupling is visible.Separately,
onNodeWithText(lastRecord.title, substring = true)(Line 166) assumeslastRecord.titleis unique among the 200 randomly generated records; ifRandomUserDatacan produce duplicate/overlapping titles, this could match multiple nodes and fail.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/e2e/RecordsScreenScrollAndAppBarTest.kt` around lines 133 - 169, Make the scroll target in scrollLongPopulatedListToEnd_lastRecordShouldBeCompletelyVisibleAndUnobstructed explicitly account for the RecordsScreen header by introducing a named header-item count or equivalent documented offset, rather than relying on combinedSorted.size implicitly matching the final record index. Also replace the ambiguous lastRecord.title text lookup with a selector or assertion that uniquely identifies the final record, using stable record semantics available in the test.app/src/androidTest/java/com/andryoga/safebox/ui/home/records/RecordsScreenTest.kt (1)
58-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest name promises filter-chip verification that never happens.
filteredOutRecordsState_shouldShowNoResultsFoundAndFiltersonly asserts the "no filtered record" title/body text; it never asserts that filter chips are rendered, despite the "AndFilters" in its name.♻️ Suggested addition
composeTestRule.onNodeWithText(context.getString(R.string.no_filtered_record_body)) .assertIsDisplayed() + composeTestRule.onNodeWithText(context.getString(R.string.type_display_login)) + .assertIsDisplayed() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/ui/home/records/RecordsScreenTest.kt` around lines 58 - 79, Update filteredOutRecordsState_shouldShowNoResultsFoundAndFilters to also assert that the expected filter chips are displayed, using the same node or text selectors established by the RecordsScreen UI. Keep the existing no-results title and body assertions unchanged.app/src/androidTest/java/com/andryoga/safebox/ui/login/LoginScreenTest.kt (1)
55-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest doesn't actually verify the visual transformation toggled.
Only the icon's continued visibility is asserted; the field's displayed text/transformation state is never checked, so the test would pass even if toggling were broken.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/ui/login/LoginScreenTest.kt` around lines 55 - 70, Update clickTogglePasswordIcon_shouldToggleVisualTransformation to assert the password field’s displayed text or visual transformation before and after performClick, using the relevant node semantics or test tags exposed by LoginScreen. Keep the existing icon visibility assertions, but ensure the test fails when clicking the toggle does not change the field’s transformation state.app/src/androidTest/java/com/andryoga/safebox/ui/signup/SignupScreenTest.kt (1)
115-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winToggle test doesn't verify the transformation itself, and uses a hardcoded content-description literal.
Two issues:
- Like the analogous
LoginScreenTesttest, this only asserts the icon stays displayed — it never checks whether the password's visual transformation actually changed."Toggle Password"is hardcoded here (Line 126) instead of using the string resource, unlike the equivalentLoginScreenTestlookup viacontext.getString(R.string.cd_toggle_sensitive_data_visibility). If the resource copy changes, this test silently breaks without touching source-of-truth strings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/ui/signup/SignupScreenTest.kt` around lines 115 - 129, Update clickTogglePasswordIcon_shouldToggleVisualTransformation to assert the password field’s visual transformation changes after clicking the toggle, matching the analogous LoginScreenTest behavior. Replace the hardcoded "Toggle Password" content description with context.getString(R.string.cd_toggle_sensitive_data_visibility) for both node lookups, while retaining the existing displayed-state assertions.app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenScrollTest.kt (2)
174-226: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame fully-qualified-name readability concern as in
SingleRecordScreenTest.kt.Consider importing
LayoutPlan,LayoutId,FieldId,FieldUiStatehere too rather than repeating fully-qualified paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenScrollTest.kt` around lines 174 - 226, In viewMode_longNotes_shouldBeScrollableAndDisplayBottomText, import and use the existing LayoutPlan, LayoutId, FieldId, and FieldUiState symbols instead of repeating their fully qualified names. Preserve the test setup and assertions unchanged.
140-152: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssertion doesn't prove the "auto bring into view" behavior.
The test clicks "Notes" and asserts it's displayed afterward, but never establishes that the field was off-screen/not-displayed beforehand, nor checks that a scroll actually happened. Since
SingleRecordScreencomposes all fields in aColumn(not lazily),onNodeWithTextwill find "Notes" andperformClick()/assertIsDisplayed()could pass even if no auto-scroll logic exists. Consider assertingassertDoesNotExist()/an off-screen state before the click, or asserting a scroll-position change, to make the test meaningful.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenScrollTest.kt` around lines 140 - 152, Strengthen clickBottomNotesFieldOnUnscrolledForm_shouldAutoBringFieldIntoView by first verifying that the Notes field is not currently visible or is off-screen, then perform the click and assert that it becomes visible as a result of the auto-scroll behavior. Ensure the assertions distinguish a genuine scroll from mere composition of the field in the non-lazy Column.app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenTest.kt (2)
36-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest name promises more than it asserts.
viewMode_shouldShowActionButtonsAndRenderReadOnlyFieldsonly asserts the action buttons are displayed; it never verifies read-only field rendering (that's covered separately byviewMode_allPopulatedFields_shouldRenderLabelsAndFormattedData). Consider renaming toviewMode_shouldShowActionButtonsto match actual scope, or fold in a field assertion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenTest.kt` around lines 36 - 58, Rename the test method viewMode_shouldShowActionButtonsAndRenderReadOnlyFields to viewMode_shouldShowActionButtons, since it only verifies the edit, share, and delete action buttons and does not assert read-only field rendering.
217-261: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer imports over repeated fully-qualified references.
These tests (and the ones below) repeat fully-qualified names like
com.andryoga.safebox.ui.singleRecord.dynamicLayout.models.LayoutPlan,...FieldId,...FieldUiState,...SpaceAfterEveryFourCharsTransformationinstead of importing them once at the top, unlike the rest of the file (ViewModeis imported). This significantly hurts readability of otherwise straightforward test fixtures.♻️ Example fix (add to top-level imports)
+import com.andryoga.safebox.ui.singleRecord.dynamicLayout.LayoutId +import com.andryoga.safebox.ui.singleRecord.dynamicLayout.models.FieldId +import com.andryoga.safebox.ui.singleRecord.dynamicLayout.models.FieldUiState +import com.andryoga.safebox.ui.singleRecord.dynamicLayout.models.LayoutPlan +import com.andryoga.safebox.ui.singleRecord.dynamicLayout.visualTransformers.SpaceAfterEveryFourCharsTransformationThen drop the fully-qualified prefixes throughout the file.
Also applies to: 288-328, 351-381, 407-426, 455-473
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenTest.kt` around lines 217 - 261, Replace the repeated fully qualified dynamic-layout references in SingleRecordScreenTest, including the fixtures in viewMode_allPopulatedFields_shouldRenderLabelsAndFormattedData and the referenced later ranges, with top-level imports for LayoutPlan, Field, FieldId, FieldUiState, Cell, and SpaceAfterEveryFourCharsTransformation. Remove the package prefixes throughout while preserving the existing test behavior.app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordTopBarTest.kt (1)
104-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest name overstates what's being exercised.
editModeWithValidFields_saveButtonShouldBeVisibleAndEnabled_andInvisibleWhenInvalidimplies field-validation logic is under test, but the body simply flipsisSaveButtonVisibledirectly via a local boolean — no validation logic fromSingleRecordViewModelis exercised. Consider renaming to reflect what's actually tested (e.g.topBarActions_shouldToggleSaveButtonVisibilityWithState), per <ai_summary>/TopBar.kt'sisSaveButtonVisible/isSaveButtonEnabledprops shown in the relevant snippet.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordTopBarTest.kt` around lines 104 - 130, Rename the test function editModeWithValidFields_saveButtonShouldBeVisibleAndEnabled_andInvisibleWhenInvalid to reflect that it only toggles the top bar’s isSaveButtonVisible state, without implying field-validation or ViewModel behavior. Use a name such as topBarActions_shouldToggleSaveButtonVisibilityWithState and leave the test body unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsE2ETest.kt`:
- Line 209: Replace the hardcoded 16-digit values assigned to updatedNumber and
the corresponding value at the second flagged fixture with non-card-like test
data, such as an alphanumeric fixture or shared test constant, while preserving
the test’s intended update and comparison behavior.
- Around line 39-41: The RecordActionsE2ETest class KDoc claims sharing coverage
that the current tests do not provide. Either add a test exercising the
cd_action_share action, or update the class documentation to remove “sharing”
while preserving the descriptions of workflows actually covered.
In `@app/src/androidTest/java/com/andryoga/safebox/e2e/SignupAndLoginE2ETest.kt`:
- Line 8: Update the two E2E tests using createEmptyComposeRule so their Compose
UI interactions are not wrapped in a separate runTest context with an unrelated
scheduler. Remove the outer runTest around the UI-driving code, or explicitly
configure both contexts to share the same test dispatcher and scheduler.
In `@app/src/androidTest/java/com/andryoga/safebox/ui/login/LoginScreenTest.kt`:
- Around line 163-183: Remove the unused biometricSuccessEmitted variable and
BiometricSuccess callback branch from
biometricSuccess_whenBiometricEnabledAndTriggered_shouldEmitBiometricSuccessAction,
then rename the test to describe that it verifies the login screen
configuration/rendering when biometric unlock is enabled. Keep the existing
welcome_back assertion and explanatory platform-window comment.
In `@app/src/androidTest/java/com/andryoga/safebox/ui/signup/SignupScreenTest.kt`:
- Around line 287-296: Update SignupScreenTest so the valid-password step
replaces the existing “abc” value instead of appending to it. In the password
field interaction after the invalid-password assertion, clear the field before
entering “Qwerty@@12”, while preserving the existing validation and
enabled-state assertions.
In
`@app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenTest.kt`:
- Around line 404-449: Strengthen
passwordField_inEditModeShouldShowToggleIconAndMaskUnmaskText_andInvisibleInViewMode
by locating the rendered password text and asserting it is masked before the
toggle click, then asserting SecretPass123 is visible after the click. Keep the
existing toggle-icon presence and VIEW-mode absence assertions unchanged.
---
Nitpick comments:
In
`@app/src/androidTest/java/com/andryoga/safebox/e2e/AddNewRecordFlowsE2ETest.kt`:
- Line 177: Replace the literal passed to performTextInput in the affected E2E
test with the recognized dummy Visa PAN 4111111111111111, preserving the rest of
the test flow unchanged.
In `@app/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.kt`:
- Around line 47-98: Extract the duplicated waitUntil/runCatching node-detection
logic into a private waitForNode helper accepting ComposeTestRule,
SemanticsMatcher, and an optional timeoutMillis defaulting to 5000L. Update
unlockApp to use it with the welcome_back matcher and 15000L timeout, and
clickAddNewRecordOption to use it for the add-button content-description and
option-text matchers, preserving the existing assertions and actions.
In `@app/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsE2ETest.kt`:
- Around line 84-458: Extract the repeated setup, MainActivity launch, and
unlock sequence from the seven test methods into a shared helper, preferably in
E2ETestUtils, that returns or scopes the ActivityScenario. Replace each test’s
duplicated setup and launch block with this helper while preserving the existing
scenario lifecycle and unlocked-home behavior.
In
`@app/src/androidTest/java/com/andryoga/safebox/e2e/RecordsScreenScrollAndAppBarTest.kt`:
- Around line 35-37: Update the test method
shouldKeepListAccessibleAndCheckTopBar to assert the Top App Bar’s expected
collapsed or visibility state after performScrollToIndex(40). Keep the existing
pre-scroll add-button assertion and list accessibility checks, and add the
follow-up assertion only after scrolling so the test validates its documented
behavior.
- Around line 133-169: Make the scroll target in
scrollLongPopulatedListToEnd_lastRecordShouldBeCompletelyVisibleAndUnobstructed
explicitly account for the RecordsScreen header by introducing a named
header-item count or equivalent documented offset, rather than relying on
combinedSorted.size implicitly matching the final record index. Also replace the
ambiguous lastRecord.title text lookup with a selector or assertion that
uniquely identifies the final record, using stable record semantics available in
the test.
In `@app/src/androidTest/java/com/andryoga/safebox/e2e/SignupAndLoginE2ETest.kt`:
- Around line 83-84: Update SignupAndLoginE2ETest to reuse
E2ETestUtils.TEST_MASTER_PASSWORD for both password replacements and
E2ETestUtils.TEST_MASTER_HINT for the hint value. In the password-field matcher
near the first replacement, replace the hardcoded “Password” text with
context.getString(R.string.password), matching the existing equivalent matcher
elsewhere in the test.
In
`@app/src/androidTest/java/com/andryoga/safebox/ui/home/records/RecordsScreenTest.kt`:
- Around line 58-79: Update
filteredOutRecordsState_shouldShowNoResultsFoundAndFilters to also assert that
the expected filter chips are displayed, using the same node or text selectors
established by the RecordsScreen UI. Keep the existing no-results title and body
assertions unchanged.
In `@app/src/androidTest/java/com/andryoga/safebox/ui/login/LoginScreenTest.kt`:
- Around line 55-70: Update
clickTogglePasswordIcon_shouldToggleVisualTransformation to assert the password
field’s displayed text or visual transformation before and after performClick,
using the relevant node semantics or test tags exposed by LoginScreen. Keep the
existing icon visibility assertions, but ensure the test fails when clicking the
toggle does not change the field’s transformation state.
In `@app/src/androidTest/java/com/andryoga/safebox/ui/signup/SignupScreenTest.kt`:
- Around line 115-129: Update
clickTogglePasswordIcon_shouldToggleVisualTransformation to assert the password
field’s visual transformation changes after clicking the toggle, matching the
analogous LoginScreenTest behavior. Replace the hardcoded "Toggle Password"
content description with
context.getString(R.string.cd_toggle_sensitive_data_visibility) for both node
lookups, while retaining the existing displayed-state assertions.
In
`@app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenScrollTest.kt`:
- Around line 174-226: In
viewMode_longNotes_shouldBeScrollableAndDisplayBottomText, import and use the
existing LayoutPlan, LayoutId, FieldId, and FieldUiState symbols instead of
repeating their fully qualified names. Preserve the test setup and assertions
unchanged.
- Around line 140-152: Strengthen
clickBottomNotesFieldOnUnscrolledForm_shouldAutoBringFieldIntoView by first
verifying that the Notes field is not currently visible or is off-screen, then
perform the click and assert that it becomes visible as a result of the
auto-scroll behavior. Ensure the assertions distinguish a genuine scroll from
mere composition of the field in the non-lazy Column.
In
`@app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenTest.kt`:
- Around line 36-58: Rename the test method
viewMode_shouldShowActionButtonsAndRenderReadOnlyFields to
viewMode_shouldShowActionButtons, since it only verifies the edit, share, and
delete action buttons and does not assert read-only field rendering.
- Around line 217-261: Replace the repeated fully qualified dynamic-layout
references in SingleRecordScreenTest, including the fixtures in
viewMode_allPopulatedFields_shouldRenderLabelsAndFormattedData and the
referenced later ranges, with top-level imports for LayoutPlan, Field, FieldId,
FieldUiState, Cell, and SpaceAfterEveryFourCharsTransformation. Remove the
package prefixes throughout while preserving the existing test behavior.
In
`@app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordTopBarTest.kt`:
- Around line 104-130: Rename the test function
editModeWithValidFields_saveButtonShouldBeVisibleAndEnabled_andInvisibleWhenInvalid
to reflect that it only toggles the top bar’s isSaveButtonVisible state, without
implying field-validation or ViewModel behavior. Use a name such as
topBarActions_shouldToggleSaveButtonVisibilityWithState and leave the test body
unchanged.
🪄 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 Plus
Run ID: ef85a1e0-43c8-435c-bb0a-db8cf364f7fd
📒 Files selected for processing (24)
.agents/AGENTS.mdapp/build.gradleapp/src/androidTest/java/com/andryoga/safebox/BaseTestApplication.ktapp/src/androidTest/java/com/andryoga/safebox/CustomHiltTestRunner.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/AddNewRecordFlowsE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/RecordsScreenScrollAndAppBarTest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/SignupAndLoginE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/home/records/RecordsScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/home/records/RecordsSearchBarTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/login/LoginScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/signup/SignupScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenScrollTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordTopBarTest.ktapp/src/main/java/com/andryoga/safebox/ui/home/records/RecordsScreen.ktapp/src/main/java/com/andryoga/safebox/ui/login/LoginScreen.ktapp/src/main/java/com/andryoga/safebox/ui/signup/SignupScreen.ktapp/src/main/java/com/andryoga/safebox/ui/singleRecord/SingleRecordViewModel.ktapp/src/main/res/values/strings.xmlapp/src/test/java/com/andryoga/safebox/ui/singleRecord/SingleRecordViewModelTest.ktapp/src/test/java/com/andryoga/safebox/ui/singleRecord/dynamicLayout/visualTransformers/VisualTransformationsUnitTest.ktgradle/libs.versions.toml
💤 Files with no reviewable changes (2)
- app/src/main/java/com/andryoga/safebox/ui/singleRecord/SingleRecordViewModel.kt
- app/src/test/java/com/andryoga/safebox/ui/singleRecord/SingleRecordViewModelTest.kt
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/androidTest/java/com/andryoga/safebox/e2e/RecordsScreenScrollAndAppBarTest.kt (1)
147-154: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the app bar after scrolling.
The only app-bar assertion runs before
performScrollToIndex, so the test does not verify the behavior claimed by its name. Repeat the visibility assertion after the scroll.Proposed fix
composeTestRule.onNode(hasScrollToIndexAction(), useUnmergedTree = true) .performScrollToIndex(40) +composeTestRule.onNodeWithContentDescription(addNewButtonDesc) + .assertIsDisplayed()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/e2e/RecordsScreenScrollAndAppBarTest.kt` around lines 147 - 154, In the test flow around performScrollToIndex(40), repeat the add-new-record app bar visibility assertion after scrolling. Reuse addNewButtonDesc and the existing onNodeWithContentDescription assertion so the test verifies the app bar remains displayed after the LazyColumn scroll.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/schemas/com.andryoga.safebox.data.db.SafeBoxDatabase/1.json`:
- Line 5: Regenerate the version-1 Room schema fixture for SafeBoxDatabase
instead of retaining the hand-written identityHash and schema metadata. Use the
exported version-1 schema generation flow so
app/schemas/com.andryoga.safebox.data.db.SafeBoxDatabase/1.json matches the
database schema used by existing users and migration tests.
In `@app/src/androidTest/java/com/andryoga/safebox/data/db/MigrationTest.kt`:
- Around line 40-55: Replace Kotlin assert calls throughout MigrationTest with
the project’s Truth/JUnit assertions, including assertions that each cursor’s
moveToFirst() returns true and that migrated column values match expected
values. Apply this consistently to the loginCursor, cardCursor, accountCursor,
and all other assertions in the file, preserving cursor cleanup.
In
`@app/src/androidTest/java/com/andryoga/safebox/e2e/CrossFeatureUserJourneysE2ETest.kt`:
- Around line 202-203: Replace the positional password-field selection in
CrossFeatureUserJourneysE2ETest with a semantic selector that uniquely targets
the master password input, using its label, test tag, or row semantics. Apply
the same change to the password and privacy-control interactions around the
additional referenced section, avoiding indexed access so UI reordering cannot
select the wrong control.
In
`@app/src/androidTest/java/com/andryoga/safebox/e2e/LoginBiometricAndHintE2ETest.kt`:
- Around line 237-285: The test
biometricErrorOrCancellation_shouldResetUiStateAndPreventInfinitePromptLoop
currently bypasses biometric handling and only verifies password login. Add a
controllable BiometricAuthHandler seam, trigger its error or cancellation
callback, and assert the biometric state is cleared before entering the existing
master-password fallback flow; retain the final unlocked-home assertion to
verify fallback succeeds without another prompt.
In
`@app/src/androidTest/java/com/andryoga/safebox/ui/home/backupAndRestore/BackupAndRestoreScreenTest.kt`:
- Around line 153-168: Rename
enterPasswordView_inWrongPasswordState_shouldShowSupportingErrorTextAndRedOutline
to indicate only supporting error text is verified, removing the untested
red-outline claim; keep the existing assertion and test behavior unchanged.
In
`@app/src/androidTest/java/com/andryoga/safebox/ui/home/backupAndRestore/BackupAndRestoreWorkersTest.kt`:
- Around line 285-286: Update the assertion around restoreWorker.doWork() to
require a normal return of Result.failure() rather than accepting any thrown
exception. Invoke doWork() directly and assert its returned value equals
Result.failure(), allowing unexpected crashes to fail the test.
- Line 306: Update the formattedIndex assignment in the backup/restore worker
test to pass Locale.ROOT to String.format, ensuring the zero-padded index always
uses ASCII digits and preserves the expected .bak filenames.
In
`@app/src/androidTest/java/com/andryoga/safebox/ui/home/settings/SettingsScreenComprehensiveTest.kt`:
- Around line 117-126: Update both slider tests in the comprehensive settings
test so assertions execute unconditionally after SetProgress. Remove the
emittedAction null checks, assert emittedAction is the expected
SettingsScreenAction type, and verify its limit value for both the
password-after-biometric and biometric-login slider cases.
In
`@app/src/androidTest/java/com/andryoga/safebox/ui/loading/LoadingScreenRootTest.kt`:
- Around line 46-71: Restore the encrypted CommonConstants.IS_SIGN_UP_REQUIRED
preference after each test in LoadingScreenRootTest. Capture its original value
before mutation and restore it during cleanup, or clear the preference from an
`@After` method, ensuring both boot tests leave persistent storage unchanged.
In `@app/src/androidTest/java/com/andryoga/safebox/ui/login/LoginScreenTest.kt`:
- Around line 188-222: Update
rapidMultiClickOnLoginButton_shouldPreventDuplicateSubmission to assert exactly
one LoginClicked emission when LoginScreen owns duplicate suppression; replace
the greater-than-zero assertion with an exact count and ensure the test
exercises the screen’s suppression behavior. If suppression belongs to the
ViewModel instead, move this coverage there and rename the component test to
reflect that it only verifies click emission.
In
`@app/src/androidTest/java/com/andryoga/safebox/ui/signup/StatefulSignupTestHelper.kt`:
- Around line 23-50: The stateful signup test helper duplicates production
password-validation rules, so it cannot detect validator regressions. Replace
the local validation logic in the helper’s
validatorState/isError/isButtonEnabled flow with SignupViewModel or the same
production validator used by it, while preserving the existing state and button
assertions.
In
`@app/src/main/java/com/andryoga/safebox/data/repository/BackupMetadataRepositoryImpl.kt`:
- Around line 34-47: Ensure BackupMetadataRepositoryImpl only calls
insertBackupMetadata after takePersistableUriPermission succeeds, skipping
persistence when the grant fails while preserving failure logging. Apply the
corresponding selection-flow adjustment in
app/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.kt:189-195 so
test-selected URIs follow the same successful-grant requirement;
app/src/main/java/com/andryoga/safebox/data/repository/BackupMetadataRepositoryImpl.kt:34-47
is the root-cause change.
In `@app/src/main/java/com/andryoga/safebox/worker/RestoreDataWorker.kt`:
- Around line 83-89: Replace the ObjectInputStream/readObject flow in
RestoreDataWorker with a schema-bound backup format such as JSON, CBOR, or
protobuf, validating the expected map structure and enforcing size and nesting
bounds before constructing importMap. If compatibility requires Java
serialization, configure a strict ObjectInputFilter before deserialization to
allow only the expected types and reject excessive depth, references, array
sizes, and total bytes.
---
Outside diff comments:
In
`@app/src/androidTest/java/com/andryoga/safebox/e2e/RecordsScreenScrollAndAppBarTest.kt`:
- Around line 147-154: In the test flow around performScrollToIndex(40), repeat
the add-new-record app bar visibility assertion after scrolling. Reuse
addNewButtonDesc and the existing onNodeWithContentDescription assertion so the
test verifies the app bar remains displayed after the LazyColumn scroll.
🪄 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 Plus
Run ID: 82bdcc4e-fd2b-485e-bf12-d93dcdb3e65b
📒 Files selected for processing (46)
app/build.gradleapp/schemas/com.andryoga.safebox.data.db.SafeBoxDatabase/1.jsonapp/src/androidTest/java/com/andryoga/safebox/BaseTestApplication.ktapp/src/androidTest/java/com/andryoga/safebox/common/CoroutineTestHelpers.ktapp/src/androidTest/java/com/andryoga/safebox/common/TestDispatchersProvider.ktapp/src/androidTest/java/com/andryoga/safebox/data/db/MigrationTest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/AddNewRecordFlowsE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/CrossFeatureUserJourneysE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/HomeScreenNavigationAndTimeoutE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/LoginBiometricAndHintE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsComprehensiveE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/RecordsScreenScrollAndAppBarTest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/RecordsSearchAndFilterE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/SignupAndLoginE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/home/backupAndRestore/BackupAndRestoreScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/home/backupAndRestore/BackupAndRestoreWorkersTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/home/records/RecordsScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/home/settings/SettingsScreenComprehensiveTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/loading/LoadingScreenRootTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/login/LoginScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/signup/SignupPasswordValidationRulesTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/signup/SignupScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/signup/StatefulSignupTestHelper.ktapp/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordActionsComprehensiveTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenTest.ktapp/src/main/java/com/andryoga/safebox/data/repository/BackupMetadataRepositoryImpl.ktapp/src/main/java/com/andryoga/safebox/di/CoroutineModule.ktapp/src/main/java/com/andryoga/safebox/providers/EncryptedPreferenceProviderImpl.ktapp/src/main/java/com/andryoga/safebox/providers/PreferenceProviderImpl.ktapp/src/main/java/com/andryoga/safebox/ui/MainViewModel.ktapp/src/main/java/com/andryoga/safebox/ui/core/BiometricAuth.ktapp/src/main/java/com/andryoga/safebox/ui/home/backupAndRestore/components/newBackupOrRestore/NewBackupOrRestoreScreen.ktapp/src/main/java/com/andryoga/safebox/ui/loading/LoadingScreenRoot.ktapp/src/main/java/com/andryoga/safebox/ui/login/LoginScreen.ktapp/src/main/java/com/andryoga/safebox/ui/login/LoginScreenAction.ktapp/src/main/java/com/andryoga/safebox/ui/login/LoginViewModel.ktapp/src/main/java/com/andryoga/safebox/ui/navigation/AppNavigation.ktapp/src/main/java/com/andryoga/safebox/ui/signup/SignupScreen.ktapp/src/main/java/com/andryoga/safebox/ui/signup/SignupViewModel.ktapp/src/main/java/com/andryoga/safebox/worker/BackupDataWorker.ktapp/src/main/java/com/andryoga/safebox/worker/RestoreDataWorker.ktapp/src/test/java/com/andryoga/safebox/ui/MainViewModelTest.ktapp/src/test/java/com/andryoga/safebox/ui/signup/SignupViewModelTest.ktgradle/libs.versions.toml
💤 Files with no reviewable changes (1)
- app/src/main/java/com/andryoga/safebox/ui/signup/SignupViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (6)
- app/build.gradle
- app/src/androidTest/java/com/andryoga/safebox/e2e/AddNewRecordFlowsE2ETest.kt
- app/src/androidTest/java/com/andryoga/safebox/e2e/SignupAndLoginE2ETest.kt
- gradle/libs.versions.toml
- app/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsE2ETest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenTest.kt
Comment on lines
+188
to
+222
| @Test | ||
| fun rapidMultiClickOnLoginButton_shouldPreventDuplicateSubmission() { | ||
| var loginClickCount = 0 | ||
| val targetPassword = "MySecretPassword123" | ||
|
|
||
| composeTestRule.setContent { | ||
| SafeBoxTheme { | ||
| LoginScreen( | ||
| uiState = LoginUiState(), | ||
| screenAction = { action -> | ||
| if (action is LoginScreenAction.LoginClicked) { | ||
| loginClickCount++ | ||
| } | ||
| } | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| composeTestRule.onNode( | ||
| hasSetTextAction() and hasText( | ||
| context.getString(R.string.password), | ||
| substring = true | ||
| ) | ||
| ) | ||
| .performTextInput(targetPassword) | ||
| composeTestRule.onNodeWithText(context.getString(R.string.login)).assertIsEnabled() | ||
|
|
||
| // Perform rapid multi-clicks | ||
| composeTestRule.onNodeWithText(context.getString(R.string.login)).performClick() | ||
| composeTestRule.onNodeWithText(context.getString(R.string.login)).performClick() | ||
| composeTestRule.onNodeWithText(context.getString(R.string.login)).performClick() | ||
|
|
||
| // Verify that multiple click actions are emitted cleanly (or handled by state/ViewModel) without race condition crash | ||
| assertThat(loginClickCount).isGreaterThan(0) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Actually verify duplicate submissions are prevented.
isGreaterThan(0) passes when all three clicks emit actions. Assert exactly one emission if the screen owns suppression; otherwise move this coverage to the ViewModel/integration layer and rename this component test.
-assertThat(loginClickCount).isGreaterThan(0)
+assertThat(loginClickCount).isEqualTo(1)📝 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.
Suggested change
| @Test | |
| fun rapidMultiClickOnLoginButton_shouldPreventDuplicateSubmission() { | |
| var loginClickCount = 0 | |
| val targetPassword = "MySecretPassword123" | |
| composeTestRule.setContent { | |
| SafeBoxTheme { | |
| LoginScreen( | |
| uiState = LoginUiState(), | |
| screenAction = { action -> | |
| if (action is LoginScreenAction.LoginClicked) { | |
| loginClickCount++ | |
| } | |
| } | |
| ) | |
| } | |
| } | |
| composeTestRule.onNode( | |
| hasSetTextAction() and hasText( | |
| context.getString(R.string.password), | |
| substring = true | |
| ) | |
| ) | |
| .performTextInput(targetPassword) | |
| composeTestRule.onNodeWithText(context.getString(R.string.login)).assertIsEnabled() | |
| // Perform rapid multi-clicks | |
| composeTestRule.onNodeWithText(context.getString(R.string.login)).performClick() | |
| composeTestRule.onNodeWithText(context.getString(R.string.login)).performClick() | |
| composeTestRule.onNodeWithText(context.getString(R.string.login)).performClick() | |
| // Verify that multiple click actions are emitted cleanly (or handled by state/ViewModel) without race condition crash | |
| assertThat(loginClickCount).isGreaterThan(0) | |
| } | |
| `@Test` | |
| fun rapidMultiClickOnLoginButton_shouldPreventDuplicateSubmission() { | |
| var loginClickCount = 0 | |
| val targetPassword = "MySecretPassword123" | |
| composeTestRule.setContent { | |
| SafeBoxTheme { | |
| LoginScreen( | |
| uiState = LoginUiState(), | |
| screenAction = { action -> | |
| if (action is LoginScreenAction.LoginClicked) { | |
| loginClickCount++ | |
| } | |
| } | |
| ) | |
| } | |
| } | |
| composeTestRule.onNode( | |
| hasSetTextAction() and hasText( | |
| context.getString(R.string.password), | |
| substring = true | |
| ) | |
| ) | |
| .performTextInput(targetPassword) | |
| composeTestRule.onNodeWithText(context.getString(R.string.login)).assertIsEnabled() | |
| // Perform rapid multi-clicks | |
| composeTestRule.onNodeWithText(context.getString(R.string.login)).performClick() | |
| composeTestRule.onNodeWithText(context.getString(R.string.login)).performClick() | |
| composeTestRule.onNodeWithText(context.getString(R.string.login)).performClick() | |
| // Verify that multiple click actions are emitted cleanly (or handled by state/ViewModel) without race condition crash | |
| assertThat(loginClickCount).isEqualTo(1) | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 190-190: A credential is hard-coded by assigning a string literal to a password/secret/API-key variable. Secrets stored in source code can be leaked and abused by internal or external malicious actors. Remove the literal, rotate the exposed secret, and load it at runtime from an environment variable, a secure secret vault, or a Hardware Security Module (HSM).
Context: val targetPassword = "MySecretPassword123"
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures
(hardcoded-password-string-literal-kotlin)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/androidTest/java/com/andryoga/safebox/ui/login/LoginScreenTest.kt`
around lines 188 - 222, Update
rapidMultiClickOnLoginButton_shouldPreventDuplicateSubmission to assert exactly
one LoginClicked emission when LoginScreen owns duplicate suppression; replace
the greater-than-zero assertion with an exact count and ensure the test
exercises the screen’s suppression behavior. If suppression belongs to the
ViewModel instead, move this coverage there and rename the component test to
reflect that it only verifies click emission.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.kt (1)
142-183: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a fixed timestamp for the seeded fixtures.
Lines 142-183 use
Date()despite the helper’s deterministic-fixture contract. Date rendering or date-based ordering assertions can vary between runs.Proposed fix
suspend fun setupSeededVaultRecords( safeBoxDatabase: SafeBoxDatabase, loginDataRepository: LoginDataRepository, bankCardDataRepository: BankCardDataRepository, bankAccountDataRepository: BankAccountDataRepository, secureNoteDataRepository: SecureNoteDataRepository ) { + val fixtureTimestamp = 1_700_000_000_000L safeBoxDatabase.loginDataDao().deleteAllData() ... - creationDate = Date(), - updateDate = Date() + creationDate = Date(fixtureTimestamp), + updateDate = Date(fixtureTimestamp)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.kt` around lines 142 - 183, Replace every Date() used for creationDate and updateDate in the seeded fixtures within E2ETestUtils with the helper’s fixed deterministic timestamp, reusing one shared timestamp value for all fixtures so date rendering and ordering remain stable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/nightly.yml:
- Around line 16-17: Update the nightly workflow permissions to use contents:
read instead of contents: write, and configure the actions/checkout step with
persist-credentials: false. Preserve the existing workflow behavior while
applying both least-privilege and credential-persistence settings.
In
`@app/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsComprehensiveE2ETest.kt`:
- Around line 308-310: Replace the ineffective targetTitle waits in both edit
flows with a save-completion condition that confirms the record has returned to
view mode, such as the edit action becoming visible or the save action
disappearing. Apply this to the login flow at
app/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsComprehensiveE2ETest.kt#L308-L310
and the bank-card flow at
app/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsComprehensiveE2ETest.kt#L377-L379
before reopening each record.
---
Outside diff comments:
In `@app/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.kt`:
- Around line 142-183: Replace every Date() used for creationDate and updateDate
in the seeded fixtures within E2ETestUtils with the helper’s fixed deterministic
timestamp, reusing one shared timestamp value for all fixtures so date rendering
and ordering remain stable.
🪄 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 Plus
Run ID: bafa80bd-8a1e-4e26-aa92-58fa937ab548
📒 Files selected for processing (26)
.agents/AGENTS.md.github/workflows/nightly.ymlapp/build.gradleapp/src/androidTest/java/com/andryoga/safebox/e2e/AddNewRecordFlowsE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/CrossFeatureUserJourneysE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/HomeScreenNavigationAndTimeoutE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/LoginBiometricAndHintE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsComprehensiveE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/RecordsScreenScrollAndAppBarTest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/RecordsSearchAndFilterE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/SignupAndLoginE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/home/backupAndRestore/BackupAndRestoreScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/home/records/RecordsScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/home/records/RecordsSearchBarTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/home/settings/SettingsScreenComprehensiveTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/loading/LoadingScreenRootTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/login/LoginScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/signup/SignupPasswordValidationRulesTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/signup/SignupScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordActionsComprehensiveTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenScrollTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordTopBarTest.ktgradle.properties
🚧 Files skipped from review as they are similar to previous changes (21)
- .agents/AGENTS.md
- app/src/androidTest/java/com/andryoga/safebox/ui/loading/LoadingScreenRootTest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/home/records/RecordsSearchBarTest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/signup/SignupScreenTest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/home/records/RecordsScreenTest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/signup/SignupPasswordValidationRulesTest.kt
- app/src/androidTest/java/com/andryoga/safebox/e2e/LoginBiometricAndHintE2ETest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/home/backupAndRestore/BackupAndRestoreScreenTest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/home/settings/SettingsScreenComprehensiveTest.kt
- app/src/androidTest/java/com/andryoga/safebox/e2e/RecordsSearchAndFilterE2ETest.kt
- app/src/androidTest/java/com/andryoga/safebox/e2e/HomeScreenNavigationAndTimeoutE2ETest.kt
- app/src/androidTest/java/com/andryoga/safebox/e2e/SignupAndLoginE2ETest.kt
- app/src/androidTest/java/com/andryoga/safebox/e2e/CrossFeatureUserJourneysE2ETest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordActionsComprehensiveTest.kt
- app/src/androidTest/java/com/andryoga/safebox/e2e/AddNewRecordFlowsE2ETest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenScrollTest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordTopBarTest.kt
- app/src/androidTest/java/com/andryoga/safebox/e2e/RecordsScreenScrollAndAppBarTest.kt
- app/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsE2ETest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/singleRecord/SingleRecordScreenTest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/login/LoginScreenTest.kt
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/src/main/java/com/andryoga/safebox/ui/core/BiometricAuth.kt (1)
42-42: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdding
onSuccess/onErrorOrCancelasrememberkeys can re-trigger the biometric prompt on recomposition.Inline lambdas from
LoginScreen(e.g.{ screenAction(LoginScreenAction.BiometricSuccess) }) create new instances each recomposition. Since these are nowrememberkeys, theBiometricPromptis recreated andLaunchedEffect(biometricPrompt)re-fires — callingauthenticate()again — wheneverLoginScreenrecomposes for any reason while the prompt is showing.Use
rememberUpdatedStatefor the callbacks and keyrememberonly onactivityso the prompt is created once but always invokes the latest callbacks:🔒 Proposed fix using rememberUpdatedState
`@Composable` fun BiometricAuthHandler( onSuccess: () -> Unit, onErrorOrCancel: () -> Unit = {}, ) { val override = biometricAuthHandlerOverride if (override != null) { override(onSuccess, onErrorOrCancel) return } val context = LocalContext.current val activity = remember(context) { context.findActivity() as? FragmentActivity } ?: return val executor = remember(context) { ContextCompat.getMainExecutor(context) } + val currentOnSuccess by rememberUpdatedState(onSuccess) + val currentOnErrorOrCancel by rememberUpdatedState(onErrorOrCancel) + - val biometricPrompt: BiometricPrompt = remember(activity, onSuccess, onErrorOrCancel) { + val biometricPrompt: BiometricPrompt = remember(activity) { BiometricPrompt( activity, executor, object : BiometricPrompt.AuthenticationCallback() { override fun onAuthenticationSucceeded(result: BiometricPrompt.AuthenticationResult) { - onSuccess() + currentOnSuccess() } override fun onAuthenticationError(errorCode: Int, errString: CharSequence) { - onErrorOrCancel() + currentOnErrorOrCancel() } } ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/andryoga/safebox/ui/core/BiometricAuth.kt` at line 42, Update the biometric prompt setup around biometricPrompt to store onSuccess and onErrorOrCancel with rememberUpdatedState, then key remember only on activity. Ensure the prompt’s callbacks invoke those latest state-backed values, keeping biometricPrompt stable across recompositions so LaunchedEffect does not re-trigger authentication.app/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.kt (3)
197-201: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a deterministic default backup timestamp.
TEST_DATEstabilizes seeded records, but this helper still defaultsmockTimestamptoSystem.currentTimeMillis(). Assertions on the displayed backup time will vary by run; default toTEST_DATE.timeor require an explicit timestamp.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.kt` around lines 197 - 201, Update the helper’s mockTimestamp default in the backup metadata setup to use the deterministic TEST_DATE.time value instead of System.currentTimeMillis(), while preserving explicit timestamp overrides.
223-236: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHide the IME explicitly instead of toggling it.
toggleSoftInput(HIDE_IMPLICIT_ONLY, 0)is deprecated and stateful on newer Android versions, so it can no-op or reopen the keyboard during focus changes. Pass a focusedView/window token and callhideSoftInputFromWindow(...)instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.kt` around lines 223 - 236, Update closeSoftKeyboard to hide the IME explicitly rather than calling the deprecated toggleSoftInput method. Obtain a focused View from the current activity/window, pass its window token to InputMethodManager.hideSoftInputFromWindow, and preserve the existing main-thread execution and waitForIdle behavior.
427-432: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winWait for the node to be displayed, not just present.
fetchSemanticsNodes().isNotEmpty()returns as soon as a matching node exists in the tree, even if it isn’t visible yet. That can let later clicks/assertions race the UI; useassertIsDisplayed()or a displayed matcher in the polling block.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.kt` around lines 427 - 432, Update E2ETestUtils.waitUntilNodeDisplayed to poll for display visibility rather than mere semantics-node presence. Replace the current waitUntilNodeDisplayed delegation or condition with logic that uses assertIsDisplayed() or an equivalent displayed matcher within the polling block, while preserving the existing matcher and timeout behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/com/andryoga/safebox/data/repository/BackupMetadataRepositoryImpl.kt`:
- Around line 40-42: Update the failure logging in the persistable URI
permission flow to avoid including the complete user URI. Replace $uriPath in
the Timber.w call with a redacted URI, authority, or stable hash while retaining
the existing failure context.
- Around line 34-45: Update the backup-selection analytics flow to record the
final outcome after the persistable URI permission check, using the same success
condition that controls the DAO insert. Ensure failed permission grants record
RESULT=false, while successful selections retain RESULT=true, and avoid logging
the event before permission validation.
- Around line 40-55: Update the backup setup flow around
takePersistableUriPermission and
BackupMetadataRepositoryImpl.insertBackupMetadata so permission failures return
an explicit failure result instead of being swallowed. Propagate that result
through BackupAndRestoreVM, add or update its error state, and have the
backup-setup UI display the failure while preserving the existing success path.
---
Outside diff comments:
In `@app/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.kt`:
- Around line 197-201: Update the helper’s mockTimestamp default in the backup
metadata setup to use the deterministic TEST_DATE.time value instead of
System.currentTimeMillis(), while preserving explicit timestamp overrides.
- Around line 223-236: Update closeSoftKeyboard to hide the IME explicitly
rather than calling the deprecated toggleSoftInput method. Obtain a focused View
from the current activity/window, pass its window token to
InputMethodManager.hideSoftInputFromWindow, and preserve the existing
main-thread execution and waitForIdle behavior.
- Around line 427-432: Update E2ETestUtils.waitUntilNodeDisplayed to poll for
display visibility rather than mere semantics-node presence. Replace the current
waitUntilNodeDisplayed delegation or condition with logic that uses
assertIsDisplayed() or an equivalent displayed matcher within the polling block,
while preserving the existing matcher and timeout behavior.
In `@app/src/main/java/com/andryoga/safebox/ui/core/BiometricAuth.kt`:
- Line 42: Update the biometric prompt setup around biometricPrompt to store
onSuccess and onErrorOrCancel with rememberUpdatedState, then key remember only
on activity. Ensure the prompt’s callbacks invoke those latest state-backed
values, keeping biometricPrompt stable across recompositions so LaunchedEffect
does not re-trigger authentication.
🪄 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 Plus
Run ID: 14f55117-a04b-4214-ac78-9645cc2933aa
📒 Files selected for processing (17)
.github/workflows/nightly.ymlapp/schemas/com.andryoga.safebox.data.db.SafeBoxDatabase/1.jsonapp/src/androidTest/java/com/andryoga/safebox/data/db/MigrationTest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/CrossFeatureUserJourneysE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/E2ETestUtils.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/LoginBiometricAndHintE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsComprehensiveE2ETest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/home/backupAndRestore/BackupAndRestoreScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/home/backupAndRestore/BackupAndRestoreWorkersTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/home/settings/SettingsScreenComprehensiveTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/loading/LoadingScreenRootTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/login/LoginScreenTest.ktapp/src/androidTest/java/com/andryoga/safebox/ui/signup/StatefulSignupTestHelper.ktapp/src/main/java/com/andryoga/safebox/data/repository/BackupMetadataRepositoryImpl.ktapp/src/main/java/com/andryoga/safebox/ui/core/BiometricAuth.ktapp/src/main/java/com/andryoga/safebox/ui/signup/SignupViewModel.ktapp/src/main/java/com/andryoga/safebox/worker/RestoreDataWorker.kt
🚧 Files skipped from review as they are similar to previous changes (10)
- .github/workflows/nightly.yml
- app/src/androidTest/java/com/andryoga/safebox/ui/home/settings/SettingsScreenComprehensiveTest.kt
- app/src/androidTest/java/com/andryoga/safebox/e2e/RecordActionsComprehensiveE2ETest.kt
- app/schemas/com.andryoga.safebox.data.db.SafeBoxDatabase/1.json
- app/src/main/java/com/andryoga/safebox/worker/RestoreDataWorker.kt
- app/src/androidTest/java/com/andryoga/safebox/e2e/CrossFeatureUserJourneysE2ETest.kt
- app/src/androidTest/java/com/andryoga/safebox/e2e/LoginBiometricAndHintE2ETest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/login/LoginScreenTest.kt
- app/src/androidTest/java/com/andryoga/safebox/ui/home/backupAndRestore/BackupAndRestoreScreenTest.kt
- app/src/androidTest/java/com/andryoga/safebox/data/db/MigrationTest.kt
Comment on lines
+34
to
+45
| var permissionGranted = true | ||
| if (uriPath.scheme == "content" || uriPath.toString().startsWith("content://")) { | ||
| runCatching { | ||
| val flags: Int = Intent.FLAG_GRANT_READ_URI_PERMISSION or | ||
| Intent.FLAG_GRANT_WRITE_URI_PERMISSION | ||
| contentResolver.takePersistableUriPermission(uriPath, flags) | ||
| }.onFailure { | ||
| permissionGranted = false | ||
| Timber.w(it, "Failed to take persistable URI permission for $uriPath") | ||
| } | ||
| } | ||
| if (permissionGranted) { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Record the actual backup-selection outcome in analytics.
The event is logged before this permission check, and its result only reflects whether the URI has a path. A failed grant now skips the DAO insert but still records RESULT=true, producing false success telemetry.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/com/andryoga/safebox/data/repository/BackupMetadataRepositoryImpl.kt`
around lines 34 - 45, Update the backup-selection analytics flow to record the
final outcome after the persistable URI permission check, using the same success
condition that controls the DAO insert. Ensure failed permission grants record
RESULT=false, while successful selections retain RESULT=true, and avoid logging
the event before permission validation.
Comment on lines
+40
to
+42
| }.onFailure { | ||
| permissionGranted = false | ||
| Timber.w(it, "Failed to take persistable URI permission for $uriPath") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Avoid logging the complete user URI.
A content:// URI can contain provider, account, or document identifiers. Log only a redacted URI/authority or a stable hash instead of the full user-selected path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/com/andryoga/safebox/data/repository/BackupMetadataRepositoryImpl.kt`
around lines 40 - 42, Update the failure logging in the persistable URI
permission flow to avoid including the complete user URI. Replace $uriPath in
the Timber.w call with a redacted URI, authority, or stable hash while retaining
the existing failure context.
Comment on lines
+40
to
+55
| }.onFailure { | ||
| permissionGranted = false | ||
| Timber.w(it, "Failed to take persistable URI permission for $uriPath") | ||
| } | ||
| } | ||
| if (permissionGranted) { | ||
| backupMetadataDao.insertBackupMetadata( | ||
| BackupMetadataEntity( | ||
| key = 1, | ||
| uriString = uriPath.toString(), | ||
| displayPath = uriPath.path ?: uriPath.toString(), | ||
| lastBackupDate = null, | ||
| createdOn = Date() | ||
| ) | ||
| ) | ||
| ) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
rg -n "takePersistableUriPermission|insertBackupMetadata|BackupAndRestoreVM|BackupMetadataRepositoryImpl" app/src/main/javaRepository: Ni3verma/Safe-Box
Length of output: 2071
🏁 Script executed:
sed -n '1,220p' app/src/main/java/com/andryoga/safebox/ui/home/backupAndRestore/BackupAndRestoreVM.kt && printf '\n---SCREEN---\n' && sed -n '1,260p' app/src/main/java/com/andryoga/safebox/ui/home/backupAndRestore/BackupAndRestoreScreen.kt && printf '\n---REPO---\n' && sed -n '1,120p' app/src/main/java/com/andryoga/safebox/data/repository/interfaces/BackupMetadataRepository.kt && printf '\n---IMPL---\n' && sed -n '1,180p' app/src/main/java/com/andryoga/safebox/data/repository/BackupMetadataRepositoryImpl.ktRepository: Ni3verma/Safe-Box
Length of output: 14094
🏁 Script executed:
rg -n "data class ScreenState|sealed class ScreenState|error|Snackbar|Toast|event|uiState" app/src/main/java/com/andryoga/safebox/ui/home/backupAndRestore app/src/main/java/com/andryoga/safebox/ui/core app/src/main/java/com/andryoga/safebox/uiRepository: Ni3verma/Safe-Box
Length of output: 34876
🏁 Script executed:
sed -n '1,160p' app/src/main/java/com/andryoga/safebox/ui/home/backupAndRestore/ScreenState.ktRepository: Ni3verma/Safe-Box
Length of output: 1365
Surface backup-setup permission failures. insertBackupMetadata() drops takePersistableUriPermission errors, and BackupAndRestoreVM has no error state, so backup setup can fail silently. Return a failure/result and surface it in the UI.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/com/andryoga/safebox/data/repository/BackupMetadataRepositoryImpl.kt`
around lines 40 - 55, Update the backup setup flow around
takePersistableUriPermission and
BackupMetadataRepositoryImpl.insertBackupMetadata so permission failures return
an explicit failure result instead of being swallowed. Propagate that result
through BackupAndRestoreVM, add or update its error state, and have the
backup-setup UI display the failure while preserving the existing success path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Summary by CodeRabbit