Skip to content

Commit

Permalink
fix(headless)!: clean up deprecated feedback methods (#4404)
Browse files Browse the repository at this point in the history
[SFINT-5585](https://coveord.atlassian.net/browse/SFINT-5585)

- Updated `sendFeedback` method in the generated answer controllers to
accept only GeneratedAnswerFeedbackV2, the latest feedback schema we
created to represent a feedback.
- Updated the analytic actions logGeneratedAnswerFeedback by remove the
logic that checks between v1 and v2 and use only
GeneratedAnswerFeedbackV2 type.
- Removed logGeneratedAnswerDetailedFeedback analytics action, as it is
no longer used.
- Renamed the type `GeneratedAnswerFeedbackV2` to
`GeneratedAnswerFeedback`

[SFINT-5585]:
https://coveord.atlassian.net/browse/SFINT-5585?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Co-authored-by: Louis Bompart <[email protected]>
  • Loading branch information
mmitiche and louis-bompart authored Sep 16, 2024
1 parent 475a217 commit 45e9f2d
Show file tree
Hide file tree
Showing 15 changed files with 109 additions and 338 deletions.
2 changes: 2 additions & 0 deletions .github/actions/e2e-quantic/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ runs:
with:
path: packages/quantic/cypress/plugins/config
key: quantic-cypress-config-${{ github.sha }}
- run: npx cypress install
shell: bash
- uses: cypress-io/github-action@v5
name: Run Cypress
with:
Expand Down
4 changes: 2 additions & 2 deletions packages/atomic/cypress/e2e/generated-answer-selectors.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {GeneratedAnswerFeedbackV2} from '@coveo/headless';
import {GeneratedAnswerFeedback} from '@coveo/headless';

export const generatedAnswerComponent = 'atomic-generated-answer';
export const feedbackModal = 'atomic-generated-answer-feedback-modal';
Expand Down Expand Up @@ -57,7 +57,7 @@ export const feedbackModalSelectors = {
feedbackModalSelectors.atomicModal().find('[part="modal-footer"]'),
other: () => feedbackModalSelectors.atomicModal().find('.other'),
feedbackOption: (
feedback: keyof GeneratedAnswerFeedbackV2,
feedback: keyof GeneratedAnswerFeedback,
optionText: 'No' | 'Yes' | 'Not sure'
) =>
feedbackModalSelectors
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {
GeneratedAnswer,
GeneratedAnswerFeedbackV2,
GeneratedAnswerFeedback,
GeneratedAnswerFeedbackOption,
} from '@coveo/headless';
import {
Expand Down Expand Up @@ -55,7 +55,7 @@ export class AtomicGeneratedAnswerFeedbackModal
@Prop({reflect: true, mutable: true}) helpful = false;

@State() public error!: Error;
@State() private currentAnswer: Partial<GeneratedAnswerFeedbackV2> =
@State() private currentAnswer: Partial<GeneratedAnswerFeedback> =
this.getInitialAnswerState();
@State() feedbackSubmitted: boolean = false;
@State() answerEvaluationRequired: boolean = false;
Expand All @@ -78,7 +78,7 @@ export class AtomicGeneratedAnswerFeedbackModal

private static options: {
localeKey: string;
correspondingAnswer: keyof GeneratedAnswerFeedbackV2;
correspondingAnswer: keyof GeneratedAnswerFeedback;
}[] = [
{
localeKey: 'feedback-correct-topic',
Expand All @@ -98,7 +98,7 @@ export class AtomicGeneratedAnswerFeedbackModal
},
];

private getInitialAnswerState(): Partial<GeneratedAnswerFeedbackV2> {
private getInitialAnswerState(): Partial<GeneratedAnswerFeedback> {
return {
documented: undefined,
correctTopic: undefined,
Expand Down Expand Up @@ -132,7 +132,7 @@ export class AtomicGeneratedAnswerFeedbackModal
private updateBreakpoints = once(() => updateBreakpoints(this.host));

private setCurrentAnswer(
key: keyof GeneratedAnswerFeedbackV2,
key: keyof GeneratedAnswerFeedback,
value: GeneratedAnswerFeedbackOption | string
) {
this.currentAnswer = {
Expand All @@ -142,8 +142,8 @@ export class AtomicGeneratedAnswerFeedbackModal
}

public sendFeedback() {
const feedback: GeneratedAnswerFeedbackV2 = {
...(this.currentAnswer as GeneratedAnswerFeedbackV2),
const feedback: GeneratedAnswerFeedback = {
...(this.currentAnswer as GeneratedAnswerFeedback),
helpful: this.helpful,
};
this.generatedAnswer.sendFeedback(feedback);
Expand Down Expand Up @@ -194,7 +194,7 @@ export class AtomicGeneratedAnswerFeedbackModal

private renderFeedbackOption(
option: GeneratedAnswerFeedbackOption,
correspondingAnswer: keyof GeneratedAnswerFeedbackV2
correspondingAnswer: keyof GeneratedAnswerFeedback
) {
const buttonClasses = [
'min-w-20',
Expand Down Expand Up @@ -229,7 +229,7 @@ export class AtomicGeneratedAnswerFeedbackModal

private renderAnswerEvaluation(
label: string,
correspondingAnswer: keyof GeneratedAnswerFeedbackV2
correspondingAnswer: keyof GeneratedAnswerFeedback
) {
const labelClasses = ['text-error-red', 'text-sm', 'hidden'];
const isRequired =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
generatedAnswerAnalyticsClient,
logCopyGeneratedAnswer,
logDislikeGeneratedAnswer,
logGeneratedAnswerDetailedFeedback,
logGeneratedAnswerFeedback,
logGeneratedAnswerHideAnswers,
logGeneratedAnswerShowAnswers,
Expand All @@ -23,7 +22,7 @@ import {
logOpenGeneratedAnswerSource,
logGeneratedAnswerExpand,
logGeneratedAnswerCollapse,
GeneratedAnswerFeedbackV2,
GeneratedAnswerFeedback,
} from '../../../features/generated-answer/generated-answer-analytics-actions';
import {generatedAnswerReducer} from '../../../features/generated-answer/generated-answer-slice';
import {
Expand Down Expand Up @@ -129,15 +128,8 @@ describe('generated answer', () => {
expect(closeGeneratedAnswerFeedbackModal).toHaveBeenCalled();
});

it('#sendFeedback dispatches the V1 actions', () => {
const exampleFeedback = 'notAccurate';
generatedAnswer.sendFeedback(exampleFeedback);
expect(sendGeneratedAnswerFeedback).toHaveBeenCalled();
expect(logGeneratedAnswerFeedback).toHaveBeenCalledWith(exampleFeedback);
});

it('#sendFeedback dispatches the V2 actions', () => {
const exampleFeedback: GeneratedAnswerFeedbackV2 = {
const exampleFeedback: GeneratedAnswerFeedback = {
helpful: true,
documented: 'yes',
correctTopic: 'no',
Expand All @@ -149,15 +141,6 @@ describe('generated answer', () => {
expect(logGeneratedAnswerFeedback).toHaveBeenCalledWith(exampleFeedback);
});

it('#sendDetailedFeedback dispatches the right actions', () => {
const exampleDetails = 'Example details';
generatedAnswer.sendDetailedFeedback(exampleDetails);
expect(sendGeneratedAnswerFeedback).toHaveBeenCalled();
expect(logGeneratedAnswerDetailedFeedback).toHaveBeenCalledWith(
exampleDetails
);
});

it('#logCitationClick dispatches analytics action', () => {
const testCitation = buildMockCitation();
generatedAnswer.logCitationClick(testCitation.id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ import {
expandGeneratedAnswer,
collapseGeneratedAnswer,
} from '../../../features/generated-answer/generated-answer-actions';
import {
GeneratedAnswerFeedback,
GeneratedAnswerFeedbackV2,
} from '../../../features/generated-answer/generated-answer-analytics-actions';
import {GeneratedAnswerFeedback} from '../../../features/generated-answer/generated-answer-analytics-actions';
import {generatedAnswerReducer as generatedAnswer} from '../../../features/generated-answer/generated-answer-slice';
import {GeneratedAnswerState} from '../../../features/generated-answer/generated-answer-state';
import {GeneratedResponseFormat} from '../../../features/generated-answer/generated-response-format';
Expand Down Expand Up @@ -74,15 +71,7 @@ export interface GeneratedAnswer extends Controller {
* Sends feedback about why the generated answer was not relevant.
* @param feedback - The feedback that the end user wishes to send.
*/
// TODO: Update feedback type, to change in TODO: SFINT-5585
sendFeedback(
feedback: GeneratedAnswerFeedback | GeneratedAnswerFeedbackV2
): void;
/**
* Sends detailed feedback about why the generated answer was not relevant.
* @param details - Details on why the generated answer was not relevant.
*/
sendDetailedFeedback(details: string): void;
sendFeedback(feedback: GeneratedAnswerFeedback): void;
/**
* Logs a custom event indicating a cited source link was clicked.
* @param id - The ID of the clicked citation.
Expand Down Expand Up @@ -120,9 +109,8 @@ export interface GeneratedAnswerAnalyticsClient {
logLikeGeneratedAnswer: () => CustomAction;
logDislikeGeneratedAnswer: () => CustomAction;
logGeneratedAnswerFeedback: (
feedback: GeneratedAnswerFeedback | GeneratedAnswerFeedbackV2
feedback: GeneratedAnswerFeedback
) => CustomAction;
logGeneratedAnswerDetailedFeedback: (details: string) => CustomAction;
logOpenGeneratedAnswerSource: (citationId: string) => CustomAction;
logHoverCitation: (
citationId: string,
Expand Down Expand Up @@ -240,11 +228,6 @@ export function buildCoreGeneratedAnswer(
dispatch(sendGeneratedAnswerFeedback());
},

sendDetailedFeedback(details) {
dispatch(analyticsClient.logGeneratedAnswerDetailedFeedback(details));
dispatch(sendGeneratedAnswerFeedback());
},

logCitationClick(citationId: string) {
dispatch(analyticsClient.logOpenGeneratedAnswerSource(citationId));
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,26 +124,24 @@ describe('searchapi-generated-answer', () => {
expect(closeGeneratedAnswerFeedbackModal).toHaveBeenCalled();
});

it('dispatches a send feedback action', () => {
it('dispatches a sendFeedback action', () => {
const generatedAnswer = createGeneratedAnswer();
const feedback: GeneratedAnswerFeedback = 'harmful';
const feedback: GeneratedAnswerFeedback = {
readable: 'unknown',
correctTopic: 'unknown',
documented: 'yes',
hallucinationFree: 'no',
helpful: false,
details: 'some details',
};
generatedAnswer.sendFeedback(feedback);

expect(
generatedAnswerAnalyticsClient.logGeneratedAnswerFeedback
).toHaveBeenCalledWith(feedback);
expect(sendGeneratedAnswerFeedback).toHaveBeenCalledTimes(1);
});

it('dispatches a send detailed feedback action', () => {
const generatedAnswer = createGeneratedAnswer();
const details = 'details';
generatedAnswer.sendDetailedFeedback(details);
expect(
generatedAnswerAnalyticsClient.logGeneratedAnswerDetailedFeedback
).toHaveBeenCalledWith(details);
expect(sendGeneratedAnswerFeedback).toHaveBeenCalledTimes(1);
});

it('dispatches a log citation click action', () => {
const generatedAnswer = createGeneratedAnswer();
const citationId = 'citationId';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from '../../../features/generated-answer/generated-answer-actions';
import {
generatedAnswerAnalyticsClient,
GeneratedAnswerFeedbackV2,
GeneratedAnswerFeedback,
} from '../../../features/generated-answer/generated-answer-analytics-actions';
import {getGeneratedAnswerInitialState} from '../../../features/generated-answer/generated-answer-state';
import {queryReducer} from '../../../features/query/query-slice';
Expand Down Expand Up @@ -149,7 +149,7 @@ describe('knowledge-generated-answer', () => {
query: {q: 'this est une question', enableQuerySyntax: false},
});
const generatedAnswer = createGeneratedAnswer();
const feedback: GeneratedAnswerFeedbackV2 = {
const feedback: GeneratedAnswerFeedback = {
readable: 'unknown',
correctTopic: 'unknown',
documented: 'yes',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
sendGeneratedAnswerFeedback,
updateAnswerConfigurationId,
} from '../../../features/generated-answer/generated-answer-actions';
import {GeneratedAnswerFeedbackV2} from '../../../features/generated-answer/generated-answer-analytics-actions';
import {GeneratedAnswerFeedback} from '../../../features/generated-answer/generated-answer-analytics-actions';
import {queryReducer as query} from '../../../features/query/query-slice';
import {
GeneratedAnswerSection,
Expand All @@ -41,7 +41,7 @@ export interface AnswerApiGeneratedAnswer
* Sends feedback about why the generated answer was not relevant.
* @param feedback - The feedback that the end user wishes to send.
*/
sendFeedback(feedback: GeneratedAnswerFeedbackV2): void;
sendFeedback(feedback: GeneratedAnswerFeedback): void;
}

interface AnswerApiGeneratedAnswerProps extends GeneratedAnswerProps {}
Expand All @@ -50,7 +50,7 @@ export interface SearchAPIGeneratedAnswerAnalyticsClient
extends GeneratedAnswerAnalyticsClient {}

interface ParseEvaluationArgumentsParams {
feedback: GeneratedAnswerFeedbackV2;
feedback: GeneratedAnswerFeedback;
answerApiState: GeneratedAnswerStream;
query: string;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ exports[`generated answer analytics actions when analyticsMode is \`next\` shoul
]
`;

exports[`generated answer analytics actions when analyticsMode is \`next\` should log #logGeneratedAnswerDetailedFeedback with the right payload 1`] = `undefined`;

exports[`generated answer analytics actions when analyticsMode is \`next\` should log #logGeneratedAnswerExpand with the right payload 1`] = `
[
"Qna.AnswerAction",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ exports[`generated answer insight analytics actions when analyticsMode is \`next
]
`;

exports[`generated answer insight analytics actions when analyticsMode is \`next\` should log #logGeneratedAnswerDetailedFeedback with the right payload 1`] = `undefined`;

exports[`generated answer insight analytics actions when analyticsMode is \`next\` should log #logGeneratedAnswerExpand with the right payload 1`] = `
[
"Qna.AnswerAction",
Expand Down
Loading

0 comments on commit 45e9f2d

Please sign in to comment.