Skip to content

Funding fixes - #2270

Closed
i5hi wants to merge 5 commits into
developfrom
funding-fixes-6110
Closed

Funding fixes#2270
i5hi wants to merge 5 commits into
developfrom
funding-fixes-6110

Conversation

@i5hi

@i5hi i5hi commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Handles KYC related errors and unique response bodies for specific jurisdictions

@claude

claude Bot commented Jun 9, 2026

Copy link
Copy Markdown

Claude finished @i5hi's task in 3m 8s —— View job


Code Review: Funding fixes

  • Read PR diff and changed files
  • Analyze error-handling, gateway parsing, bloc, and presentation changes
  • Verify localization coverage (27/27 locales)
  • Post review

Overall this is a clean, well-structured improvement to error handling for the funding flow. The error-code → localized-message mapping is consistent, and the loading/error lifecycle in the bloc is handled correctly (try/on/catch/finally). A few observations below — mostly minor.

Findings

1. Raw backend message shown to the user (i18n) — minor
lib/features/fund_exchange/presentation/fund_exchange_presentation_error.dart:56

String displayMessage(AppLocalizations loc) => emptyList
    ? loc.fundExchangeErrorFetchingBankCodes
    : (message.isNotEmpty ? message : loc.fundExchangeErrorLoadingDetails);

When emptyList is false but message is non-empty, the raw API string (sourced from apiError['en'] / error['message'] in fetchInstitutions) is shown verbatim. That string is always English and may be technical, regardless of the user's locale. Everywhere else in this PR you map to localized strings, so this is the one spot that leaks an untranslated backend message. Consider using it only as a last-resort fallback, or mapping known codes like you do for FundingOptionUnavailable.

2. _onFundingDetailsErrorCleared now clears the institutions error even in the default case — confirm intent
lib/features/fund_exchange/presentation/bloc/fund_exchange_bloc.dart:252

emit(state.copyWith(
  getExchangeFundingDetailsException: null,
  listFundingInstitutionsException: null,   // now always cleared
  fundingInstitutions: event.resetInstitutions ? null : state.fundingInstitutions,
));

The COP screen caller at fund_exchange_cop_bank_transfer_input_screen.dart:101 dispatches fundingDetailsErrorCleared() with the default resetInstitutions: false. Previously that handler only cleared the details exception; now it also clears listFundingInstitutionsException. This is likely harmless (the list itself is preserved), but worth a sanity check that it doesn't silently dismiss a relevant bank-codes error on that screen.

3. Redundant empty-institutions check — minor
fund_exchange_bloc.dart:86 throws FetchInstitutionsFailed.emptyList() when result.institutions.isEmpty, but bullbitcoin_api_funding_gateway.dart:150-153 already throws FetchInstitutionsFailed.emptyList() when elements is null/empty. The bloc check is effectively dead for the API path. Not a bug — just defensive duplication; fine to keep, but flagging in case one of them was meant to handle a different layer.

Things I checked that look correct

  • numString coercion in bullbitcoin_api_funding_gateway.dart:93-96 is safe: every field in GetFundingDetailsResponseModel is String?, so this prevents a type 'int' is not a subtype of type 'String?' crash rather than introducing one. Good defensive fix. Same for element ?? {} guarding the previous non-null cast.
  • numTelefono normalization correctly runs after coercion, and ??= leaves an existing value untouched while tolerating an all-missing case (field is nullable).
  • 'Could not parse details' hardcoded string (:113) is log/internal only — it maps to LoadFundingDetailsFailed, which renders the localized fundExchangeErrorLoadingDetails, so it's never user-facing.
  • Error-code consistency: isOptionUnavailable (ERR_RCP_PO404, ERR_RCP_POSINPE404, ERR_ORD_PO404) is fully covered by the FundingOptionUnavailable.displayMessage switch (with ERR_RCP_PO404 as the default branch). No unhandled code.
  • Localization completeness: all 27 .arb files include the 4 new keys; the +5/-1 diffs are just trailing-comma fixes on the previously-last key. No broken entries.
  • backup_card.dart: wrapping the text Column in Expanded + switching Spacer to a Gap correctly fixes overflow when titles wrap; crossAxisAlignment: .start on the Row aligns the icon to the top. Reasonable.

@ethicnology

Copy link
Copy Markdown
Member

closed in favor of #2272

@ethicnology ethicnology closed this Jun 9, 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