Skip to content

fix(recipients): tolerate missing SINPE owner names - #2541

Open
ethicnology wants to merge 1 commit into
developfrom
fix/sinpe-owner-name
Open

fix(recipients): tolerate missing SINPE owner names#2541
ethicnology wants to merge 1 commit into
developfrom
fix/sinpe-owner-name

Conversation

@ethicnology

Copy link
Copy Markdown
Member

SINPE Movil/IBAN recipients were dropped when ownerName was absent, since a server-nullable-by-design field was required client-side. Made nullable with label/identifier fallback for display.

Closes #2529

@ethicnology ethicnology self-assigned this Jul 30, 2026
@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown

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


Review: tolerate missing SINPE owner names

  • Gather context (single-commit diff, changed files)
  • Read changed files and all ownerName usage sites across the codebase
  • Review for correctness bugs, security issues, regressions
  • Post findings and summary

Verdict: the change is correct and well-contained. I traced every ownerName usage in lib/ and every consumer of the affected classes; no remaining code assumes a non-null SINPE owner name. One related pre-existing issue and one nit below.

Note: I can't submit formal inline review comments (GitHub App limitation), so findings are listed here with file/line references.

Findings

1. Same root cause still unguarded in the SINPE check flow (pre-existing, not introduced by this PR)

lib/features/recipients/interface_adapters/gateways/bullbitcoin_api_recipients_gateway.dart:165

final ownerName = result['ownerName'] as String;

The PR description says the server fills ownerName from a Ridivi lookup that can fail or return empty. If checkSinpe ever returns a null/absent ownerName for the same reason, this unguarded cast throws a TypeError in the add-recipient flow instead of a clean error. Since this PR establishes "ownerName is server-nullable by design", this call site deserves the same tolerance (as String? plus explicit handling). Fix this →

2. Nit: presenter test in a DTO test file

test/features/recipients/application/dtos/recipient_details_dto_test.dart:41-57 — the displayName fallback test exercises RecipientViewModel (interface_adapters/presenters layer), not the DTO. Fine to keep, but a recipient_view_model_test.dart would be a more natural home if the suite grows.

What I verified

  • All mapping layers already nullableRecipientDetailsModel.ownerName (recipient_details_model.dart:45), RecipientDetailsDto.ownerName (recipient_details_dto.dart:34), and RecipientViewModel.ownerName (recipient_view_model.dart:18) were already String?, so the domain change propagates without further edits. No other code reads ownerName off the SINPE detail classes.
  • Fix mechanism confirmed — the recipients gateway wraps per-recipient parsing in try/catch and drops failures (bullbitcoin_api_recipients_gateway.dart:120-136), which is exactly how the old StateError silently dropped these recipients.
  • No weakened creation validation — the new-recipient forms still submit through RecipientFormDataModel variants with a required non-null ownerName, so client-side creation is unchanged; only parsing of server-provided recipients is relaxed.
  • Display fallbacks are safe — all displayName consumers (recipients_list_tile.dart:48, pay/withdraw confirmation screens) already handle null with ?? '-'. Falling back to IBAN/phone number exposes nothing new — the list tile already renders those identifiers on the line below.
  • Blank normalization_nullIfBlank keeps the previous trim behavior and maps whitespace-only server values to null, so the display fallback chain also covers the empty-string case.

I could not run the new test locally (Flutter is not installed on this runner); the Analyze and Test CI workflow is in progress and will cover it.


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.

SINPE Móvil recipients silently dropped when ownerName is absent

1 participant