Skip to content
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

feat(headless): expose logRecommendationOpen action #902

Merged
merged 11 commits into from
Jun 17, 2021
Merged
43,454 changes: 21,728 additions & 21,726 deletions packages/headless/package-lock.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/headless/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"@reduxjs/toolkit": "^1.5.0",
Copy link
Contributor Author

@samisayegh samisayegh Jun 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what happened to the package-lock. If you try installing the latest analytics package, is the diff large?
I am using node 16.2.0, npm 7.13.0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's definitely something wonky with the package locks, I think they need to be updated more often

"@types/pino": "^6.3.4",
"@types/redux-mock-store": "^1.0.2",
"coveo.analytics": "^2.18.19",
"coveo.analytics": "^2.18.27",
"cross-fetch": "^3.0.6",
"dayjs": "^1.9.6",
"exponential-backoff": "^3.1.0",
Expand Down
116 changes: 82 additions & 34 deletions packages/headless/src/features/analytics/analytics-utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,47 +1,95 @@
import {createMockState} from '../../test';
import {buildMockResult} from '../../test';
import {partialDocumentInformation} from './analytics-utils';
import {createMockRecommendationState} from '../../test/mock-recommendation-state';
import {
partialDocumentInformation,
partialRecommendationInformation,
} from './analytics-utils';

