Skip to content

Commit

Permalink
feat(uaa-logging): Log UAA Parity data (#3629)
Browse files Browse the repository at this point in the history
* feat(uaa-logging): Create parity data objects for uaa testing

* feat(uaa-logging): Fix logging data

* feat(uaa-logging): Fix test

* feat(uaa-logging): Fix test

* feat(uaa-logging): Fix test

* feat(uaa-logging): Fix test

* feat(uaa-logging): Log api parity data async

* feat(uaa-logging): fix tests

* feat(uaa-logging): fix tests

* feat(uaa-logging): update to log feed items

* feat(uaa-logging): Fix logging

* feat(uaa-logging): Fix lint issue
  • Loading branch information
JChan106 authored Sep 24, 2024
1 parent f6abd4e commit 6cb5d8f
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 46 deletions.
78 changes: 49 additions & 29 deletions src/api/Feed.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ import type {
Tasks,
ThreadedComments as ThreadedCommentsType,
} from '../common/types/feed';
import { collapseFeedState } from '../elements/content-sidebar/activity-feed/activity-feed/activityFeedUtils';

const TASK_NEW_INITIAL_STATUS = TASK_NEW_NOT_STARTED;

Expand Down Expand Up @@ -550,6 +551,7 @@ class Feed extends Base {
shouldShowVersions?: boolean,
shouldUseUAA?: boolean,
} = {},
logAPIParity?: Function,
): void {
const { id, permissions = {} } = file;
const cachedItems = this.getCachedItems(id);
Expand All @@ -571,23 +573,16 @@ class Feed extends Base {
this.errorCallback = onError;

// Using the UAA File Activities endpoint replaces the need for these calls
const annotationsPromise =
!shouldUseUAA && shouldShowAnnotations
? this.fetchAnnotations(permissions, shouldShowReplies)
: Promise.resolve();
const annotationsPromise = shouldShowAnnotations
? this.fetchAnnotations(permissions, shouldShowReplies)
: Promise.resolve();
const commentsPromise = () => {
if (shouldUseUAA) {
return Promise.resolve();
}

return shouldShowReplies ? this.fetchThreadedComments(permissions) : this.fetchComments(permissions);
};
const tasksPromise = !shouldUseUAA && shouldShowTasks ? this.fetchTasksNew() : Promise.resolve();
const appActivityPromise =
!shouldUseUAA && shouldShowAppActivity ? this.fetchAppActivity(permissions) : Promise.resolve();
const versionsPromise = !shouldUseUAA && shouldShowVersions ? this.fetchVersions() : Promise.resolve();
const currentVersionPromise =
!shouldUseUAA && shouldShowVersions ? this.fetchCurrentVersion() : Promise.resolve();
const tasksPromise = shouldShowTasks ? this.fetchTasksNew() : Promise.resolve();
const appActivityPromise = shouldShowAppActivity ? this.fetchAppActivity(permissions) : Promise.resolve();
const versionsPromise = shouldShowVersions ? this.fetchVersions() : Promise.resolve();
const currentVersionPromise = shouldShowVersions ? this.fetchCurrentVersion() : Promise.resolve();

const annotationActivityType =
shouldShowAnnotations && permissions[PERMISSION_CAN_VIEW_ANNOTATIONS]
Expand Down Expand Up @@ -622,29 +617,54 @@ class Feed extends Base {
}
};

const v2Promises = [
versionsPromise,
currentVersionPromise,
commentsPromise(),
tasksPromise,
appActivityPromise,
annotationsPromise,
];

const fetchV2FeedItems = async promises => {
return Promise.all(promises).then(
([versions: ?FileVersions, currentVersion: ?BoxItemVersion, ...feedItems]) => {
const versionsWithCurrent = currentVersion
? this.versionsAPI.addCurrentVersion(currentVersion, versions, this.file)
: undefined;
return sortFeedItems(versionsWithCurrent, ...feedItems);
},
);
};

const compareV2AndUaaFeedItems = async (uaaFeedItems, uaaResponse) => {
fetchV2FeedItems(v2Promises).then(v2FeedItems => {
const transformedV2FeedItems = collapseFeedState(v2FeedItems);
const transformedUAAFeedItems = collapseFeedState(uaaFeedItems);

if (logAPIParity) {
logAPIParity({
uaaResponse,
uaaFeedItems: transformedUAAFeedItems,
v2FeedItems: transformedV2FeedItems,
});
}
});
};

if (shouldUseUAA) {
fileActivitiesPromise.then(response => {
if (!response) {
return;
}

const parsedFeedItems = getParsedFileActivitiesResponse(response);
handleFeedItems(parsedFeedItems);
const uaaFeedItems = getParsedFileActivitiesResponse(response);
compareV2AndUaaFeedItems(uaaFeedItems, response);
handleFeedItems(uaaFeedItems);
});
} else {
Promise.all([
versionsPromise,
currentVersionPromise,
commentsPromise(),
tasksPromise,
appActivityPromise,
annotationsPromise,
]).then(([versions: ?FileVersions, currentVersion: ?BoxItemVersion, ...feedItems]) => {
const versionsWithCurrent = currentVersion
? this.versionsAPI.addCurrentVersion(currentVersion, versions, this.file)
: undefined;
const sortedFeedItems = sortFeedItems(versionsWithCurrent, ...feedItems);
handleFeedItems(sortedFeedItems);
fetchV2FeedItems(v2Promises).then(v2FeedItems => {
handleFeedItems(v2FeedItems);
});
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/api/__tests__/Feed.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ describe('api/Feed', () => {
});
});

test('should use the file activities api if shouldUseUAA is true', done => {
test('should use the file activities api if shouldUseUAA is true and shadow call v2 APIs', done => {
feed.feedItems(file, false, successCb, errorCb, errorCb, {
shouldUseUAA: true,
shouldShowAnnotations: true,
Expand All @@ -627,8 +627,7 @@ describe('api/Feed', () => {
],
true,
);
expect(feed.fetchComments).not.toBeCalled();
expect(feed.fetchThreadedComments).not.toBeCalled();
expect(feed.fetchThreadedComments).toBeCalled();
done();
});
});
Expand Down
4 changes: 3 additions & 1 deletion src/common/types/logging.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ import {
METRIC_TYPE_PREVIEW,
METRIC_TYPE_ELEMENTS_LOAD_METRIC,
METRIC_TYPE_ELEMENTS_PERFORMANCE_METRIC,
METRIC_TYPE_UAA_PARITY_METRIC,
} from '../../constants';

type MetricType =
| typeof METRIC_TYPE_PREVIEW
| typeof METRIC_TYPE_ELEMENTS_LOAD_METRIC
| typeof METRIC_TYPE_ELEMENTS_PERFORMANCE_METRIC;
| typeof METRIC_TYPE_ELEMENTS_PERFORMANCE_METRIC
| typeof METRIC_TYPE_UAA_PARITY_METRIC;

type ElementsLoadMetricData = {
endMarkName: string,
Expand Down
1 change: 1 addition & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ export const ORIGIN_OPEN_WITH: 'open_with' = 'open_with';
export const METRIC_TYPE_PREVIEW: 'preview_metric' = 'preview_metric';
export const METRIC_TYPE_ELEMENTS_PERFORMANCE_METRIC: 'elements_performance_metric' = 'elements_performance_metric';
export const METRIC_TYPE_ELEMENTS_LOAD_METRIC: 'elements_load_metric' = 'elements_load_metric';
export const METRIC_TYPE_UAA_PARITY_METRIC = 'uaa_parity_metric';

/* ------------------ Error Keys ---------------------- */
export const IS_ERROR_DISPLAYED = 'isErrorDisplayed'; // used to determine if user will see some error state or message
Expand Down
22 changes: 17 additions & 5 deletions src/elements/common/logger/Logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import noop from 'lodash/noop';
import { v4 as uuidv4 } from 'uuid';
import { isMarkSupported } from '../../../utils/performance';
import { EVENT_DATA_READY, EVENT_JS_READY } from './constants';
import { METRIC_TYPE_PREVIEW, METRIC_TYPE_ELEMENTS_LOAD_METRIC } from '../../../constants';
import {
METRIC_TYPE_PREVIEW,
METRIC_TYPE_ELEMENTS_LOAD_METRIC,
METRIC_TYPE_UAA_PARITY_METRIC,
} from '../../../constants';
import type { ElementOrigin } from '../flowTypes';
import type { MetricType, ElementsLoadMetricData, LoggerProps } from '../../../common/types/logging';

Expand Down Expand Up @@ -123,10 +127,18 @@ class Logger extends React.Component<Props> {
*/
handlePreviewMetric = (data: Object) => {
const { onMetric } = this.props;
onMetric({
...data,
type: METRIC_TYPE_PREVIEW,
});

if (data.type === METRIC_TYPE_UAA_PARITY_METRIC) {
onMetric({
...data,
type: METRIC_TYPE_UAA_PARITY_METRIC,
});
} else {
onMetric({
...data,
type: METRIC_TYPE_PREVIEW,
});
}
};

/**
Expand Down
18 changes: 18 additions & 0 deletions src/elements/content-sidebar/ActivitySidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
ORIGIN_ACTIVITY_SIDEBAR,
SIDEBAR_VIEW_ACTIVITY,
TASK_COMPLETION_RULE_ALL,
METRIC_TYPE_UAA_PARITY_METRIC,
} from '../../constants';
import type {
TaskCompletionRule,
Expand Down Expand Up @@ -736,6 +737,7 @@ class ActivitySidebar extends React.PureComponent<Props, State> {
shouldShowVersions,
shouldUseUAA,
},
shouldUseUAA ? this.logAPIParity : undefined,
);
}

Expand Down Expand Up @@ -795,6 +797,22 @@ class ActivitySidebar extends React.PureComponent<Props, State> {
this.setState({ feedItems, activityFeedError: undefined });
};

/**
* Logs diff between UAA and v2 API data
*
* @param {{}[]} responseParity array of aggragated responses from UAA and v2
* @param {{}} parsedDataParity parsed data from UAA and v2
* @return {void}
*/
logAPIParity = (parityData: { uaaFeedItems: FeedItems, v2FeedItems: FeedItems }): void => {
const { logger } = this.props;

logger.onPreviewMetric({
parityData,
type: METRIC_TYPE_UAA_PARITY_METRIC,
});
};

/**
* Handles a failed feed item fetch
*
Expand Down
13 changes: 5 additions & 8 deletions src/elements/content-sidebar/__tests__/ActivitySidebar.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,7 @@ describe('elements/content-sidebar/ActivitySidebar', () => {
instance.errorCallback = jest.fn();
instance.fetchFeedItemsErrorCallback = jest.fn();
instance.fetchFeedItemsSuccessCallback = jest.fn();
instance.logAPIParity = jest.fn();

instance.fetchFeedItems();
expect(feedAPI.feedItems).toHaveBeenCalledWith(
Expand All @@ -746,6 +747,7 @@ describe('elements/content-sidebar/ActivitySidebar', () => {
shouldShowVersions: expectedVersions,
shouldUseUAA: expectedUseUAA,
},
expectedUseUAA ? instance.logAPIParity : undefined,
);
},
);
Expand Down Expand Up @@ -779,6 +781,7 @@ describe('elements/content-sidebar/ActivitySidebar', () => {
shouldShowVersions: true,
shouldUseUAA: false,
},
undefined,
);
});
});
Expand Down Expand Up @@ -1519,14 +1522,8 @@ describe('elements/content-sidebar/ActivitySidebar', () => {
`(
'should filter feed items of type "comment" or "annotation" based on status equal to $status',
({ status, expected }) => {
const {
annotationOpen,
annotationResolved,
commentOpen,
commentResolved,
taskItem,
versionItem,
} = cloneDeep(filterableActivityFeedItems);
const { annotationOpen, annotationResolved, commentOpen, commentResolved, taskItem, versionItem } =
cloneDeep(filterableActivityFeedItems);
const wrapper = getWrapper();
const instance = wrapper.instance();
instance.setState({
Expand Down

0 comments on commit 6cb5d8f

Please sign in to comment.