Skip to content

[Reimbursements] Fix the "Open on Wise" recipient link - #15620

Open
kashsuks wants to merge 9 commits into
hackclub:mainfrom
kashsuks:fix-wise-recipient-link
Open

kashsuks wants to merge 9 commits into
hackclub:mainfrom
kashsuks:fix-wise-recipient-link

Conversation

@kashsuks

@kashsuks kashsuks commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Summary of the problem

The "Open on Wise" button in the Wise approval modal sent admins to a broken URL like https://wise.com/send#?contact=https://wise.com/recipients/<uuid>?new=undefined.

Admins paste the full Wise recipient URL into the "Wise recipient ID" field. WiseTransfer strips that down to the UUID, but the same value was saved as-is onto the payout method. The button then added that full URL onto a send#?contact= link.

Describe your changes

  • Payout methods now save only the recipient UUID, not the full pasted URL.
  • The button now links to https://wise.com/recipients/<uuid>.
  • Added a maintenance task to fix existing payout methods. Run it once after deploy.

@kashsuks
kashsuks requested a review from a team November 9, 2026 22:19
@kashsuks

kashsuks commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Need to backfill once this is merged

@manu-gurudath manu-gurudath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@Luke-Oldenburg

Copy link
Copy Markdown
Contributor

@claude review

@claude

claude Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Claude finished @Luke-Oldenburg's task in 23s —— View job


Review of #15620: Fix the "Open on Wise" recipient link

Verdict: looks good, with a few non-blocking notes. I read the changed files directly. git diff origin/main...HEAD failed here (no merge base in the shallow checkout), and I didn't run the specs.

What I checked

  • Normalization: moving normalize_wise_recipient_id into HasWiseRecipient means the WiseTransfer, LegalEntity::PayoutMethod::WiseTransfer and User::PayoutMethod::WiseTransfer models all get it. This fixes the root cause (the full URL being saved) rather than only the link.
  • Constants: WISE_RECIPIENT_ID_FORMAT and WISE_RECIPIENT_ID_REGEXP are defined at the bottom of the module. They are only read when a validation or callback runs, after the module has fully loaded, so the ordering is fine.
  • Query strings: the unanchored regex handles ?new=undefined and similar suffixes.
  • Link: wise_recipient_url is nil-safe, and CGI.escape is harmless on a UUID.
  • Leftovers: no references to the deleted strip_prefix_controller remain in app/.
  • Maintenance task: the SQL filter ~* '^[0-9a-f-]{36}$' matches Ruby's format check, so only non-conforming rows are processed. Running the task twice is safe.

Non-blocking notes

  1. Unparseable values: if a stored value is neither a UUID nor a recognizable Wise URL, the task leaves it unchanged and update_columns rewrites the same value. Those rows will show up in every future run. Logging or skipping them would make the run output clearer.
  2. Backfill scope: the task only covers LegalEntity::PayoutMethod::WiseTransfer. User::PayoutMethod::WiseTransfer also includes the concern. If it has its own wise_recipient_id column that could hold URLs, it needs the same backfill. If it's legacy and unused, ignore this.
  3. Host match: /wise\.com\/recipients\/.../ is case-sensitive, so a pasted Wise.com URL won't normalize. Adding /i would cover it, but it's unlikely to matter.
  4. Missing specs: the specs cover the WiseTransfer model and the task. A spec for wise_recipient_url, or for the "" → nil behavior from the payout_form hidden field, would be a useful addition.
  5. Deploy step: the task must be run manually after deploy, as the PR notes say. Until then, existing bad rows keep the broken link.

This branch has not been deployed

No deployments
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.

4 participants