Skip to content

Merge main hotfixes in develop - #2274

Merged
ethicnology merged 11 commits into
developfrom
merge-main-in-develop
Jun 9, 2026
Merged

Merge main hotfixes in develop#2274
ethicnology merged 11 commits into
developfrom
merge-main-in-develop

Conversation

@ethicnology

Copy link
Copy Markdown
Member

No description provided.

@ethicnology ethicnology self-assigned this Jun 9, 2026
@ethicnology
ethicnology merged commit cf05412 into develop Jun 9, 2026
2 checks passed
@claude

claude Bot commented Jun 9, 2026

Copy link
Copy Markdown

Claude finished @ethicnology's task in 2m 41s —— View job


Code Review — Merge main hotfixes in develop

I reviewed the code changes merged by this PR (diff of the merge-main-in-develop branch vs develop), focusing on the fund_exchange error-handling rework, backup_card, and the localization additions.

Note: this PR is already merged, so this is a retrospective review — no code changes were made.

Verdict: No blocking bugs or regressions found. The error-handling refactor is well-structured (API error code → localized title/message, safe casting, rethrow to avoid double-wrapping). A few minor observations below.


✅ Things that look correct

  • bullbitcoin_api_funding_gateway.dart:99result['element'] changed from a hard cast to (... as Map<String, dynamic>?) ?? {}, removing a potential TypeError crash on malformed responses. 👍
  • bullbitcoin_api_funding_gateway.dart:113 — the new on FetchFundingDetailsFailed { rethrow; } correctly prevents the inner "Missing funding details" error from being re-wrapped as the generic "Could not parse details". 👍
  • log.warning(...) calls match the logger signature (warning(Object? message, {error, trace}) in lib/core/utils/logger.dart:259) — positional message is correct.
  • fund_exchange_method_selection_screen.dart:135 — clearing both local error state and dispatching fundingDetailsErrorCleared(resetInstitutions: true) on jurisdiction change correctly drops stale institution lists.
  • Error type plumbing through _fundingDetailsError / _institutionsError matches the FundExchangePresentationError types in state.

🟡 Minor observations (non-blocking)

  1. Empty placeholder produces a dangling stringfund_exchange_api_error_copy.dart:34

    'ERR_ORD_KYC400' => loc.fundExchangeErrorOrdKyc400(messageData?['missingFields'] ?? ''),

    If messageData is missing the field, the message renders as "You do not have the KYC requirements for this action: " (trailing colon, nothing after). Same shape applies to ERR_ORD_CSRCP400 ({iban} → empty). Cosmetic, but worth guarding if the backend doesn't always populate messageData.

  2. Dead parameterfund_exchange_presentation_error.dart:92
    LoadInstitutionsFailed gained a message field/constructor param, but the only construction site (line 34) uses const LoadInstitutionsFailed() with no message. The message/displayMessage branch is currently unreachable. Either wire it up or drop it.

  3. Redundant empty-list checkFetchInstitutionsFailed.emptyList() is thrown both in the gateway (listInstitutions, when elements is null/empty) and again in the bloc (fund_exchange_bloc.dart:80, when result.institutions.isEmpty). Not a bug (both paths land on the same handled error), just duplicated logic.

  4. Localization formattinglocalization/app_fr.arb (and likely other locales) has inconsistent indentation on the new keys (e.g. a leading-space "fundExchangeErrorTitleOrdPo404" and a zero-indent "fundExchangeErrorOrdPo404"). Valid JSON, purely cosmetic. Worth confirming the non-en locales preserved the {missingFields} / {error} / {iban} placeholders so interpolation works in every language.

  5. Log severity downgrade (intentional, flagging for awareness) — funding-details/institution fetch failures moved from log.severelog.warning. This reduces Sentry noise for now-handled API errors, which is reasonable, but unexpected fetch failures will no longer surface as severe events.


Nice cleanup overall — moving from a single generic "could not load details" string to code-keyed, localized titles/messages is a solid UX improvement.

@claude claude Bot mentioned this pull request Jul 5, 2026
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