Skip to content

refactor(autoswap): sanitize user-facing error messages - #2544

Merged
ethicnology merged 4 commits into
developfrom
refactor-errors-autoswap
Aug 3, 2026
Merged

refactor(autoswap): sanitize user-facing error messages #2544
ethicnology merged 4 commits into
developfrom
refactor-errors-autoswap

Conversation

@wired-pasteque

Copy link
Copy Markdown
Collaborator

No description provided.

@wired-pasteque wired-pasteque self-assigned this Jul 30, 2026
@claude

This comment was marked as outdated.

@wired-pasteque

Copy link
Copy Markdown
Collaborator Author

@claude, check the fixes

@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown

Claude finished @wired-pasteque's task in 2m 19s —— View job


Fix verification — refactor(autoswap): sanitize user-facing error messages

  • Identify the fix commit(s) pushed since the last review
  • Verify finding 1: shared state.failure slot is now rendered
  • Verify finding 2: disabling auto swap no longer blocked by threshold validation
  • Verify finding 3: AutoswapUnexpectedFailure dead code resolved
  • Verify finding 4: usecase naming collision addressed
  • Check new/updated tests and CI

Verdict

The fix commit 224915d (fix(autoswap): surface settings failures and allow disabling) cleanly resolves findings 1–3. I traced each change end-to-end and found no new issues. The "Analyze and Test" CI run on this commit passed ✅.

Finding 1 — shared failure slot now rendered: ✅ Fixed.
Both surfaces now render state.failure through the existing AutoswapFailureL10n.toTranslated extension in an error-styled InfoCard — full screen at autoswap_settings_screen.dart:69-76 and bottom sheet at autoswap_settings.dart:111-121. Details I checked:

  • The card sits above the if (enabled) block, so failures from a disable-save (the trap from finding 2) are visible even when the fields are hidden.
  • Both cards live inside a full-state BlocBuilder, so they rebuild when the failure changes.
  • No stale-card risk: the cubit clears failure at the start of every save (autoswap_settings_cubit.dart:66), on load (:26), and on wallet selection (:206).
  • AutoswapSettingsSaveFailure now maps to the pre-existing autoswapUpdateSettingsError key (autoswap_failure_l10n.dart:12-13, present in app_en.arb:5520) instead of the generic string — a nice upgrade over what I suggested.

Finding 2 — disable no longer blocked by threshold validation: ✅ Fixed.
AutoSwap.violation now short-circuits with null when enabled == false (auto_swap.dart:62-63), exactly the fix proposed: an invalid-but-disabled row is inert, and re-enabling re-validates since the save then runs with enabled: true. So a legacy row below the 50 000-sat minimum can now be switched off, and the persisted state can't silently diverge from the toggle. If the write itself fails on disable, that failure now renders via the fix for finding 1. Covered by two new tests: auto_swap_test.dart ("accepts invalid thresholds while disabled") and save_autoswap_settings_usecase_test.dart ("writes disabled settings even when their thresholds are invalid", which also verifies the core save is actually invoked).

Finding 3 — dead AutoswapUnexpectedFailure: ✅ Fixed (removed).
The class and its l10n arm are gone. No gap results: the catch-alls in the load/save usecases already produce AutoswapSettingsUnavailableFailure / AutoswapSettingsSaveFailure (save_autoswap_settings_usecase.dart:33), and the l10n switch stays exhaustive over the sealed hierarchy. No remaining references anywhere in lib/ or test/.

Finding 4 — SaveAutoswapSettingsUsecase vs core SaveAutoSwapSettingsUsecase naming: not addressed (autoswap_locator.dart:24-27). This was a nit; fine to leave as-is if the team prefers the layer-consistent names.

New tests

test/features/autoswap/ui/autoswap_settings_failure_test.dart is a good addition — it pumps both the full screen and the bottom sheet with a failing load usecase and asserts the translated message ("Failed to load auto swap settings") appears, guarding the exact regression from finding 1 on both surfaces.

Nothing blocking from my side — LGTM.


@wired-pasteque
wired-pasteque marked this pull request as ready for review July 31, 2026 18:49
@ethicnology
ethicnology merged commit 0961f6f into develop Aug 3, 2026
2 checks passed
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.

2 participants