Skip to content

Commit

Permalink
fix(exclude): use unexclude actionCause when unexcluding (#3478)
Browse files Browse the repository at this point in the history
  • Loading branch information
louis-bompart authored Dec 12, 2023
1 parent 2365788 commit 8f7071e
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 22 deletions.
2 changes: 2 additions & 0 deletions .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@
"tsdoc",
"ttfb",
"uikit",
"unexclude",
"unexcluded",
"univer",
"Unregisters",
"unsub",
Expand Down
10 changes: 1 addition & 9 deletions packages/atomic/cypress/e2e/breadbox.cypress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,15 +312,7 @@ describe('Breadbox Test Suites', () => {
}

describe('when path-limit is lower than min', () => {
const pathLimit = 1;
beforeEach(() => {
setupBreadboxWithPathLimit({'path-limit': pathLimit});
});
CommonAssertions.assertConsoleError();
});

describe('when path-limit is higher than max', () => {
const pathLimit = 11;
const pathLimit = 0;
beforeEach(() => {
setupBreadboxWithPathLimit({'path-limit': pathLimit});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import {logCategoryFacetBreadcrumb} from '../facets/category-facet-set/category-
import {
LogFacetBreadcrumbActionCreatorPayload,
LogFacetDeselectActionCreatorPayload,
LogFacetExcludeActionCreatorPayload,
LogFacetUnexcludeActionCreatorPayload,
LogFacetSelectActionCreatorPayload,
} from '../facets/facet-set/facet-set-analytics-actions';
import {
Expand Down Expand Up @@ -54,6 +56,8 @@ export type {
LogFacetBreadcrumbActionCreatorPayload,
LogFacetUpdateSortActionCreatorPayload,
LogFacetDeselectActionCreatorPayload,
LogFacetUnexcludeActionCreatorPayload,
LogFacetExcludeActionCreatorPayload,
LogFacetSelectActionCreatorPayload,
SmartSnippetFeedback,
StaticFilterValueMetadata,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@ import {
import {
logFacetBreadcrumb,
logFacetClearAll,
logFacetUnexclude,
logFacetExclude,
logFacetDeselect,
logFacetSelect,
logFacetShowLess,
logFacetShowMore,
logFacetUpdateSort,
LogFacetBreadcrumbActionCreatorPayload,
LogFacetDeselectActionCreatorPayload,
LogFacetExcludeActionCreatorPayload,
LogFacetUnexcludeActionCreatorPayload,
LogFacetSelectActionCreatorPayload,
LogFacetUpdateSortActionCreatorPayload,
} from '../facets/facet-set/facet-set-analytics-actions';
Expand Down Expand Up @@ -88,6 +92,8 @@ import {CustomAction, LegacySearchAction} from './analytics-utils';
export type {
LogCategoryFacetBreadcrumbActionCreatorPayload,
LogFacetBreadcrumbActionCreatorPayload,
LogFacetExcludeActionCreatorPayload,
LogFacetUnexcludeActionCreatorPayload,
LogFacetDeselectActionCreatorPayload,
LogFacetSelectActionCreatorPayload,
LogFacetUpdateSortActionCreatorPayload,
Expand Down Expand Up @@ -198,6 +204,26 @@ export interface SearchAnalyticsActionCreators {
payload: LogFacetSelectActionCreatorPayload
): LegacySearchAction;

/**
* The event to log when a selected facet value is unexcluded.
*
* @param payload - The action creator payload.
* @returns A dispatchable action.
*/
logFacetUnexclude(
payload: LogFacetDeselectActionCreatorPayload
): LegacySearchAction;

/**
* The event to log when an idle facet value is excluded.
*
* @param payload - The action creator payload.
* @returns A dispatchable action.
*/
logFacetExclude(
payload: LogFacetSelectActionCreatorPayload
): LegacySearchAction;

/**
* The event to log when shrinking a facet to show fewer values.
*
Expand Down Expand Up @@ -486,6 +512,8 @@ export function loadSearchAnalyticsActions(
logCategoryFacetBreadcrumb,
logFacetBreadcrumb,
logFacetClearAll,
logFacetUnexclude,
logFacetExclude,
logFacetDeselect,
logFacetSelect,
logFacetShowLess,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,21 @@ export const logFacetSelect = (
return client.makeFacetSelect(metadata);
});

export interface LogFacetExcludeActionCreatorPayload {
/**
* The facet id.
*/
facetId: string;

/**
* The facet value that was excluded.
*/
facetValue: string;
}

//TODO: KIT-2859
export const logFacetExclude = (
payload: LogFacetSelectActionCreatorPayload
payload: LogFacetExcludeActionCreatorPayload
): LegacySearchAction =>
makeAnalyticsAction('analytics/facet/exclude', (client, state) => {
validatePayload(payload, {
Expand Down Expand Up @@ -164,6 +176,36 @@ export const logFacetDeselect = (
return client.makeFacetDeselect(metadata);
});

export interface LogFacetUnexcludeActionCreatorPayload {
/**
* The facet id.
*/
facetId: string;

/**
* The facet value that was unexcluded.
*/
facetValue: string;
}

//TODO: KIT-2859
export const logFacetUnexclude = (
payload: LogFacetUnexcludeActionCreatorPayload
): LegacySearchAction =>
makeAnalyticsAction('analytics/facet/unexclude', (client, state) => {
validatePayload(payload, {
facetId: facetIdDefinition,
facetValue: requiredNonEmptyString,
});
const stateForAnalytics = getStateNeededForFacetMetadata(state);
const metadata = buildFacetSelectionChangeMetadata(
payload,
stateForAnalytics
);

return client.makeFacetUnexclude(metadata);
});

export interface LogFacetBreadcrumbActionCreatorPayload {
/**
* The facet id associated with the breadcrumb.
Expand Down Expand Up @@ -240,6 +282,14 @@ export const facetDeselect = (id: string, value: string): SearchAction => {
};
};

export const facetUnexclude = (id: string, value: string): SearchAction => {
return {
actionCause: SearchPageEvents.facetUnexclude,
getEventExtraPayload: (state) =>
new SearchAnalyticsProvider(() => state).getFacetMetadata(id, value),
};
};

export const breadcrumbFacet = (id: string, value: string): SearchAction => {
return {
actionCause: SearchPageEvents.breadcrumbFacet,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ import {
facetDeselect,
facetExclude,
facetSelect,
facetUnexclude,
logFacetDeselect,
logFacetExclude,
logFacetSelect,
logFacetUnexclude,
} from './facet-set-analytics-actions';
import {FacetSelectionChangeMetadata} from './facet-set-analytics-actions-utils';
import {FacetValue} from './interfaces/response';
Expand Down Expand Up @@ -51,7 +53,7 @@ export const getLegacyAnalyticsActionForToggleFacetExclude = (
};

return isFacetValueExcluded(selection)
? logFacetDeselect(payload)
? logFacetUnexclude(payload)
: logFacetExclude(payload);
};

Expand All @@ -60,6 +62,6 @@ export const getAnalyticsActionForToggleFacetExclude = (
selection: FacetValue
): SearchAction => {
return isFacetValueExcluded(selection)
? facetDeselect(facetId, selection.value)
? facetUnexclude(facetId, selection.value)
: facetExclude(facetId, selection.value);
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ import {
facetDeselect,
facetExclude,
facetSelect,
facetUnexclude,
logFacetDeselect,
logFacetExclude,
logFacetSelect,
logFacetUnexclude,
} from '../../facet-set/facet-set-analytics-actions';
import {FacetSelectionChangeMetadata} from '../../facet-set/facet-set-analytics-actions-utils';
import {RangeFacetValue} from './interfaces/range-facet';
Expand Down Expand Up @@ -48,7 +50,7 @@ export const getLegacyAnalyticsActionForToggleRangeFacetExclude = (
const payload: FacetSelectionChangeMetadata = {facetId, facetValue};

return isRangeFacetValueExcluded(selection)
? logFacetDeselect(payload)
? logFacetUnexclude(payload)
: logFacetExclude(payload);
};

Expand All @@ -58,6 +60,6 @@ export const getAnalyticsActionForToggleRangeFacetExclude = (
): SearchAction => {
const facetValue = `${selection.start}..${selection.end}`;
return isRangeFacetValueExcluded(selection)
? facetDeselect(facetId, facetValue)
? facetUnexclude(facetId, facetValue)
: facetExclude(facetId, facetValue);
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
logFacetDeselect,
logFacetSelect,
logFacetExclude,
logFacetUnexclude,
} from '../facets/facet-set/facet-set-analytics-actions';
import {
logPageNumber,
Expand Down Expand Up @@ -103,7 +104,29 @@ function testFacetExcludeLogging(
action2: LegacySearchAction
) => void
) {
testFacetLogging('fExcluded', expectIdenticalActionType);
it('should log #logFacetDeselect when an fExcluded parameter with a single value is removed', () => {
expectIdenticalActionType(
logParametersChange({fExcluded: {author: ['Cervantes']}}, {}),
logFacetUnexclude({facetId: 'author', facetValue: 'Cervantes'})
);
});

it('should log #logFacetClearAll when an fExcluded parameter with multiple values is removed', () => {
expectIdenticalActionType(
logParametersChange({fExcluded: {author: ['Cervantes', 'Orwell']}}, {}),
logFacetClearAll('author')
);
});

it('should log #logFacetDeselect when an fExcluded parameter is modified & a value removed', () => {
expectIdenticalActionType(
logParametersChange(
{fExcluded: {author: ['Cervantes', 'Orwell']}},
{fExcluded: {author: ['Cervantes']}}
),
logFacetUnexclude({facetId: 'author', facetValue: 'Orwell'})
);
});

it('should log #logFacetSelect when an fExcluded parameter is added', () => {
expectIdenticalActionType(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
logFacetDeselect,
logFacetSelect,
logFacetExclude,
logFacetUnexclude,
} from '../../features/facets/facet-set/facet-set-analytics-actions';
import {logSearchboxSubmit} from '../../features/query/query-analytics-actions';
import {SearchParameters} from '../../features/search-parameters/search-parameter-actions';
Expand Down Expand Up @@ -112,9 +113,20 @@ function logFacetAnalyticsAction(
const removedIds = previousIds.filter((id) => !newIds.includes(id));
if (removedIds.length) {
const facetId = removedIds[0];
return previousFacets[facetId].length > 1
? logFacetClearAll(facetId)
: logFacetDeselect({facetId, facetValue: previousFacets[facetId][0]});
switch (true) {
case previousFacets[facetId].length > 1:
return logFacetClearAll(facetId);
case excluded:
return logFacetUnexclude({
facetId,
facetValue: previousFacets[facetId][0],
});
default:
return logFacetDeselect({
facetId,
facetValue: previousFacets[facetId][0],
});
}
}

const addedIds = newIds.filter((id) => !previousIds.includes(id));
Expand Down Expand Up @@ -164,10 +176,15 @@ function logFacetAnalyticsAction(
);

if (removedValues.length) {
return logFacetDeselect({
facetId: facetIdWithDifferentValues,
facetValue: removedValues[0],
});
return excluded
? logFacetUnexclude({
facetId: facetIdWithDifferentValues,
facetValue: removedValues[0],
})
: logFacetDeselect({
facetId: facetIdWithDifferentValues,
facetValue: removedValues[0],
});
}

return logInterfaceChange();
Expand Down

0 comments on commit 8f7071e

Please sign in to comment.