feat: show a shareable link on takeable orders - #669
Conversation
The app could already open order links but never produced one, so sharing an order meant reading its id aloud and having the other side look it up by hand. The `mostro:` scheme is registered in the manifest, NostrUtils parses `mostro:<order-id>?relays=...&mostro=<pubkey>`, and DeepLinkService resolves the order from those relays, switching Mostro instance when the link points at a different one. Only the encoder was missing. NostrUtils.buildMostroUrl now writes that same format, next to the two functions that read it. It keeps only relays carrying a WebSocket scheme, drops duplicates, caps the list so the link stays pasteable, and emits the instance pubkey only when it passes the check the parser applies, so a link this builds is always one the parser accepts. OrderShareLinkCard shows the link with a copy button, following the shape of the OrderIdCard beside it, and renders nothing when no relay can resolve the order. It appears on the order book's detail screen and, in My Trades, only while the order is still pending: a link to a trade already under way is of no use to whoever opens it. Adds orderLinkLabel and orderLinkCopiedMessage to the six locales.
|
Filed the pre-existing deep-link crash mentioned above as #670, with the reproduction and the check against |
|
Warning Review limit reached
Next review available in: 16 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![ermeme[bot]](https://avatars.githubusercontent.com/in/3690068?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: Request changes
The link builder/card implementation is small and mostly looks sound: the generated URLs round-trip through the existing parser, the new ARB keys are present in the touched locales, and the widget copy path is covered.
I do not think this should merge as-is while it closes #451, though. The feature creates mostro: links specifically so another user can open the order directly, but the PR body documents that opening one while the app is not foregrounded currently crashes/does not navigate (#670). That is the common receive path for links shared through Telegram/Signal/etc. If this lands before the receiver-side fix, the app starts handing users links that are known to fail in the scenario this feature is meant to support.
Please make this PR explicitly depend on a merged fix for #670 (or include the receiver-side fix here) before closing #451. Otherwise the UI copy action works, but the shared link is not reliably usable by the recipient.
Verification performed:
- Reviewed current head
b0b1f1ab4e0514d530d969678cfd127ac8790042against basec3c2d7a7b318e70b2d7555f43d49ef1bfc009624. - Read the PR body, linked issue #451, PR comments, and current CI state.
- Parsed the touched ARB locale files with duplicate-key detection; the new keys are present. The only parity drift found is pre-existing
myActiveTradesextra inintl_es.arb. - Ran
git diff --checkon the changed files successfully. - Could not run
flutter testlocally because this environment does not haveflutteronPATH; GitHub'sbuildcheck is currently green for this head.
|
Agreed — handing out links that the recipient cannot open is the wrong order to ship this in. The PR body now says so explicitly: this depends on #673, which fixes the receiving side (#670). Merge order is #673 → this one. No code change here; the dependency was already argued in the body, it just was not stated as a blocking one. |
|
Hi @21Mill this kind of UI new features needs to be discussed before implementing them, in fact we are not working on new UI features in this app, new features are going to the new app https://github.com/MostroP2P/app please redirect this efforts to create new issues and then PRs on https://github.com/MostroP2P/app and your contributions can help us to have the new app faster |
2 participants
Depends on #673
#673 fixes the receiving end (#670): until it lands, a recipient opening one of these links with the app closed does not reliably reach the order. Merge order: #673 → this.
Closes #451
Problem
There is no way to share a specific order from the app. Someone who creates an order and waits for a taker can only read its id aloud, leaving the other side to look it up by hand.
What makes this odd is that half the work was already there, pointing the other way: the app knows how to open order links but never produces one.
mostro:scheme is registered inandroid/app/src/main/AndroidManifest.xml.NostrUtils.isValidMostroUrl/parseMostroUrldefine the format:mostro:<order-id>?relays=wss://r1,wss://r2&mostro=<pubkey-hex>, whererelaysis mandatory andmostrois optional but must be 64-char hex.DeepLinkService.processMostroLinkresolves the order from those relays, andDeepLinkHandlereven switches Mostro instance when the link points at a different one.So no format had to be invented and nothing had to be agreed with other clients — only the encoder was missing.
Change
NostrUtils.buildMostroUrl, placed right next to the two functions that read the same format, so all three stay together:It keeps only relays carrying a WebSocket scheme (same check
isValidMostroUrlapplies), drops duplicates, caps the list so the link stays pasteable, and emits the instance pubkey only when it passes the exact validationparseMostroUrluses. It returnsnullwhen no usable relay is left, since the format cannot resolve an order without one. The invariant: anything this builds is something the project's own parser accepts and reads back unchanged.OrderShareLinkCardshows the link with a copy button, following the shape of theOrderIdCardbeside it, and renders nothing when the link isnull. It reads the active relays and instance pubkey fromsettingsProvider.The card on a sell order's detail screen, below the order id:
Copying puts the whole link on the clipboard — three relays and the instance pubkey — even though the card itself elides it:
It appears on the order book's detail screen, and in My Trades only while the order is still
pending: a link to a trade already under way is of no use to whoever opens it.Adds
orderLinkLabelandorderLinkCopiedMessageto the six locales. NoOrderIdCardrefactor and no new automation id, so the automation contract is untouched.Copying is the whole of what #451 asked for, so that is all this does. Offering the system share sheet as well is tracked separately in #671.
Tests
test/shared/utils/deep_link_parsing_test.dart— newbuildMostroUrlgroup: round-trip throughisValidMostroUrl+parseMostroUrl; non-WebSocket relays dropped; duplicates removed; relay count capped;nullwith no usable relay and with no order id; pubkey accepted upper-case or0x-prefixed, and omitted when malformed rather than emitted for the parser to discard.test/shared/widgets/order_cards_test.dart— the card renders the full link, the copy button puts exactly that string on the clipboard (asserted throughSystemChannels.platform), and nothing renders when no relay is configured.Test plan
flutter gen-l10n— no untranslated messagesflutter analyzeonlib/sharedandtest/shared— no issuesflutter test test/shared/— 203 passingflutter test— 899 passing, 11 pre-existing failures (staletest/mocks.mocks.dart, repo issue Dart run build_runner build fails on Flutter 3.44.0, source_gen 3.1.0 incompatible with analyzer 8.x #606; same list before and after this branch)&mostro=…VIEWintent opens that exact order, so the encoder is validated against the real parser and not only in testsUnrelated pre-existing bug found while testing
Opening a
mostro:link while the app is not in the foreground crashes it:Reproduced by leaving an order detail screen with Back until the app drops out of the foreground, then opening the link. I built and installed
main(c3c2d7a7) without this branch and it fails identically, so it is not introduced here — being anassert, release builds would not show the red screen but the navigation is just as broken.It is worth flagging alongside this PR: until now almost nobody had a
mostro:link to open, and this feature hands one to every user. I will file it separately.