Confirm new expenses with a “View” toast on Spend instead of scrolling to and highlighting the row - #94866
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
4754efa to
4af5484
Compare
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
- Fix multi-receipt batches firing navigation/growl once per receipt:
createTransaction now threads isLastTransactionOfBatch into
requestMoney/trackExpense so post-create feedback fires only on the
final transaction
- Forward backToReport from useExpenseSubmission into requestMoney so
post-create navigation returns to the originating report
- Narrow cleanupAfterSkipConfirmSubmit to cleanup-only
(CleanupAfterExpenseCreateParams): the write action owns post-create
navigation now, so the dispatcher must never navigate too; trim the
dead navigation params from all skip-confirm call sites and drop the
now-unused action/getExistingTransactionID plumbing from
ScanRouter/ScanSkipConfirmation/distance navigation
- Trim performPostBatchCleanup to {allTransactionsCreated} and remove
the dead chat-target resolution from useExpenseSubmission
- Remove the newSearchResultKeys highlight stamping from
useSearchSnapshot and the shouldUseLiveData prop from
useSearchAutoRefetch (both dead after the growl replaced the
post-create highlight)
- showExpenseAddedGrowl: clear the safety timeout once resolved, resolve
immediately when there is no iouReportID to subscribe to, and drop the
microtask deferral on the single-transaction inbox path
- Update affected tests for the cleanup-only contract
- Remove the dead shouldNavigate param from navigateAfterExpenseCreate;
its "feedback without navigation" job is fully owned by
surfaceExpenseCreatedFeedback since the nav-ownership refactor
- Resolve the growl View button's inbox/spend context at press time
instead of freezing it at growl-show time (drop the isInbox param)
- Delete the cleanupAfterSkipConfirmSubmit passthrough; call sites use
cleanupAfterExpenseCreate directly
- Restore backToReport on skip-confirm submits: createTransaction
accepts and forwards it to requestMoney, AmountSubmission passes it -
in-chat "Add expense" flows return to the originating chat again
- Scope the per-iteration optimistic transaction id to the submit loops
in useExpenseSubmission (the outer "last" variable was a leftover)
- Rename the useSearchAutoRefetch params type to
UseSearchAutoRefetchParams
- Wire mockUseIsFocused into the navigation mock in
useSearchAutoRefetchTest so the not-focused test actually exercises
the focus branch
JakubKorytko
left a comment
JakubKorytko
left a comment
There was a problem hiding this comment.
really good job! two medium-to-minor concerns, approved!
showExpenseAddedGrowl: a provided transactionThreadReportID no longer skips the report-actions wait when an iouReportID exists - the thread ID arrives before the deferred API.write flushes, so View could open a thread not yet in Onyx. Fast path now requires no iouReportID or an already-present IOU action. Share flow: shouldHandleNavigation: false still surfaced the growl before SubmitDetailsPage navigated, and global-create paths could show it twice. Added shouldShowPostCreateFeedback (default true) so the page keeps the action silent while cleanupAndNavigateAfterExpenseCreate owns navigation and feedback. Also threaded iouReportID / transactionThreadReportID through the cleanup helper so the growl View deep link resolves. Also removes dead isTransactionMatchWithGroupItem from SearchUIUtils - this branch already deleted its only caller and its export, leaving an unused function that failed lint.
|
This PR adds a new |
|
currently recording videos, made the pr ready for review to run the AI reviews, converted back to draft |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecb42ae7ca
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
borys3kk
changed the title
Per-diem tracked to the self-DM now surfaces the "Expense added" growl on global create (and feedback on the orchestrator-owned path, which previously got nothing): submitPerDiemExpenseForSelfDM returns the transaction/thread IDs and the hook keeps its dismiss-to-self-DM navigation while matching the other flows feedback-wise. Defer transaction-thread materialization to "View" press time: showing the growl now only runs a pure resolvability check, and the context merge or optimistic thread creation (with its OpenReport write) happens in onPress - matching every other thread navigation entry point and keeping untapped growls free of Onyx/API side effects. Also: remove the now-dead resolveChatTargetForSubmitCleanup (and its test file and leftover mocks), cover the offline-stash -> reconnect fires-exactly- once path in useSearchAutoRefetchTest, and promote the growl safety timeout to CONST.TIMING.EXPENSE_ADDED_GROWL_SAFETY_TIMEOUT.
…ce-growls-pr2 # Conflicts: # src/components/Search/hooks/useSearchSnapshot.ts
|
All automated review comments are resolved, @mhawryluk will take this PR over |
…arch/view in navigateToTransactionThread
…x.connectWithoutView
…g stale value as argument
| if (channel.onFlush) { | ||
| for (const cb of channel.onFlush) { | ||
| cb(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Run `callback` once the pending optimistic write has been applied to Onyx, after the next flush of the | ||
| * most-recently-registered active channel or immediately if nothing is pending. | ||
| * Used to sequence work that depends on the optimistic data existing. | ||
| */ | ||
| function runAfterDeferredWrite(callback: () => void) { | ||
| const channel = [...channels.values()].reverse().find((c) => !c.isReserved); | ||
| if (!channel) { | ||
| callback(); | ||
| return; | ||
| } | ||
| channel.onFlush = [...(channel.onFlush ?? []), callback]; |
There was a problem hiding this comment.
This looks good to me as an interim step. runAfterDeferredWrite() without a key is not fully bulletproof as a generic primitive, since it relies on the latest non-reserved channel being the intended one. In this flow that should be OK because it is called synchronously right after the deferred write is registered.
I would avoid spreading this unkeyed pattern further, since the longer-term direction is to move this into an API-owned API.writeWhenReady(...) model: #96010
So I’m OK with this PR as long as this stays scoped to the current flow.
|
closing this one, here's the current |
Explanation of Change
Replaces the post-create Search row highlight with an "Expense added" / "Invoice sent" growl that has a "View" action deep-linking to the new expense's transaction thread RHP. The IOU actions (requestMoney/trackExpense/split/invoice/per-diem/distance) now solely own post-create navigation + feedback (batch-aware via
isLastTransactionOfBatch), fixing double navigation/growls. RemovesuseSearchHighlightAndScroll(replaced byuseSearchAutoRefetch, refetch-only) and theTRANSACTION_IDS_HIGHLIGHT_ON_SEARCH_ROUTEOnyx key.Fixed Issues
$ #91408
PROPOSAL:
Tests
Test 1: Expense-added growl with working "View" link (global create from Inbox)
Test 2: Single growl and single navigation on skip-confirm and multi-scan flows
Test 3: In-report add highlights the row instead of a growl; Share flow stays correct
Offline tests
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari