Repository navigation
chore(tipping): remove the unreachable tip amount flow - #1463
Merged
Merged
Conversation
github-actions
Bot
added
type: chore
TipUserModal was the only thing that pushed AppRoute.Sheets.TipAmountEntry, and it went with the scan-lands-in-chat change. Nothing opens the route now, so the route, its registration, the screen and its view model all go. The chat's own amount entry carried a comment pointing at this screen as the other sheet-presented amount entry; it loses the reference and keeps the reasoning.
confirmTip was the tip modal's submit button, and TipEvent.LaunchChat was the hand-off it emitted on success — open the tips flow, then the chat on top of it. Nothing calls either now: the chat charges the fee itself, and the scanned card goes straight to the chat with no tips list underneath. With LaunchChat gone the decorator's event collector handles one case, so the when collapses and navigateAll loses its last use there. TippingCoordinatorTest covered confirmTip's one invariant (a tip card always reports TipAction.TIP) through a reflection helper that existed only to feed it; both go.
TokenPurpose.Tip was constructed in one place — the tip modal's token picker — and nothing constructs it now, so the variant and its seven when branches go. Token select loses its one purpose-specific dismiss style with it: Tip was the only purpose forcing a close (X), so the screen now always takes the ambient one and the CompositionLocalProvider around it is redundant. title_amountToTip, label_of and content_description_customTipAmount were read only by the amount entry and the modal. action_swipeToTip still has a caller in ChatViewModel and stays for now.
bmc08gt
force-pushed
the
chore/remove-tip-amount-flow
branch
from
September 15, 2026 18:14
ef1d8e7 to
3d985b2
Compare
bmc08gt
added a commit
that referenced
this pull request
Sep 25, 2026
…#1554) * refactor(analytics): add the track interface beside the legacy service * refactor(analytics): convert Android domain types for the shared builders * refactor(analytics): move chat events onto the shared builders * refactor(analytics): move transfer events onto the shared builders Every transfer call site now tracks a TransferEvents builder. The amount conversion follows the type each caller already had (LocalFiat, or the scanned Fiat on a failed grab), so the property block is unchanged. Grab Time now goes out as a number instead of a string. The session delegate tests assert the event each grab, give, claim and cash-link share sends. TippingCoordinator lost its transfer call in #1463; only its unused Analytics import goes here. * refactor(analytics): move add money events onto the shared builders Every add-money call site now tracks an AddMoneyEvents builder instead of the legacy Analytics.AddMoney enums and FlipcashAnalyticsService methods. Fiat/LocalFiat amounts and thrown errors go through their .analytics converters, so each property block is unchanged except numbers going out as numbers instead of strings. Analytics.AddMoneySource, AddMoneyMethod, the AddMoneyEvent sealed interface, and the Mixpanel delegate overrides are dead now that every caller uses the shared builder, so they are removed. * refactor(analytics): move onramp events onto the shared builders The five contact-verification screens now track OnrampEvents.step(...) from the same LaunchedEffect. The events carry no properties, and a new test pins each step to the name the screens sent before. openOnramp, onrampPurchase, OnrampSource, OnrampPurchaseStep and their eight events had no callers and are deleted. * refactor(analytics): move wallet events onto the shared builders Wallet connect/request-amount/failed/cancel in SwapViewModel now go through WalletEvents instead of the bespoke FlipcashAnalyticsService methods, so amounts and providers convert through .analytics like the other builder call sites. transactionSubmittedToWallet had no caller and is deleted along with its event and property mapping. * refactor(analytics): move token info events onto the shared builders TokenInfoEntry.Discovery, Chat and ChatGate used to report as "Token Info: Opened From Wallet". They now send Opened From Discovery, Chat and Chat Gate, so the Wallet count drops by what those three entries carried. Deeplink and Wallet are unchanged. The mapping has no else branch, so a new TokenInfoEntry fails to compile until it is mapped. TokenInfoSource.Give and its event have had no caller since 6419a95 and are deleted. * refactor(analytics): move buy and sell events onto the shared builders SwapViewModel.trackTransaction now calls the generated SwapEvents.purchase and SwapEvents.sell builders instead of the ad-hoc Analytics.buy/sell path, dropping the local Analytics.SwapMethod/PurchaseMethod enums and the TokenTransactionEvent sealed hierarchy they drove. Fee is sent as a decimal (events.toml already defines it as `type = "decimal"`) instead of a string. * refactor(analytics): move scan events onto the shared builders Gallery scan (Scanner.kt), tip card scan (CodeScanDelegate) and tip card present (TipCardDelegate) now call analytics.track(ScanEvents.*) instead of the FlipcashAnalyticsService methods, which are deleted along with their Events.kt classes and MixpanelAnalyticsDelegate wiring. Tier, Zoom and Time go out as numbers and Exhausted as a flag, matching the shared builder; Scanner.kt has no test harness so its three call sites are reviewed by diff only. CodeScanDelegateTest and TipCardDelegateTest now assert against RecordingAnalytics with ScanEvents builders instead of verifying the legacy methods. * refactor(analytics): move deeplink events onto the shared builders App.kt, Scanner.kt and CashLinkDelegate.kt track the DeeplinkEvents builders instead of the legacy service methods. Two values change: - Type comes from DeeplinkType.analytics, an explicit string per subclass, instead of javaClass.simpleName. The strings match the source class names, but no keep rule covers DeeplinkType, so a minified release build may have sent R8-renamed names until now. - The parse-failure Error carries the URL without its query or fragment, which can hold secrets. CashLinkDelegateTest asserts the routed event, with the type as a string literal, on each early return and on success. * refactor(analytics): move display name events onto the shared builders Replaces FlipcashAnalyticsService.displayNameSubmitted(source, hadPreviousName) with DisplayNameEvents.set/updated from the generated builder, called directly from NameEntryViewModel. Deletes the old DisplayNameEvent.Set/Updated sealed classes from Events.kt, the interface/stub method from Analytics.kt, and the Mixpanel override, since nothing else referenced them. NameEntryViewModel now takes a DispatcherProvider (wired to BaseViewModel's defaultDispatcher) so its CheckName pipeline runs deterministically under a test dispatcher, matching WalletViewModel's existing pattern; needed to cover the branch through NameEntryViewModelTest with RecordingAnalytics. * refactor(analytics): move error modal events onto the shared builders NavMessagingEntryDecorator.kt tracks ErrorModalEvents.displayed instead of the legacy displayedErrorModal method. Name, keys (Title, Message, Screen, Call Site) and value types are unchanged; Screen and Call Site still drop from the map when null, matching :libs:analytics-events' ErrorModalEventsTest, which already pins that exact name and property map. The decorator has no test harness, so the call site was reviewed by diff against that existing builder test rather than given a new one. Deletes displayedErrorModal from FlipcashAnalyticsService, StubFlipcashAnalytics and MixpanelAnalyticsDelegate, and ErrorModalDisplayed from Events.kt. No dead declarations in this domain. * refactor(analytics): move button and account events onto the shared builders Button taps track ButtonEvents.tapped with the shared Button enum, and the account actions and paidForAccount track AccountEvents. Every event name is unchanged; Create Account Payment's Fiat goes out as a number. Actions.kt is gone, along with Button.AllowCamera and Button.Sell, which had no callers. Compose callers read the service through rememberAnalytics(), since LocalAnalytics is typed as the base service without track(). The login tests assert the tracked button events with RecordingAnalytics instead of verifying action() on a mock. * refactor(analytics): delete the legacy analytics service Every caller now goes through FlipcashAnalytics.track/increment, so FlipcashAnalyticsService, StubFlipcashAnalytics, Events.kt and :libs:analytics have no users left. LocalAnalytics moves into the Flipcash analytics module, typed as FlipcashAnalytics, with previews providing FlipcashAnalytics.None. The delegate's app-start trace went with the lifecycle hooks it lived in; nothing called those hooks. * test(chat): pin State: Failure on a failed chat send code/cash left successful at its default on this path, so a failed Sent Cash / Sent Tip reported State: Success; this branch sends Failure and the test keeps it that way. * refactor(analytics): move RecordingAnalytics into test fixtures It sat in the main source set, so it shipped in the app and production code could inject it. It now lives in the analytics module's testFixtures, which the nine modules whose tests use it pull in with testImplementation(testFixtures(...)), the same way :libs:coroutines and :ui:resources share their fakes. The fixture compilation needs the Compose BOM and ui on its classpath because the module applies the Compose compiler. * refactor(tipping): drop the unused analytics dependency from TippingCoordinator TippingCoordinator took a FlipcashAnalytics it never called, both before and after the track() migration. Remove the constructor parameter and the module's dependency on :apps:flipcash:shared:analytics. * test(messenger): run the chat send-failure test on the test dispatcher ChatViewModel dispatched events on the real Dispatchers.Default, so ChatSendFailureAnalyticsTest polled for up to 5s and could leave the send handler running after the test returned. CI then failed ReportViewModelTest, the next messenger test, with UncaughtExceptionsBeforeTest, which is how kotlinx-coroutines-test reports an exception thrown by an earlier test's leftover coroutine. It did not reproduce locally. Inject DispatcherProvider into ChatViewModel, as NameEntryViewModel does, and pass the test's dispatcher so no work outlives the test. Production still gets Dispatchers.Default.
bmc08gt
added a commit
that referenced
this pull request
Sep 25, 2026
…ay (#1560) * refactor(analytics): add the track interface beside the legacy service * refactor(analytics): convert Android domain types for the shared builders * refactor(analytics): move chat events onto the shared builders * refactor(analytics): move transfer events onto the shared builders Every transfer call site now tracks a TransferEvents builder. The amount conversion follows the type each caller already had (LocalFiat, or the scanned Fiat on a failed grab), so the property block is unchanged. Grab Time now goes out as a number instead of a string. The session delegate tests assert the event each grab, give, claim and cash-link share sends. TippingCoordinator lost its transfer call in #1463; only its unused Analytics import goes here. * refactor(analytics): move add money events onto the shared builders Every add-money call site now tracks an AddMoneyEvents builder instead of the legacy Analytics.AddMoney enums and FlipcashAnalyticsService methods. Fiat/LocalFiat amounts and thrown errors go through their .analytics converters, so each property block is unchanged except numbers going out as numbers instead of strings. Analytics.AddMoneySource, AddMoneyMethod, the AddMoneyEvent sealed interface, and the Mixpanel delegate overrides are dead now that every caller uses the shared builder, so they are removed. * refactor(analytics): move onramp events onto the shared builders The five contact-verification screens now track OnrampEvents.step(...) from the same LaunchedEffect. The events carry no properties, and a new test pins each step to the name the screens sent before. openOnramp, onrampPurchase, OnrampSource, OnrampPurchaseStep and their eight events had no callers and are deleted. * refactor(analytics): move wallet events onto the shared builders Wallet connect/request-amount/failed/cancel in SwapViewModel now go through WalletEvents instead of the bespoke FlipcashAnalyticsService methods, so amounts and providers convert through .analytics like the other builder call sites. transactionSubmittedToWallet had no caller and is deleted along with its event and property mapping. * refactor(analytics): move token info events onto the shared builders TokenInfoEntry.Discovery, Chat and ChatGate used to report as "Token Info: Opened From Wallet". They now send Opened From Discovery, Chat and Chat Gate, so the Wallet count drops by what those three entries carried. Deeplink and Wallet are unchanged. The mapping has no else branch, so a new TokenInfoEntry fails to compile until it is mapped. TokenInfoSource.Give and its event have had no caller since 6419a95 and are deleted. * refactor(analytics): move buy and sell events onto the shared builders SwapViewModel.trackTransaction now calls the generated SwapEvents.purchase and SwapEvents.sell builders instead of the ad-hoc Analytics.buy/sell path, dropping the local Analytics.SwapMethod/PurchaseMethod enums and the TokenTransactionEvent sealed hierarchy they drove. Fee is sent as a decimal (events.toml already defines it as `type = "decimal"`) instead of a string. * refactor(analytics): move scan events onto the shared builders Gallery scan (Scanner.kt), tip card scan (CodeScanDelegate) and tip card present (TipCardDelegate) now call analytics.track(ScanEvents.*) instead of the FlipcashAnalyticsService methods, which are deleted along with their Events.kt classes and MixpanelAnalyticsDelegate wiring. Tier, Zoom and Time go out as numbers and Exhausted as a flag, matching the shared builder; Scanner.kt has no test harness so its three call sites are reviewed by diff only. CodeScanDelegateTest and TipCardDelegateTest now assert against RecordingAnalytics with ScanEvents builders instead of verifying the legacy methods. * refactor(analytics): move deeplink events onto the shared builders App.kt, Scanner.kt and CashLinkDelegate.kt track the DeeplinkEvents builders instead of the legacy service methods. Two values change: - Type comes from DeeplinkType.analytics, an explicit string per subclass, instead of javaClass.simpleName. The strings match the source class names, but no keep rule covers DeeplinkType, so a minified release build may have sent R8-renamed names until now. - The parse-failure Error carries the URL without its query or fragment, which can hold secrets. CashLinkDelegateTest asserts the routed event, with the type as a string literal, on each early return and on success. * refactor(analytics): move display name events onto the shared builders Replaces FlipcashAnalyticsService.displayNameSubmitted(source, hadPreviousName) with DisplayNameEvents.set/updated from the generated builder, called directly from NameEntryViewModel. Deletes the old DisplayNameEvent.Set/Updated sealed classes from Events.kt, the interface/stub method from Analytics.kt, and the Mixpanel override, since nothing else referenced them. NameEntryViewModel now takes a DispatcherProvider (wired to BaseViewModel's defaultDispatcher) so its CheckName pipeline runs deterministically under a test dispatcher, matching WalletViewModel's existing pattern; needed to cover the branch through NameEntryViewModelTest with RecordingAnalytics. * refactor(analytics): move error modal events onto the shared builders NavMessagingEntryDecorator.kt tracks ErrorModalEvents.displayed instead of the legacy displayedErrorModal method. Name, keys (Title, Message, Screen, Call Site) and value types are unchanged; Screen and Call Site still drop from the map when null, matching :libs:analytics-events' ErrorModalEventsTest, which already pins that exact name and property map. The decorator has no test harness, so the call site was reviewed by diff against that existing builder test rather than given a new one. Deletes displayedErrorModal from FlipcashAnalyticsService, StubFlipcashAnalytics and MixpanelAnalyticsDelegate, and ErrorModalDisplayed from Events.kt. No dead declarations in this domain. * refactor(analytics): move button and account events onto the shared builders Button taps track ButtonEvents.tapped with the shared Button enum, and the account actions and paidForAccount track AccountEvents. Every event name is unchanged; Create Account Payment's Fiat goes out as a number. Actions.kt is gone, along with Button.AllowCamera and Button.Sell, which had no callers. Compose callers read the service through rememberAnalytics(), since LocalAnalytics is typed as the base service without track(). The login tests assert the tracked button events with RecordingAnalytics instead of verifying action() on a mock. * refactor(analytics): delete the legacy analytics service Every caller now goes through FlipcashAnalytics.track/increment, so FlipcashAnalyticsService, StubFlipcashAnalytics, Events.kt and :libs:analytics have no users left. LocalAnalytics moves into the Flipcash analytics module, typed as FlipcashAnalytics, with previews providing FlipcashAnalytics.None. The delegate's app-start trace went with the lifecycle hooks it lived in; nothing called those hooks. * test(chat): pin State: Failure on a failed chat send code/cash left successful at its default on this path, so a failed Sent Cash / Sent Tip reported State: Success; this branch sends Failure and the test keeps it that way. * refactor(analytics): move RecordingAnalytics into test fixtures It sat in the main source set, so it shipped in the app and production code could inject it. It now lives in the analytics module's testFixtures, which the nine modules whose tests use it pull in with testImplementation(testFixtures(...)), the same way :libs:coroutines and :ui:resources share their fakes. The fixture compilation needs the Compose BOM and ui on its classpath because the module applies the Compose compiler. * refactor(tipping): drop the unused analytics dependency from TippingCoordinator TippingCoordinator took a FlipcashAnalytics it never called, both before and after the track() migration. Remove the constructor parameter and the module's dependency on :apps:flipcash:shared:analytics. * test(messenger): run the chat send-failure test on the test dispatcher ChatViewModel dispatched events on the real Dispatchers.Default, so ChatSendFailureAnalyticsTest polled for up to 5s and could leave the send handler running after the test returned. CI then failed ReportViewModelTest, the next messenger test, with UncaughtExceptionsBeforeTest, which is how kotlinx-coroutines-test reports an exception thrown by an earlier test's leftover coroutine. It did not reproduce locally. Inject DispatcherProvider into ChatViewModel, as NameEntryViewModel does, and pass the test's dispatcher so no work outlives the test. Production still gets Dispatchers.Default. * fix(tokens): track currency-info opens and shares from the card overlay Tapping a wallet card opens CurrencyInfoExpansion, an overlay rather than the pushed TokenInfoScreen, and the overlay tracked nothing. Wallet opens never sent `Token Info: Opened From Wallet`, and its share button never sent `Button: Share Token Info`. Deeplinks open the same overlay through beginExpanded, so `Opened From Deeplink` was missing too. iOS sends all three from WalletScreen.openCard, openCardImmediately and the share link. The open event fires once per open, keyed on the mint, with WALLET when the card flew out of the deck and DEEPLINK when there was no source card. The "already tracked" flag is rememberSaveable because a pushed Give, Convert or Withdraw takes the wallet entry, and the overlay with it, out of composition. The entry's saved state outlives that, so coming back does not count a second open.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purely subtractive. Scanning a tip card now opens the recipient's chat (node 10074:18893), and the chat charges the same fee the modal used to collect, so the whole scan-and-tip path is reachable by nothing. Each deletion was grepped for callers first:
AppRoute.Sheets.TipAmountEntryand its entry inAppScreenContentTipAmountEntryScreenandTipAmountEntryViewModelTippingCoordinator.confirmTip, theTipEvent.LaunchChatit emitted on success, and the privatesetSendStatethat only it calledTokenPurpose.Tipand itswhenbranches across token selecttitle_amountToTipandlabel_ofTwo knock-on simplifications:
TipCardDecorator's event collector handles one case now, so thewhencollapses andnavigateAllloses its use there; andTokenPurpose.Tipwas the only purpose forcing a close (X) on token select, so that screen takes the ambient dismiss style and drops theCompositionLocalProvideraround it.What stays, and why
subtitle_tipHintMinimum,error_title_tipMinimum,error_description_tipMinimum— the chat's own amount entry still enforces a floor and still reports breaking it.label_tipUser(three callers) andtitle_sendTip(the chat call-to-action's copy).action_swipeToTip— feat(chat): send to start chatting #1459 removed its last Kotlin caller, so the string is now orphaned too. Left for the same follow-up as the items below rather than widened into here.TipSelectionState.sendState,selectAmount,TipAmount,maxTipAmount,exceedsSendLimit— orphaned by the same change but left alone here. Removing them pullsTipSelectionHolderapart, which is a wider edit than this PR's subject; worth its own pass.TippingCoordinatorTestloses theconfirmTipcase, which covered one invariant — a tip card always reportsTipAction.TIPon the wire, neverSEND. Its reflection helper for seeding the coordinator's private recipient state existed only for that test and goes with it. The invariant itself is gone rather than untested: nothing sends through a tip card any more.