Skip to content

Commit

Permalink
fix(answerAPI): controller search listener (#4531)
Browse files Browse the repository at this point in the history
https://coveord.atlassian.net/browse/SVCC-4254 

SVCC-4254

# headless-answerapi-generated-answer fix
## Two triggers to fetchAnswer are made after an empty request.
uselessly hitting the RTK query cache

### Problem description
When the answer-api is enabled

**TLDR**
Whenever a search request was made after a precedent search request is
made with an empty query, the answerAPI was called two times with the
same parameters. hitting the RTK query cache while painting red in the
console.

**Further**
The last params with which a search was triggered are kept in a variable
used to compare with the current params the listener current run uses.
The comparison between the lastParams and the current ones determines if
we should call the answerAPI or not. Because the query in the state is
not updated at the same time than the request ID,the listener is run
twice. If we do not update the `lastTriggerParams` when the query is
empty, the condition will receive a wrong request ID the first run and
let it pass through, calling the answerAPI twice with the same params.

### Fix description

The current fix is not changing the user behavior. Since RTK query was
preventing the API to be called twice anyway. But we esteem that the
implementation should not rely on RTK query being nice.
 
The `lastTriggerParams` are now updated even when the query is empty
when the listener is called. Preventing the condition to let pass the
query with the exact same parameters.

Co-authored-by: Danny Gauthier <[email protected]>
  • Loading branch information
dmgauthier and Danny Gauthier authored Oct 28, 2024
1 parent 5c235a8 commit 3b02591
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,30 @@ vi.mock(
'../../../features/generated-answer/generated-answer-analytics-actions'
);
vi.mock('../../../features/search/search-actions');

vi.mock('../../../api/knowledge/stream-answer-api', async () => {
const originalStreamAnswerApi = await vi.importActual(
'../../../api/knowledge/stream-answer-api'
);
const queryCounter = {count: 0};
const queries = [
{q: '', requestId: ''},
{q: 'this est une question', requestId: '12'},
{q: 'this est une another question', requestId: '12'},
{q: '', requestId: '34'},
{q: 'this est une yet another question', requestId: '56'},
];
return {
...originalStreamAnswerApi,
fetchAnswer: vi.fn(),
selectAnswer: () => ({
data: {answer: 'This est une answer', answerId: '12345_6'},
}),
selectAnswerTriggerParams: () => {
const query = {...queries[queryCounter.count]};
queryCounter.count++;
return query;
},
};
});
vi.mock('../../../api/knowledge/post-answer-evaluation', () => ({
Expand All @@ -54,10 +68,6 @@ vi.mock('../../../api/knowledge/post-answer-evaluation', () => ({
}));

describe('knowledge-generated-answer', () => {
it('should be tested', () => {
expect(true).toBe(true);
});

let engine: MockedSearchEngine;

const createGeneratedAnswer = (props: GeneratedAnswerProps = {}) =>
Expand Down Expand Up @@ -175,4 +185,32 @@ describe('knowledge-generated-answer', () => {
expectedArgs
);
});

describe('subscribeToSearchRequest', () => {
it('triggers a fetchAnswer only when there is a request id, a query, and the request is made with another request than the last one', () => {
createGeneratedAnswer();

const listener = engine.subscribe.mock.calls[0][0];

// no request id, no call
listener();
expect(fetchAnswer).not.toHaveBeenCalled();

// first request id, call
listener();
expect(fetchAnswer).toHaveBeenCalledTimes(1);

// same request id, no call
listener();
expect(fetchAnswer).toHaveBeenCalledTimes(1);

// empty query, no call
listener();
expect(fetchAnswer).toHaveBeenCalledTimes(1);

// new request id, call
listener();
expect(fetchAnswer).toHaveBeenCalledTimes(2);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,24 @@ const subscribeToSearchRequest = (
const strictListener = () => {
const state = engine.state;
const triggerParams = selectAnswerTriggerParams(state);
if (triggerParams.q.length === 0 || triggerParams.requestId.length === 0) {
return;

if (!lastTriggerParams || triggerParams.q.length === 0) {
lastTriggerParams = triggerParams;
}
if (triggerParams?.requestId === lastTriggerParams?.requestId) {

if (
triggerParams.q.length === 0 ||
triggerParams.requestId.length === 0 ||
triggerParams.requestId === lastTriggerParams.requestId
) {
return;
}

lastTriggerParams = triggerParams;
engine.dispatch(resetAnswer());
engine.dispatch(fetchAnswer(state));
};

engine.subscribe(strictListener);
};

Expand Down

0 comments on commit 3b02591

Please sign in to comment.