Repository navigation
Open a support ticket straight from the Talk to a human link - #104572
ishpaul777 wants to merge 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [isOffline]); | ||
|
|
||
| return <FullScreenLoadingIndicator shouldUseGoBackButton={false} />; |
There was a problem hiding this comment.
❌ UI-1 (docs)
This screen renders a FullScreenLoadingIndicator with no navigation component (no HeaderWithBackButton or close button) in the same return, and it explicitly sets shouldUseGoBackButton={false}. If opening the support ticket hangs (slow or stuck network response), the user is trapped on a full-screen spinner with no escape route. The shouldUseGoBackButton={false} escape hatch is reserved for cases where going back is dead or harmful (global overlays, mid sign-out). Here going back is safe and useful, so the emergency "Go Back" button should be available.
Remove the shouldUseGoBackButton={false} override (it defaults to true), or set it explicitly to true:
return <FullScreenLoadingIndicator shouldUseGoBackButton />;Reviewed at: 8b33174 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| } | ||
| hasRequestedTicket.current = true; | ||
| openSupportTicketOrConcierge(); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
❌ CONSISTENCY-5 (docs)
The eslint-disable-next-line react-hooks/exhaustive-deps suppresses the exhaustive-deps rule without an accompanying comment explaining why openSupportTicketOrConcierge is intentionally omitted from the dependency array. Since this is a permanent suppression (the effect should only re-run when isOffline changes), add a justification so future maintainers understand the intent.
// openSupportTicketOrConcierge is a new reference every render; the hasRequestedTicket ref already
// guards against re-running, so we only want this effect to react to isOffline changing.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isOffline]);Reviewed at: 8b33174 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
|
Please ping me when ready for review. Thanks |
![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: 8b33174b58
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return; | ||
| } | ||
| hasRequestedTicket.current = true; | ||
| openSupportTicketOrConcierge(); |
There was a problem hiding this comment.
When this effect is entered from a Concierge link, openSupportTicket() calls getReportRouteForCurrentContext(), which records the currently active settings/help/talk-to-human route as the new report's backTo; ReportHeader honors that value when the customer leaves the ticket. The settings tab remains mounted with hasRequestedTicket.current set, so Back returns to this page's permanent loader (which explicitly has no Go Back button) instead of the originating chat; if the page was remounted, it can instead create another ticket. Remove/replace this transient route or preserve the pre-link route before invoking the action.
Useful? React with 👍 / 👎.
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.
|
This branch has not been deployed
2 participants
Explanation of Change
Concierge now ends most replies with a Talk to a human link instead of escalating (Web-Expensify #56788). That link currently lands on Account > Help, where the customer still has to find and press the Talk to a human row. This adds a
settings/help/talk-to-humanroute that performs that step for them, so the link opens a support ticket directly.The ticket logic already existed in
openSupportTicket(); the only new behaviour is an entry point for it. Ticket creation and the no-rep-available fallback to Concierge are extracted intouseOpenSupportTicket, so the Help page row and this route stay identical. A route is used rather than intercepting link presses, so email links, refreshes and mobile deep links all work.The route leaves itself before creating the ticket, so the ticket's Back returns to the chat the link was pressed in instead of this loader. With nothing behind it (a direct link) it falls back to the Help page and opens the ticket from there. Offline returns to the Help page, since ticket creation needs the network, and a ref keeps a remount from opening a second ticket.
Deploy order: this ships before Auth and Web-Expensify, which point the link here.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/684546
Tests
settings/help/talk-to-humandirectly in the URL bar. Verify it opens a ticket and Back lands on the Help page.Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)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