[Suggested Follow-ups][R3] Implement instant reply + optimistic reconciliation #80918
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,26 +1,35 @@ | ||
| import type {OnyxEntry} from 'react-native-onyx'; | ||
| import Onyx from 'react-native-onyx'; | ||
| import {rand64} from '@libs/NumberUtils'; | ||
| import type {Followup} from '@libs/ReportActionFollowupUtils'; | ||
| import type {Ancestor, OptimisticReportAction} from '@libs/ReportUtils'; | ||
| import {buildOptimisticAddCommentReportAction} from '@libs/ReportUtils'; | ||
| import CONST from '@src/CONST'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import type {Report, ReportAction} from '@src/types/onyx'; | ||
| 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; | ||
|
|
||
| /** | ||
| * Resolves a suggested followup by posting the selected question as a comment | ||
| * and optimistically updating the HTML to mark the followup-list as resolved. | ||
| * If the followup has a pre-generated response, it will show a "Concierge is typing" | ||
| * indicator briefly before displaying the response. | ||
| * @param report - The report where the action exists | ||
| * @param notifyReportID - The report ID to notify for new actions | ||
| * @param reportAction - The report action containing the followup-list | ||
| * @param selectedFollowup - The followup object containing the question text and optional pre-generated response | ||
| * @param timezoneParam - The user's timezone | ||
| * @param ancestors - Array of ancestor reports for proper threading | ||
| */ | ||
| function resolveSuggestedFollowup( | ||
| report: OnyxEntry<Report>, | ||
| notifyReportID: string | undefined, | ||
| reportAction: OnyxEntry<ReportAction>, | ||
| selectedFollowup: Followup, | ||
| timezoneParam: Timezone, | ||
| ancestors: Ancestor[] = [], | ||
| ) { | ||
|
|
@@ -42,8 +51,49 @@ function resolveSuggestedFollowup( | |
| [reportActionID]: resolvedAction, | ||
| }); | ||
|
|
||
| if (!selectedFollowup.response) { | ||
| addComment(report, notifyReportID ?? reportID, ancestors, selectedFollowup.text, timezoneParam); | ||
| return; | ||
| } | ||
|
|
||
| // If there's a pre-generated response, show typing indicator then display response after delay | ||
|
|
||
| const optimisticConciergeReportActionID = rand64(); | ||
|
|
||
| // Post user's comment immediately | ||
| addComment(report, notifyReportID ?? reportID, ancestors, selectedFollowup.text, timezoneParam, false, false, { | ||
|
Contributor
There was a problem hiding this comment. ❌ CONSISTENCY-5 (docs)There is no error handling if the API call for This creates a confusing UX where Concierge appears to respond to a message that failed to send. Suggested fix: The timeout should be tied to the API calls success. Consider passing a cleanup callback to // One approach: pass the optimistic action as part of the API call
// and let the API success handler trigger the delayed response
// Alternative: Return a promise from addComment and only schedule on success
const commentResult = await addComment(...);
if (commentResult.success) {
addOptimisticConciergeActionWithDelay(reportID, optimisticConciergeAction);
}Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
Contributor
Author
There was a problem hiding this comment. same as below : this is actually it's getting cleared in |
||
| optimisticConciergeReportActionID, | ||
| pregeneratedResponse: selectedFollowup.response, | ||
| }); | ||
|
|
||
| const optimisticConciergeAction = buildOptimisticAddCommentReportAction( | ||
| selectedFollowup.response, | ||
| undefined, | ||
| CONST.ACCOUNT_ID.CONCIERGE, | ||
| CONCIERGE_RESPONSE_DELAY_MS, | ||
|
Comment on lines
+69
to
+73
There was a problem hiding this comment.
The optimistic concierge response is created with Useful? React with 👍 / 👎. |
||
| reportID, | ||
| optimisticConciergeReportActionID, | ||
| ); | ||
|
|
||
| addOptimisticConciergeActionWithDelay(reportID, optimisticConciergeAction); | ||
| } | ||
|
|
||
| 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, | ||
| }); | ||
|
|
||
| setTimeout(() => { | ||
|
jmusial marked this conversation as resolved.
Contributor
There was a problem hiding this comment. If setTimeout, there should be clearTimeout
Contributor
Author
There was a problem hiding this comment. this one is not in the component lifecycle and the danger of user navigating away during that is small. It'll be garbage collected after it fires, so I think it's ok to leave as is. |
||
| // Clear the typing indicator | ||
| Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${reportID}`, { | ||
| [CONST.ACCOUNT_ID.CONCIERGE]: false, | ||
| }); | ||
|
jmusial marked this conversation as resolved.
|
||
|
|
||
| Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { | ||
| [optimisticConciergeAction.reportAction.reportActionID]: optimisticConciergeAction.reportAction, | ||
| }); | ||
|
Comment on lines
+93
to
+95
There was a problem hiding this comment.
The optimistic concierge response is merged after a fixed delay regardless of whether the underlying Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. responded to that above: it's getting cleared in
Comment on lines
+93
to
+95
There was a problem hiding this comment.
This merge is scheduled unconditionally after a timeout, so if the underlying Useful? React with 👍 / 👎. |
||
| }, CONCIERGE_RESPONSE_DELAY_MS); | ||
|
Contributor
There was a problem hiding this comment. We usually don't accept setTimeout but 1.5s is reliable delay duration?
Contributor
There was a problem hiding this comment. For this one is fine, since we don't want to show the optimistic response so it can feels a bit more natural
Contributor
Author
There was a problem hiding this comment. This is just to mimic "real person" typing effect from concierge a visual gimmick, |
||
| } | ||
|
|
||
| export {resolveSuggestedFollowup, CONCIERGE_RESPONSE_DELAY_MS}; | ||
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.
Confirmed this is safe removal
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.
yup, confirmed with the design team. We added it in previous PR, should be all g