Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

SFINT-3372 Fix incoherent event for good #92

Merged
merged 3 commits into from
Sep 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 1 addition & 29 deletions src/components/UserActions/ClickedDocumentList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,12 @@ import {
QueryUtils,
l,
get,
ResultLink,
} from 'coveo-search-ui';
import { UserProfileModel, UserAction } from '../../models/UserProfileModel';
import { ExpandableList } from './ExpandableList';
import { UserActionType } from '../../rest/UserProfilingEndpoint';
import { duplicate } from '../../utils/icons';
import './Strings';
import { UserActionEvents } from './Events';

/**
* Initialization options of the **ClickedDocumentList** class.
Expand Down Expand Up @@ -138,11 +136,7 @@ export class ClickedDocumentList extends Component {
currentLayout: 'list',
responsiveComponents: this.searchInterface.responsiveComponents,
})).then((element) => {
Initialization.automaticallyCreateComponentsInsideResult(element, result, {
ResultLink: {
onClick: this.openDocument(result),
},
});
Initialization.automaticallyCreateComponentsInsideResult(element, result);
return element;
});
},
Expand All @@ -152,28 +146,6 @@ export class ClickedDocumentList extends Component {
showLessMessage: l(`${ClickedDocumentList.ID}_less`),
});
}

private openDocument(result: IQueryResult) {
return function (this: ResultLink) {
this.usageAnalytics.logCustomEvent(
UserActionEvents.documentClick,
{
documentUrl: result.clickUri,
documentTitle: result.title,
sourceName: QueryUtils.getSource(result),
author: QueryUtils.getAuthor(result),
},
this.element,
result
);

if (this.options.alwaysOpenInNewWindow) {
this.openLinkInNewWindow(false);
} else {
this.openLink(false);
}
};
}
}

Initialization.registerAutoCreateComponent(ClickedDocumentList);
4 changes: 2 additions & 2 deletions src/components/UserActions/Events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { IAnalyticsActionCause } from 'coveo-search-ui';
export const USER_ACTION_EVENT_TYPE = 'User Actions';

export class UserActionEvents {
public static readonly documentClick: IAnalyticsActionCause = Object.freeze({
name: 'userActionDocumentClick',
public static readonly load: IAnalyticsActionCause = Object.freeze({
name: 'userActionLoad',
type: USER_ACTION_EVENT_TYPE,
});
public static readonly submit: IAnalyticsActionCause = Object.freeze({
Expand Down
52 changes: 47 additions & 5 deletions src/models/UserProfileModel.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,17 @@
import { Model, QueryBuilder, IQueryResult, AccessToken, Assert, ISearchEndpoint, SearchEndpoint } from 'coveo-search-ui';
import {
Model,
QueryBuilder,
IQueryResult,
AccessToken,
Assert,
ISearchEndpoint,
SearchEndpoint,
Component,
SearchInterface,
IQuery,
IQueryResults,
} from 'coveo-search-ui';
import { UserActionEvents } from '../components/UserActions/Events';
import { UserProfilingEndpoint, IActionHistory, UserActionType } from '../rest/UserProfilingEndpoint';

/**
Expand Down Expand Up @@ -135,18 +148,22 @@ export class UserProfileModel extends Model {
return Promise.resolve({});
}

const query = new QueryBuilder();
query.advancedExpression.addFieldExpression(
const builder = new QueryBuilder();
builder.advancedExpression.addFieldExpression(
'@urihash',
'==',
urihashes.filter((x) => x)
);

// Ensure we fetch the good amount of document.
query.numberOfResults = urihashes.length;
builder.numberOfResults = urihashes.length;

// Here we directly use the Search Endpoint to query without side effects.
const searchRequest = await this.options.searchEndpoint.search(query.build());
const query = builder.build();
const searchRequest = await this.options.searchEndpoint.search(query);

// Here we directly send the event using the Analytics Endpoint to prevent any unwanted side effects.
this.sendUserActionLoad(query, searchRequest);

const documentsDict = searchRequest.results.reduce((acc, result) => ({ ...acc, [result.raw.urihash]: result }), {});

Expand All @@ -165,6 +182,7 @@ export class UserProfileModel extends Model {
try {
documents = await this.fetchDocuments(urihashes);
} catch (error) {
console.log(error);
this.logger.error(UserProfileModel.ERROR_MESSAGE.FETCH_CLICKED_DOCUMENT_FAIL, error);
}

Expand All @@ -190,6 +208,30 @@ export class UserProfileModel extends Model {
private isSearch(action: IActionHistory) {
return action.name === UserActionType.Search;
}

private sendUserActionLoad(query: IQuery, result: IQueryResults) {
const uaClient = (Component.get(this.element, SearchInterface, true) as SearchInterface)?.usageAnalytics;
uaClient?.logSearchEvent(UserActionEvents.load, {});
uaClient?.endpoint.sendSearchEvents([
{
...uaClient.getPendingSearchEvent().templateSearchEvent,
...{
queryPipeline: result.pipeline,
splitTestRunName: result.splitTestRun,
splitTestRunVersion: result.splitTestRun ? result.pipeline : undefined,
queryText: query.q ?? '',
advancedQuery: query.aq ?? '',
didYouMean: query.enableDidYouMean,
numberOfResults: result.totalCount,
responseTime: result.duration,
pageNumber: query.firstResult / query.numberOfResults,
resultsPerPage: query.numberOfResults,
searchQueryUid: result.searchUid,
contextual: false,
},
},
]);
}
}

/**
Expand Down
92 changes: 1 addition & 91 deletions tests/components/UserActions/ClickedDocumentList.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ import { createSandbox, SinonSandbox, SinonStub } from 'sinon';
import { Mock, Fake } from 'coveo-search-ui-tests';
import { ClickedDocumentList } from '../../../src/components/UserActions/ClickedDocumentList';
import { UserProfileModel, UserAction } from '../../../src/models/UserProfileModel';
import { Logger, Initialization, ResultLink, NoopAnalyticsClient, QueryUtils } from 'coveo-search-ui';
import { Logger, Initialization } from 'coveo-search-ui';
import { generate, fakeUserProfileModel, waitForPromiseCompletion } from '../../utils';
import { UserActionType } from '../../../src/rest/UserProfilingEndpoint';
import { UserActionEvents } from '../../../src/components/UserActions/Events';

describe('ClickedDocumentList', () => {
const BUILD_ACTION = (hash: string, i: number) => {
Expand Down Expand Up @@ -271,95 +270,6 @@ describe('ClickedDocumentList', () => {
expect(mock.cmp.disabled).toBe(true);
});

it('Should open the link in a new tab when alwaysOpenInNewWindow is true', async () => {
let openLinkStub = sandbox.stub(ResultLink.prototype, 'openLink');
let openInANewTabStub = sandbox.stub(ResultLink.prototype, 'openLinkInNewWindow');
const mock = Mock.advancedComponentSetup<ClickedDocumentList>(
ClickedDocumentList,
new Mock.AdvancedComponentSetupOptions(null, { userId: 'testuserId' }, (env) => {
fakeUserProfileModel(env.root, sandbox).getActions.callsFake(() => Promise.resolve(TEST_CLICKS));
env.componentOptionsModel;
env.usageAnalytics = new NoopAnalyticsClient();
env.withResult();
env.searchInterface.options.originalOptionsObject['ResultLink'] = {
...env.searchInterface.options.originalOptionsObject['ResultLink'],
};
env.searchInterface.options.originalOptionsObject['ResultLink']['alwaysOpenInNewWindow'] = true;
return env;
})
);

await waitForPromiseCompletion();

mock.env.element.querySelector<HTMLOListElement>('.coveo-list .CoveoResultLink').click();

expect(openLinkStub.calledWith(false)).toBe(false);
expect(openInANewTabStub.calledWith(false)).toBe(true);
});

it('Should not open the link in a new tab when alwaysOpenInNewWindow is false', async () => {
let openLinkStub = sandbox.stub(ResultLink.prototype, 'openLink');
let openInANewTabStub = sandbox.stub(ResultLink.prototype, 'openLinkInNewWindow');
const mock = Mock.advancedComponentSetup<ClickedDocumentList>(
ClickedDocumentList,
new Mock.AdvancedComponentSetupOptions(null, { userId: 'testuserId' }, (env) => {
fakeUserProfileModel(env.root, sandbox).getActions.callsFake(() => Promise.resolve(TEST_CLICKS));
env.componentOptionsModel;
env.usageAnalytics = new NoopAnalyticsClient();
env.withResult();
env.searchInterface.options.originalOptionsObject['ResultLink'] = {
...env.searchInterface.options.originalOptionsObject['ResultLink'],
};
env.searchInterface.options.originalOptionsObject['ResultLink']['alwaysOpenInNewWindow'] = false;
return env;
})
);

await waitForPromiseCompletion();

mock.env.element.querySelector<HTMLOListElement>('.coveo-list .CoveoResultLink').click();

expect(openLinkStub.calledWith(false)).toBe(true);
expect(openInANewTabStub.calledWith(false)).toBe(false);
});

it('should log a userActionDocumentClick event when a result link in the template is clicked', async () => {
let openLinkStub = sandbox.stub(ResultLink.prototype, 'openLink');
let logSearchStub: SinonStub;
let logCustomStub: SinonStub;
const mock = Mock.advancedComponentSetup<ClickedDocumentList>(
ClickedDocumentList,
new Mock.AdvancedComponentSetupOptions(null, { userId: 'testuserId' }, (env) => {
fakeUserProfileModel(env.root, sandbox).getActions.callsFake(() => Promise.resolve(TEST_CLICKS));
env.usageAnalytics = new NoopAnalyticsClient();
env.withResult();
logSearchStub = sandbox.stub(env.usageAnalytics, 'logSearchEvent');
logCustomStub = sandbox.stub(env.usageAnalytics, 'logCustomEvent');
return env;
})
);

await waitForPromiseCompletion();

mock.env.element.querySelector<HTMLOListElement>('.coveo-list .CoveoResultLink').click();

expect(openLinkStub.calledWith(false)).toBe(true);
expect(logSearchStub.called).toBeFalse;
expect(
logCustomStub.calledWith(
UserActionEvents.documentClick,
{
documentUrl: mock.env.result.clickUri,
documentTitle: mock.env.result.title,
sourceName: QueryUtils.getSource(mock.env.result),
author: QueryUtils.getAuthor(mock.env.result),
},
null,
mock.env.result
)
).toBeTrue;
});

describe('template', () => {
it('should use the given template in options', async () => {
sandbox.stub(Initialization, 'automaticallyCreateComponentsInsideResult');
Expand Down
2 changes: 1 addition & 1 deletion tests/components/UserActions/UserActions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ describe('UserActions', () => {
it('should fetch all user actions', async function () {
await mock.cmp.show();

expect(modelMock.getActions.calledWithExactly(someUserId)).toBe(true);
expect(modelMock.getActions.calledWith(someUserId)).toBe(true);
});

it('should trigger a userActionsShow event', async () => {
Expand Down
52 changes: 50 additions & 2 deletions tests/models/UserProfilingModel.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { createSandbox, SinonFakeXMLHttpRequest } from 'sinon';
import { createSandbox, SinonFakeXMLHttpRequest, SinonStub } from 'sinon';
import { Fake } from 'coveo-search-ui-tests';
import { UserProfileModel, UserAction } from '../../src/models/UserProfileModel';
import { UserActionType } from '../../src/rest/UserProfilingEndpoint';
import { buildActionHistoryResponse, buildAccessToken } from '../utils';
import { Logger, SearchEndpoint, QueryBuilder } from 'coveo-search-ui';
import { Logger, SearchEndpoint, QueryBuilder, Component } from 'coveo-search-ui';

describe('UserProfilingModel', () => {
const TEST_URI_HASH = 'testUriHash';
Expand Down Expand Up @@ -131,6 +131,25 @@ describe('UserProfilingModel', () => {
});

describe('getActions', () => {
let logSearchEventStub: SinonStub;
let sendSearchEventsStub: SinonStub;

beforeEach(() => {
sandbox.stub(Component, 'get').callsFake(() => {
logSearchEventStub = sandbox.stub();
sendSearchEventsStub = sandbox.stub();
return {
usageAnalytics: {
logSearchEvent: logSearchEventStub,
getPendingSearchEvent: sandbox.stub().returns({}),
endpoint: {
sendSearchEvents: sendSearchEventsStub,
},
},
} as any;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ain't that at least a Partial something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a test mock polyfill

});
});

it('should attach available documents on click actions', async () => {
const documentResults = Fake.createFakeResults(1);
documentResults.results[0].raw.urihash = TEST_URI_HASH;
Expand All @@ -153,9 +172,11 @@ describe('UserProfilingModel', () => {
);

const actions = await actionsPromise;
console.log(actions);
const actionsWithDocument = actions.filter((action) => action.document);
const uniqueUriHashes = FAKE_ACTIONS_WITH_URI_HASH.map((x) => x.value.uri_hash).filter((x, i, l) => l.indexOf(x) === i);

expect(FAKE_ACTIONS_WITH_URI_HASH.length).toEqual(actions.length);
expect(((endpoint.search.args[0][0] as unknown) as QueryBuilder).numberOfResults).toEqual(uniqueUriHashes.length);
expect(actionsWithDocument.length).toBeGreaterThanOrEqual(documentResults.results.length);
actionsWithDocument.forEach((action, i) => {
Expand All @@ -166,6 +187,33 @@ describe('UserProfilingModel', () => {
});
});

it('should send a userActionLoad event when document are fetched', async () => {
const documentResults = Fake.createFakeResults(1);
documentResults.results[0].raw.urihash = TEST_URI_HASH;

const endpoint = sandbox.createStubInstance(SearchEndpoint);
endpoint.search.callsFake(() => Promise.resolve(documentResults));

const model = new UserProfileModel(document.createElement('div'), {
organizationId: TEST_ORGANIZATION,
restUri: TEST_REST_URI,
accessToken: TEST_TOKEN,
searchEndpoint: endpoint,
});

const actionsPromise = model.getActions(TEST_USER);
requests[requests.length - 1].respond(
200,
{ 'Content-Type': 'application/json' },
JSON.stringify(buildActionHistoryResponse(FAKE_ACTIONS_WITH_URI_HASH))
);

await actionsPromise;

expect(logSearchEventStub.called).toBe(true);
expect(sendSearchEventsStub.called).toBe(true);
});

it('should attach no documents on click actions when no document are available to the searching user', async () => {
const endpoint = sandbox.createStubInstance(SearchEndpoint);
endpoint.search.callsFake(() => Promise.resolve(Fake.createFakeResults(0)));
Expand Down