-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[No QA] Improve short delay before showing Concierge suggested response answer #82154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5af8b4e
840ebf0
178d253
26e90ed
22166d9
f960fcd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import {useEffect} from 'react'; | ||
| import {applyPendingConciergeAction, discardPendingConciergeAction} from '@libs/actions/Report/SuggestedFollowup'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import useOnyx from './useOnyx'; | ||
|
|
||
| /** If displayAfter is more than this far in the past, the response is stale (e.g. app was killed and restarted) */ | ||
| const STALE_THRESHOLD_MS = 10_000; | ||
|
|
||
| /** | ||
| * Processes pending concierge responses stored in Onyx for a given report. | ||
| * When a pending response exists, schedules the action to be moved to REPORT_ACTIONS | ||
| * after the remaining delay, with automatic cleanup on unmount via useEffect. | ||
| */ | ||
| function usePendingConciergeResponse(reportID: string) { | ||
| const [pendingResponse] = useOnyx(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${reportID}`, {canBeMissing: true}); | ||
|
|
||
| useEffect(() => { | ||
| if (!pendingResponse) { | ||
| return; | ||
| } | ||
|
|
||
| const remaining = pendingResponse.displayAfter - Date.now(); | ||
|
|
||
| // If the pending response is stale (e.g. app was killed/restarted), discard it | ||
| // instead of displaying a phantom message that was never confirmed by the server. | ||
| if (remaining < -STALE_THRESHOLD_MS) { | ||
| discardPendingConciergeAction(reportID); | ||
| return; | ||
| } | ||
|
|
||
| const timer = setTimeout( | ||
| () => { | ||
| applyPendingConciergeAction(reportID, pendingResponse.reportAction); | ||
| }, | ||
| Math.max(0, remaining), | ||
| ); | ||
|
|
||
| return () => clearTimeout(timer); | ||
| }, [pendingResponse, reportID]); | ||
| } | ||
|
|
||
| export default usePendingConciergeResponse; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ import type {Timezone} from '@src/types/onyx/PersonalDetails'; | |
| import {addComment, buildOptimisticResolvedFollowups} from '.'; | ||
|
|
||
| /** Delay before showing pre-generated Concierge response (in milliseconds) */ | ||
| const CONCIERGE_RESPONSE_DELAY_MS = 1500; | ||
| const CONCIERGE_RESPONSE_DELAY_MS = 4000; | ||
|
|
||
| /** | ||
| * Resolves a suggested followup by posting the selected question as a comment | ||
|
|
@@ -89,22 +89,73 @@ function resolveSuggestedFollowup( | |
| addOptimisticConciergeActionWithDelay(reportID, optimisticConciergeAction); | ||
| } | ||
|
|
||
| /** | ||
| * Queues an optimistic concierge response for delayed display. | ||
| * Writes action to Onyx — the usePendingConciergeResponse hook | ||
| * handles the actual delay and moves the action to REPORT_ACTIONS | ||
| * when the time arrives, with proper lifecycle cleanup. | ||
| */ | ||
| function addOptimisticConciergeActionWithDelay(reportID: string, optimisticConciergeAction: OptimisticReportAction) { | ||
| // Show "Concierge is typing..." indicator | ||
| Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${reportID}`, { | ||
| [CONST.ACCOUNT_ID.CONCIERGE]: true, | ||
| }); | ||
| Onyx.update([ | ||
| // Store the pending response for the scheduler to process | ||
| { | ||
| onyxMethod: Onyx.METHOD.SET, | ||
| key: `${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${reportID}`, | ||
| value: { | ||
|
Comment on lines
+102
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Storing each delayed Concierge reply at a single key ( Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should be only one follow up for the report in that 4s window.
Comment on lines
+102
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎. |
||
| reportAction: optimisticConciergeAction.reportAction, | ||
| displayAfter: Date.now() + CONCIERGE_RESPONSE_DELAY_MS, | ||
| }, | ||
| }, | ||
| // Show "Concierge is typing..." indicator | ||
| { | ||
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: `${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${reportID}`, | ||
| value: {[CONST.ACCOUNT_ID.CONCIERGE]: true}, | ||
| }, | ||
| ]); | ||
| } | ||
|
|
||
| setTimeout(() => { | ||
| // Clear the typing indicator | ||
| Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${reportID}`, { | ||
| [CONST.ACCOUNT_ID.CONCIERGE]: false, | ||
| }); | ||
| /** | ||
| * Discards a stale pending concierge response and clears the typing indicator. | ||
| * Called when the response has been pending too long (e.g. app was killed and restarted). | ||
| */ | ||
| function discardPendingConciergeAction(reportID: string) { | ||
| Onyx.update([ | ||
| { | ||
| onyxMethod: Onyx.METHOD.SET, | ||
| key: `${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${reportID}`, | ||
| value: null, | ||
| }, | ||
| { | ||
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: `${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${reportID}`, | ||
| value: {[CONST.ACCOUNT_ID.CONCIERGE]: false}, | ||
| }, | ||
| ]); | ||
| } | ||
|
|
||
| Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { | ||
| [optimisticConciergeAction.reportAction.reportActionID]: optimisticConciergeAction.reportAction, | ||
| }); | ||
| }, CONCIERGE_RESPONSE_DELAY_MS); | ||
| /** | ||
| * Applies a pending concierge response by moving it to REPORT_ACTIONS | ||
| * and clearing the pending state and typing indicator. | ||
| */ | ||
| function applyPendingConciergeAction(reportID: string, reportAction: ReportAction) { | ||
| Onyx.update([ | ||
| { | ||
| onyxMethod: Onyx.METHOD.SET, | ||
| key: `${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${reportID}`, | ||
| value: null, | ||
| }, | ||
| { | ||
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: `${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${reportID}`, | ||
| value: {[CONST.ACCOUNT_ID.CONCIERGE]: false}, | ||
| }, | ||
| { | ||
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, | ||
| value: {[reportAction.reportActionID]: reportAction}, | ||
| }, | ||
| ]); | ||
| } | ||
|
|
||
| export {resolveSuggestedFollowup, CONCIERGE_RESPONSE_DELAY_MS}; | ||
| export {resolveSuggestedFollowup, discardPendingConciergeAction, applyPendingConciergeAction, CONCIERGE_RESPONSE_DELAY_MS}; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import type ReportAction from './ReportAction'; | ||
|
|
||
| /** Pending concierge response queued for delayed display in a report */ | ||
| type PendingConciergeResponse = { | ||
| /** The optimistic report action to add after the delay */ | ||
| reportAction: ReportAction; | ||
|
|
||
| /** Timestamp (ms) after which the response should be displayed */ | ||
| displayAfter: number; | ||
| }; | ||
|
|
||
| export default PendingConciergeResponse; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stale guard drops pending responses whenever
displayAfteris more than 10s old, but this path is also hit during normal navigation (not just app restarts):useEffectclears the timer on unmount, so if a user leaves the report and comes back after ~10s, the queued Concierge reply is discarded instead of shown. This regresses the delayed-response flow for active sessions and can make a selected follow-up appear to have no Concierge answer.Useful? React with 👍 / 👎.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The optimistic follow up is later replaced by actual response that comes from Concierge on BE. I think that if the user comes back after 10 s it's ok to just wait for the actual one. What do you think @marcochavezf ?