describe('analytics-utils', () => {
it('should extract documentation information with a single author', () => {
const result = buildMockResult();
result.raw['author'] = 'john';
describe('#partialDocumentInformation', () => {
it('should extract documentation information with a single author', () => {
const result = buildMockResult();
result.raw['author'] = 'john';

expect(
partialDocumentInformation(result, createMockState()).documentAuthor
).toBe('john');
});
expect(
partialDocumentInformation(result, createMockState()).documentAuthor
).toBe('john');
});

it('should extract documentation information with multiple author', () => {
const result = buildMockResult();
result.raw['author'] = ['john', 'doe'];
it('should extract documentation information with multiple author', () => {
const result = buildMockResult();
result.raw['author'] = ['john', 'doe'];

expect(
partialDocumentInformation(result, createMockState()).documentAuthor
).toBe('john;doe');
});
expect(
partialDocumentInformation(result, createMockState()).documentAuthor
).toBe('john;doe');
});

it('should extract document information when there is no author', () => {
const result = buildMockResult();
delete result.raw['author'];
expect(
partialDocumentInformation(result, createMockState()).documentAuthor
).toBe('unknown');
});
it('should extract document information when there is no author', () => {
const result = buildMockResult();
delete result.raw['author'];
expect(
partialDocumentInformation(result, createMockState()).documentAuthor
).toBe('unknown');
});

it('should extract sourceName information from source field', () => {
const result = buildMockResult();
result.raw.source = 'mysource';
expect(
partialDocumentInformation(result, createMockState()).sourceName
).toBe('mysource');
});

it('should extract sourceName information when there is no source field', () => {
const result = buildMockResult();
delete result.raw['source'];
expect(
partialDocumentInformation(result, createMockState()).sourceName
).toBe('unknown');
});

it('when the result is not found in state, the documentPosition is 0', () => {
const result = buildMockResult({uniqueId: '1'});
const state = createMockState();

it('should extract sourceName information from source field', () => {
const result = buildMockResult();
result.raw.source = 'mysource';
expect(
partialDocumentInformation(result, createMockState()).sourceName
).toBe('mysource');
const {documentPosition} = partialDocumentInformation(result, state);
expect(documentPosition).toBe(0);
});

it('when the result is found in state, the documentPosition is the index + 1', () => {
const result = buildMockResult({uniqueId: '1'});
const state = createMockState();
state.search.response.results = [result];

const {documentPosition} = partialDocumentInformation(result, state);
expect(documentPosition).toBe(1);
});
});

it('should extract sourceName information when there is no source field', () => {
const result = buildMockResult();
delete result.raw['source'];
expect(
partialDocumentInformation(result, createMockState()).sourceName
).toBe('unknown');
describe('#partialRecommendationInformation', () => {
it('when the recommendation is not found in state, the documentPosition is 0', () => {
const recommendation = buildMockResult({uniqueId: '1'});
const state = createMockRecommendationState();

const {documentPosition} = partialRecommendationInformation(
recommendation,
state
);
expect(documentPosition).toBe(0);
});

it('when the recommendation is found in state, the documentPosition is the index + 1', () => {
const recommendation = buildMockResult({uniqueId: '1'});
const state = createMockRecommendationState();
state.recommendation.recommendations = [recommendation];

const {documentPosition} = partialRecommendationInformation(
recommendation,
state
);
expect(documentPosition).toBe(1);
});
});
});
24 changes: 23 additions & 1 deletion packages/headless/src/features/analytics/analytics-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {SearchEventResponse} from 'coveo.analytics/dist/definitions/events';
import {AsyncThunkAction, createAsyncThunk} from '@reduxjs/toolkit';
import {requiredNonEmptyString} from '../../utils/validate-payload';
import {ThunkExtraArguments} from '../../app/thunk-extra-arguments';
import {PipelineSection} from '../../state/state-sections';
import {RecommendationAppState} from '../../state/recommendation-app-state';

export enum AnalyticsType {
Search,
Expand Down Expand Up @@ -117,6 +119,26 @@ export const partialDocumentInformation = (
({uniqueId}) => result.uniqueId === uniqueId
) || 0;

return buildPartialDocumentInformation(result, resultIndex, state);
};

export const partialRecommendationInformation = (
result: Result,
state: Partial<RecommendationAppState>
): PartialDocumentInformation => {
const resultIndex =
state.recommendation?.recommendations.findIndex(
({uniqueId}) => result.uniqueId === uniqueId
) || 0;
Copy link
Contributor Author

@samisayegh samisayegh Jun 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question on this ternary, similar to the one on line 120:

  • If the recommendation state property does not exist, the index is 0, and the document position is 1 (line 149).
  • If the recommendation state exists, but the result is not found in the array, the index is -1 and document position is 0.

I feel it's weird to send two different values for the same condition of "result not found".
However, I'm not sure on what the correct value for documentPosition should be, 0 or 1?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Those documents not being found * should * not happen, since it's not really logical to be able to click on a document that does not exist. But.. 🤷

  • It's there as a fallback/artefact of the type system being unable to assure us that a given value will really be in the array, (which is normal, I guess 🤔 ) Another approach would be to just throw an error and not log anything. Which should happen upstream anyway, not in this function, which is pretty deep in the codebase.

  • For now I would suggest that index should be 0, and documentPosition -1 when it's not found. Which most likely means it's going to be rejected by UA (I would assume...).

In any case, I guess just keep in mind that no matter what, if we are unable to find the given result unique ID in the list of results, there's not much we can do to make the UA payload "correct", it's most likely going to fail in one manner or the other. And it being 0, or -1, is a small detail versus the fact that there's some code upstream that is completely wrong when calling this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I don't feel this is important enough to solve right now. It's definitely niche, but I still think it would be good to addressed, whether through a value, or some kind of feedback to the devs. I've created a jira to revisit.


return buildPartialDocumentInformation(result, resultIndex, state);
};

function buildPartialDocumentInformation(
result: Result,
resultIndex: number,
state: Partial<PipelineSection>
): PartialDocumentInformation {
const collection = result.raw.collection;
const collectionName =
typeof collection === 'string' ? collection : 'default';
Expand All @@ -133,7 +155,7 @@ export const partialDocumentInformation = (
sourceName: getSourceName(result),
queryPipeline: state.pipeline || getPipelineInitialState(),
};
};
}

export const documentIdentifier = (result: Result): DocumentIdentifier => {
return {
Expand Down
8 changes: 6 additions & 2 deletions packages/headless/src/features/analytics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,16 @@ export namespace QuerySuggestAnalyticsActions {
export const logQuerySuggestionClick = logQuerySuggestionClickAlias;
}

import {logRecommendationUpdate as logRecommendationUpdateAlias} from '../recommendation/recommendation-analytics-actions';
import {
logRecommendationUpdate as logRecommendationUpdateAlias,
logRecommendationOpen as logRecommendationOpenAlias,
} from '../recommendation/recommendation-analytics-actions';
/**
* @deprecated - This namespace will be removed. Please use `loadSearchAnalyticsActions` instead.
* @deprecated - This namespace will be removed. Please use `loadClickAnalyticsActions` from "@coveo/headless/recommendation" instead.
*/
export namespace RecommendationAnalyticsActions {
export const logRecommendationUpdate = logRecommendationUpdateAlias;
export const logRecommendationOpen = logRecommendationOpenAlias;
}

import {logTriggerRedirect as logTriggerRedirectAlias} from '../redirection/redirection-analytics-actions';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import {AnalyticsType, makeAnalyticsAction} from '../analytics/analytics-utils';
import {Result} from '../../api/search/search/result';
import {
AnalyticsType,
documentIdentifier,
makeAnalyticsAction,
partialRecommendationInformation,
validateResultPayload,
} from '../analytics/analytics-utils';

/**
* Logs a search event with an `actionCause` value of `recommendationInterfaceLoad`.
Expand All @@ -8,3 +15,16 @@ export const logRecommendationUpdate = makeAnalyticsAction(
AnalyticsType.Search,
(client) => client.logRecommendationInterfaceLoad()
);

export const logRecommendationOpen = (result: Result) =>
makeAnalyticsAction(
'analytics/recommendation/open',
AnalyticsType.Click,
(client, state) => {
validateResultPayload(result);
return client.logRecommendationOpen(
partialRecommendationInformation(result, state),
documentIdentifier(result)
);
}
)();
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import {AsyncThunkAction} from '@reduxjs/toolkit';
import {StateNeededByAnalyticsProvider} from '../../api/analytics/analytics';
import {Result} from '../../api/search/search/result';
import {Engine} from '../../app/headless-engine';
import {
AnalyticsType,
AsyncThunkAnalyticsOptions,
} from '../analytics/analytics-utils';
import {logRecommendationOpen} from './recommendation-analytics-actions';

/**
* The click analytics action creators.
*/
export interface ClickAnalyticsActionCreators {
/**
* The event to log when a recommendation is selected.
*
* @param result - The selected recommendation.
* @returns A dispatchable action.
*/
logRecommendationOpen(
recommendation: Result
): AsyncThunkAction<
{
analyticsType: AnalyticsType.Click;
},
void,
AsyncThunkAnalyticsOptions<StateNeededByAnalyticsProvider>
>;
}

/**
* Returns possible click analytics action creators.
*
* @param engine - The headless engine.
* @returns An object holding the action creators.
*/
export function loadClickAnalyticsActions(
engine: Engine<object>
): ClickAnalyticsActionCreators {
engine.addReducers({});

return {
logRecommendationOpen,
};
}
3 changes: 3 additions & 0 deletions packages/headless/src/recommendation.index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export * from './features/pipeline/pipeline-actions-loader';
export * from './features/search-hub/search-hub-actions-loader';
export * from './features/debug/debug-actions-loader';
export * from './features/recommendation/recommendation-actions-loader';
export * from './features/recommendation/recommendation-click-analytics-actions-loader';

// Controllers
export {
Expand All @@ -40,3 +41,5 @@ export {
ContextPayload,
buildContext,
} from './controllers/context/headless-context';

export {Result} from './api/search/search/result';
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import {Component, ContextType} from 'react';
import {ResultLink} from '../result-list/result-link';
import {
buildRecommendationList,
RecommendationList as HeadlessRecommendationList,
RecommendationListOptions,
RecommendationListState,
Result,
Unsubscribe,
} from '@coveo/headless';
import {loadClickAnalyticsActions} from '@coveo/headless/recommendation';
import {AppContext} from '../../context/engine';
import {filterProtocol} from '../../utils/filter-protocol';

export class RecommendationList extends Component<
RecommendationListOptions,
Expand Down Expand Up @@ -37,6 +39,17 @@ export class RecommendationList extends Component<
this.setState(this.controller.state);
}

private logClick(recommendation: Result) {
const engine = this.context.recommendationEngine;

if (!engine) {
return;
}

const {logRecommendationOpen} = loadClickAnalyticsActions(engine);
engine.dispatch(logRecommendationOpen(recommendation));
}

render() {
if (!this.state) {
return null;
Expand Down Expand Up @@ -65,9 +78,15 @@ export class RecommendationList extends Component<
<article>
<h2>
{/* Make sure to log analytics when the result link is clicked. */}
<ResultLink result={recommendation}>
<a
href={filterProtocol(recommendation.clickUri)} // Filters out dangerous URIs that can create XSS attacks such as `javascript:`.
onClick={() => this.logClick(recommendation)}
onContextMenu={() => this.logClick(recommendation)}
onMouseDown={() => this.logClick(recommendation)}
onMouseUp={() => this.logClick(recommendation)}
>
{recommendation.title}
</ResultLink>
</a>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These samples will eventually leverage a buildInteractiveRecommendation controller.

</h2>
<p>{recommendation.excerpt}</p>
</article>
Expand Down
Loading