Skip to content

Commit 9b428c1

Browse files
authored
Merge pull request Expensify#88630 from Expensify/issa-remove-optimistic-concierge-typing-updates
Remove optimistic Concierge typing updates for delayed follow-up responses
2 parents 4cfb6a6 + 520b4bb commit 9b428c1

3 files changed

Lines changed: 22 additions & 46 deletions

File tree

src/libs/actions/Report/SuggestedFollowup.ts

Lines changed: 14 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@ const CONCIERGE_RESPONSE_DELAY_MS = 4000;
1717
/**
1818
* Resolves a suggested followup by posting the selected question as a comment
1919
* and optimistically updating the HTML to mark the followup-list as resolved.
20-
* If the followup has a pre-generated response, it will show a "Concierge is typing"
21-
* indicator briefly before displaying the response.
20+
* If the followup has a pre-generated response, it will be queued and displayed
21+
* after a short delay. The server-owned agentZeroProcessingIndicator NVP drives
22+
* the "Concierge is thinking..." indicator during that window.
2223
* @param report - The report where the action exists
2324
* @param notifyReportID - The report ID to notify for new actions
2425
* @param reportAction - The report action containing the followup-list
@@ -66,8 +67,7 @@ function resolveSuggestedFollowup(
6667
return;
6768
}
6869

69-
// If there's a pre-generated response, show typing indicator then display response after delay
70-
70+
// If there's a pre-generated response, queue it for delayed display.
7171
const optimisticConciergeReportActionID = rand64();
7272

7373
// Post user's comment immediately
@@ -110,47 +110,26 @@ function resolveSuggestedFollowup(
110110
* when the time arrives, with proper lifecycle cleanup.
111111
*/
112112
function addOptimisticConciergeActionWithDelay(reportID: string, optimisticConciergeAction: OptimisticReportAction) {
113-
Onyx.update([
114-
// Store the pending response for the scheduler to process
115-
{
116-
onyxMethod: Onyx.METHOD.SET,
117-
key: `${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${reportID}`,
118-
value: {
119-
reportAction: optimisticConciergeAction.reportAction,
120-
displayAfter: Date.now() + CONCIERGE_RESPONSE_DELAY_MS,
121-
},
122-
},
123-
// Show "Concierge is typing..." indicator
124-
{
125-
onyxMethod: Onyx.METHOD.MERGE,
126-
key: `${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${reportID}`,
127-
value: {[CONST.ACCOUNT_ID.CONCIERGE]: true},
128-
},
129-
]);
113+
// The "Concierge is thinking..." indicator is driven by the server-owned
114+
// agentZeroProcessingIndicator NVP, so this client-local delayed response
115+
// must not also write REPORT_USER_IS_TYPING for Concierge.
116+
Onyx.set(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${reportID}`, {
117+
reportAction: optimisticConciergeAction.reportAction,
118+
displayAfter: Date.now() + CONCIERGE_RESPONSE_DELAY_MS,
119+
});
130120
}
131121

132122
/**
133-
* Discards a stale pending concierge response and clears the typing indicator.
123+
* Discards a stale pending concierge response.
134124
* Called when the response has been pending too long (e.g. app was killed and restarted).
135125
*/
136126
function discardPendingConciergeAction(reportID: string | undefined) {
137-
Onyx.update([
138-
{
139-
onyxMethod: Onyx.METHOD.SET,
140-
key: `${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${reportID}`,
141-
value: null,
142-
},
143-
{
144-
onyxMethod: Onyx.METHOD.MERGE,
145-
key: `${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${reportID}`,
146-
value: {[CONST.ACCOUNT_ID.CONCIERGE]: false},
147-
},
148-
]);
127+
Onyx.set(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${reportID}`, null);
149128
}
150129

151130
/**
152131
* Applies a pending concierge response by moving it to REPORT_ACTIONS
153-
* and clearing the pending state and typing indicator.
132+
* and clearing the pending state.
154133
*/
155134
function applyPendingConciergeAction(reportID: string | undefined, reportAction: ReportAction) {
156135
Onyx.update([
@@ -159,11 +138,6 @@ function applyPendingConciergeAction(reportID: string | undefined, reportAction:
159138
key: `${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${reportID}`,
160139
value: null,
161140
},
162-
{
163-
onyxMethod: Onyx.METHOD.MERGE,
164-
key: `${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${reportID}`,
165-
value: {[CONST.ACCOUNT_ID.CONCIERGE]: false},
166-
},
167141
{
168142
onyxMethod: Onyx.METHOD.MERGE,
169143
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`,

tests/actions/ReportTest.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5429,9 +5429,11 @@ describe('actions/Report', () => {
54295429
expect(pendingResponse?.reportAction.actorAccountID).toBe(CONST.ACCOUNT_ID.CONCIERGE);
54305430
expect(pendingResponse?.displayAfter).toBeGreaterThan(Date.now() - CONCIERGE_RESPONSE_DELAY_MS);
54315431

5432-
// Verify the typing indicator was set
5432+
// Verify the client did NOT set REPORT_USER_IS_TYPING. The server-owned
5433+
// agentZeroProcessingIndicator is the source of truth for the "Concierge is thinking..."
5434+
// state during the delayed pre-generated response window.
54335435
const typingStatus = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${REPORT_ID}` as const);
5434-
expect(typingStatus?.[CONST.ACCOUNT_ID.CONCIERGE]).toBe(true);
5436+
expect(typingStatus?.[CONST.ACCOUNT_ID.CONCIERGE]).toBeFalsy();
54355437
});
54365438

54375439
it('should create Concierge response with timestamp strictly after the user comment', async () => {

tests/unit/hooks/usePendingConciergeResponse.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,9 @@ describe('usePendingConciergeResponse', () => {
6767
const pendingResponse = await getOnyxValue(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${REPORT_ID}` as const);
6868
expect(pendingResponse).toBeUndefined();
6969

70-
// And the typing indicator should be cleared
70+
// And the typing state should be left untouched
7171
const typingStatus = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${REPORT_ID}` as const);
72-
expect(typingStatus?.[CONST.ACCOUNT_ID.CONCIERGE]).toBe(false);
72+
expect(typingStatus?.[CONST.ACCOUNT_ID.CONCIERGE]).toBe(true);
7373
});
7474

7575
it('should not move the action before the delay elapses', async () => {
@@ -175,9 +175,9 @@ describe('usePendingConciergeResponse', () => {
175175
const pendingResponse = await getOnyxValue(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${REPORT_ID}` as const);
176176
expect(pendingResponse).toBeUndefined();
177177

178-
// And the typing indicator should be cleared
178+
// And the typing state should be left untouched
179179
const typingStatus = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${REPORT_ID}` as const);
180-
expect(typingStatus?.[CONST.ACCOUNT_ID.CONCIERGE]).toBe(false);
180+
expect(typingStatus?.[CONST.ACCOUNT_ID.CONCIERGE]).toBe(true);
181181
});
182182

183183
it('should do nothing when there is no pending response', async () => {

0 commit comments

Comments
 (0)