Skip to content

fix(activity-feed): request enhanced_comment_timespan for timespan comments #4873

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.

Sign up for GitHub

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

Draft
kduncanhsu wants to merge 5 commits into
base: master
Choose a base branch
from
+139 −9
Draft

fix(activity-feed): request enhanced_comment_timespan for timespan comments #4873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
14ffabd
feat(activity-feed): request enhanced_comment_v2 for timespan comments
kduncanhsu Sep 28, 2026
180c65f
fix(activity-feed): rename enhanced_comment_v2 to enhanced_comment_ti…
kduncanhsu Oct 2, 2026
783342a
fix(activity-feed): request timespan comments only for audio player v2
kduncanhsu Oct 6, 2026
c070baa
fix(activity-feed): rename the timespan comments feed option
kduncanhsu Oct 6, 2026
f7c35de
fix(activity-feed): request timespan comments only for audio files
kduncanhsu Oct 7, 2026
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
32 changes: 23 additions & 9 deletions src/api/Feed.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
FILE_ACTIVITY_TYPE_COMMENT,
FILE_ACTIVITY_TYPE_ENHANCED_ANNOTATION,
FILE_ACTIVITY_TYPE_ENHANCED_COMMENT,
FILE_ACTIVITY_TYPE_ENHANCED_COMMENT_TIMESPAN,
FILE_ACTIVITY_TYPE_TASK,
FILE_ACTIVITY_TYPE_VERSION,
HTTP_STATUS_CODE_CONFLICT,
Expand Down Expand Up @@ -173,11 +174,16 @@ export const getParsedFileActivitiesResponse = (
return taskItem;
}
case FILE_ACTIVITY_TYPE_COMMENT:
case FILE_ACTIVITY_TYPE_ENHANCED_COMMENT: {
const rawCommentItem =
item.activity_type === FILE_ACTIVITY_TYPE_ENHANCED_COMMENT
? source[FILE_ACTIVITY_TYPE_ENHANCED_COMMENT]
: source[FILE_ACTIVITY_TYPE_COMMENT];
case FILE_ACTIVITY_TYPE_ENHANCED_COMMENT:
case FILE_ACTIVITY_TYPE_ENHANCED_COMMENT_TIMESPAN: {
let rawCommentItem;
if (item.activity_type === FILE_ACTIVITY_TYPE_ENHANCED_COMMENT_TIMESPAN) {
rawCommentItem = source[FILE_ACTIVITY_TYPE_ENHANCED_COMMENT_TIMESPAN];
} else if (item.activity_type === FILE_ACTIVITY_TYPE_ENHANCED_COMMENT) {
rawCommentItem = source[FILE_ACTIVITY_TYPE_ENHANCED_COMMENT];
} else {
rawCommentItem = source[FILE_ACTIVITY_TYPE_COMMENT];
}
if (!rawCommentItem) {
return null;
}
Expand All @@ -190,7 +196,7 @@ export const getParsedFileActivitiesResponse = (
}

commentItem.tagged_message = commentItem.tagged_message || commentItem.message || '';
// enhanced_comment is a wire-only variant; downstream consumers see the legacy type
// enhanced_comment and enhanced_comment_timespan are wire-only; downstream consumers see the legacy type
commentItem.type = FEED_ITEM_TYPE_COMMENT;

return commentItem;
Expand Down Expand Up @@ -582,6 +588,7 @@ class Feed extends Base {
shouldShowTasks = true,
shouldShowVersions = true,
shouldUseEnhancedActivities = false,
shouldUseEnhancedTimespanComments = false,
shouldUseUAA = false,
shouldEnableRichText = false,
}: {
Expand All @@ -591,6 +598,7 @@ class Feed extends Base {
shouldShowTasks?: boolean,
shouldShowVersions?: boolean,
shouldUseEnhancedActivities?: boolean,
shouldUseEnhancedTimespanComments?: boolean,
shouldUseUAA?: boolean,
shouldEnableRichText?: boolean,
} = {},
Expand Down Expand Up @@ -641,9 +649,15 @@ class Feed extends Base {
const appActivityActivityType = shouldShowAppActivity ? [FILE_ACTIVITY_TYPE_APP_ACTIVITY] : [];
const taskActivityType = shouldShowTasks ? [FILE_ACTIVITY_TYPE_TASK] : [];
const versionsActivityType = shouldShowVersions ? [FILE_ACTIVITY_TYPE_VERSION] : [];
const commentActivityType = permissions[PERMISSION_CAN_COMMENT]
? [shouldUseEnhancedActivities ? FILE_ACTIVITY_TYPE_ENHANCED_COMMENT : FILE_ACTIVITY_TYPE_COMMENT]
: [];
// comment: older clients. enhanced_comment: point timestamps.
// enhanced_comment_timespan: audio files when the audio player updates split is on.
let commentFileActivityType = FILE_ACTIVITY_TYPE_COMMENT;
if (shouldUseEnhancedActivities) {
commentFileActivityType = shouldUseEnhancedTimespanComments
? FILE_ACTIVITY_TYPE_ENHANCED_COMMENT_TIMESPAN
: FILE_ACTIVITY_TYPE_ENHANCED_COMMENT;
}
const commentActivityType = permissions[PERMISSION_CAN_COMMENT] ? [commentFileActivityType] : [];
const filteredActivityTypes = [
...annotationActivityType,
...appActivityActivityType,
Expand Down
50 changes: 50 additions & 0 deletions src/api/__tests__/Feed.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
FILE_ACTIVITY_TYPE_COMMENT,
FILE_ACTIVITY_TYPE_ENHANCED_ANNOTATION,
FILE_ACTIVITY_TYPE_ENHANCED_COMMENT,
FILE_ACTIVITY_TYPE_ENHANCED_COMMENT_TIMESPAN,
FILE_ACTIVITY_TYPE_TASK,
FILE_ACTIVITY_TYPE_VERSION,
IS_ERROR_DISPLAYED,
Expand Down Expand Up @@ -707,6 +708,35 @@ describe('api/Feed', () => {
done();
});
});

test('should request enhanced_comment_timespan when the audio player updates split is on', done => {
feed.feedItems(file, false, successCb, errorCb, errorCb, {
shouldShowAnnotations: true,
shouldShowAppActivity: true,
shouldShowReplies: true,
shouldShowTasks: true,
shouldShowVersions: true,
shouldUseEnhancedActivities: true,
shouldUseEnhancedTimespanComments: true,
shouldUseUAA: true,
});
setImmediate(() => {
expect(feed.fetchFileActivities).toBeCalledWith(
file.permissions,
[
FILE_ACTIVITY_TYPE_ENHANCED_ANNOTATION,
FILE_ACTIVITY_TYPE_APP_ACTIVITY,
FILE_ACTIVITY_TYPE_ENHANCED_COMMENT_TIMESPAN,
FILE_ACTIVITY_TYPE_TASK,
FILE_ACTIVITY_TYPE_VERSION,
],
true,
true,
false,
);
done();
});
});
});

describe('fetchAnnotations()', () => {
Expand Down Expand Up @@ -2539,6 +2569,26 @@ describe('api/Feed', () => {
expect(byId['enh-annotation-1'].type).toBe(FEED_ITEM_TYPE_ANNOTATION);
});

test('should remap enhanced_comment_timespan activity types to the legacy comment type', () => {
const enhancedCommentTimespan = {
...threadedCommentsFormatted[0],
id: 'enh-comment-timespan',
message: '#[timestamp:8055,endTimestamp:12000,versionId:1] range',
type: FILE_ACTIVITY_TYPE_ENHANCED_COMMENT_TIMESPAN,
};
const parsed = getParsedFileActivitiesResponse({
entries: [
{
activity_type: FILE_ACTIVITY_TYPE_ENHANCED_COMMENT_TIMESPAN,
source: { [FILE_ACTIVITY_TYPE_ENHANCED_COMMENT_TIMESPAN]: enhancedCommentTimespan },
},
],
});
expect(parsed).toHaveLength(1);
expect(parsed[0].id).toBe('enh-comment-timespan');
expect(parsed[0].type).toBe(FEED_ITEM_TYPE_COMMENT);
});

test('should drop entries whose activity_type does not match a populated source key', () => {
const response = {
entries: [
Expand Down
5 changes: 5 additions & 0 deletions src/common/types/feed.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
FILE_ACTIVITY_TYPE_COMMENT,
FILE_ACTIVITY_TYPE_ENHANCED_ANNOTATION,
FILE_ACTIVITY_TYPE_ENHANCED_COMMENT,
FILE_ACTIVITY_TYPE_ENHANCED_COMMENT_TIMESPAN,
FILE_ACTIVITY_TYPE_TASK,
FILE_ACTIVITY_TYPE_VERSION,
} from '../../constants';
Expand Down Expand Up @@ -183,6 +184,7 @@ type FileActivityTypes =
| typeof FILE_ACTIVITY_TYPE_COMMENT
| typeof FILE_ACTIVITY_TYPE_ENHANCED_ANNOTATION
| typeof FILE_ACTIVITY_TYPE_ENHANCED_COMMENT
| typeof FILE_ACTIVITY_TYPE_ENHANCED_COMMENT_TIMESPAN
| typeof FILE_ACTIVITY_TYPE_TASK
| typeof FILE_ACTIVITY_TYPE_VERSION;

Expand All @@ -202,6 +204,9 @@ type FileActivitySource =
| {
enhanced_comment: Comment,
}
| {
enhanced_comment_timespan: Comment,
}
| {
task: TaskNew,
}
Expand Down
1 change: 1 addition & 0 deletions src/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ export const FILE_ACTIVITY_TYPE_APP_ACTIVITY: 'app_activity' = 'app_activity';
export const FILE_ACTIVITY_TYPE_COMMENT: 'comment' = 'comment';
export const FILE_ACTIVITY_TYPE_ENHANCED_ANNOTATION: 'enhanced_annotation' = 'enhanced_annotation';
export const FILE_ACTIVITY_TYPE_ENHANCED_COMMENT: 'enhanced_comment' = 'enhanced_comment';
export const FILE_ACTIVITY_TYPE_ENHANCED_COMMENT_TIMESPAN: 'enhanced_comment_timespan' = 'enhanced_comment_timespan';
export const FILE_ACTIVITY_TYPE_TASK: 'task' = 'task';
export const FILE_ACTIVITY_TYPE_VERSION: 'versions' = 'versions';

Expand Down
4 changes: 4 additions & 0 deletions src/elements/content-sidebar/ActivitySidebar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { mark } from '../../utils/performance';
import { withAnnotatorContext } from '../common/annotator-context';
import { withAPIContext } from '../common/api-context';
import { withErrorBoundary } from '../common/error-boundary';
import { FILE_EXTENSIONS } from '../common/item/constants';
import { withFeatureConsumer, isFeatureEnabled, getFeatureConfig } from '../common/feature-checking';
import { withLogger } from '../common/logger';
import { withRouterAndRef } from '../common/routing';
Expand Down Expand Up @@ -814,6 +815,8 @@ class ActivitySidebar extends React.PureComponent<Props, State> {
hasVersions: shouldShowVersions,
} = this.props;
const isThreadedRepliesV2Enabled = isFeatureEnabled(features, 'activityFeed.threadedRepliesV2.enabled');
const isAudioPlayerV2Enabled = isFeatureEnabled(features, 'audioPlayerV2.enabled');
const isAudioFile = Boolean(file.extension && FILE_EXTENSIONS.audio.includes(file.extension));
const shouldShowReplies = hasReplies || isThreadedRepliesV2Enabled;
const shouldFetchReplies =
shouldRefreshCache && hasReplies && activeFeedEntryId && activeFeedEntryType === FEED_ITEM_TYPE_COMMENT;
Expand All @@ -835,6 +838,7 @@ class ActivitySidebar extends React.PureComponent<Props, State> {
shouldShowTasks,
shouldShowVersions,
shouldUseEnhancedActivities: isThreadedRepliesV2Enabled,
shouldUseEnhancedTimespanComments: isAudioPlayerV2Enabled && isAudioFile,
shouldUseUAA,
shouldEnableRichText,
},
Expand Down
56 changes: 56 additions & 0 deletions src/elements/content-sidebar/__tests__/ActivitySidebar.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,7 @@ describe('elements/content-sidebar/ActivitySidebar', () => {
shouldShowTasks: expectedTasks,
shouldShowVersions: expectedVersions,
shouldUseEnhancedActivities: false,
shouldUseEnhancedTimespanComments: false,
shouldUseUAA: expectedUseUAA,
shouldEnableRichText: false,
},
Expand Down Expand Up @@ -881,6 +882,7 @@ describe('elements/content-sidebar/ActivitySidebar', () => {
shouldShowTasks: true,
shouldShowVersions: true,
shouldUseEnhancedActivities: false,
shouldUseEnhancedTimespanComments: false,
shouldUseUAA: false,
shouldEnableRichText: false,
},
Expand Down Expand Up @@ -938,6 +940,60 @@ describe('elements/content-sidebar/ActivitySidebar', () => {
);
});

test('should request timespan comments for an audio file when audioPlayerV2 is enabled', () => {
const audioFile = { ...file, extension: 'mp3' };
wrapper = getWrapper({
file: audioFile,
features: {
activityFeed: {
threadedRepliesV2: { enabled: true },
},
audioPlayerV2: { enabled: true },
},
});
instance = wrapper.instance();
instance.fetchFeedItems();

expect(feedAPI.feedItems).toHaveBeenCalledWith(
audioFile,
false,
instance.fetchFeedItemsSuccessCallback,
instance.fetchFeedItemsErrorCallback,
instance.errorCallback,
expect.objectContaining({
shouldUseEnhancedActivities: true,
shouldUseEnhancedTimespanComments: true,
}),
);
});

test('should request enhanced_comment for a non-audio file when audioPlayerV2 is enabled', () => {
const videoFile = { ...file, extension: 'mp4' };
wrapper = getWrapper({
file: videoFile,
features: {
activityFeed: {
threadedRepliesV2: { enabled: true },
},
audioPlayerV2: { enabled: true },
},
});
instance = wrapper.instance();
instance.fetchFeedItems();

expect(feedAPI.feedItems).toHaveBeenCalledWith(
videoFile,
false,
instance.fetchFeedItemsSuccessCallback,
instance.fetchFeedItemsErrorCallback,
instance.errorCallback,
expect.objectContaining({
shouldUseEnhancedActivities: true,
shouldUseEnhancedTimespanComments: false,
}),
);
});

test('should set shouldShowReplies and shouldUseEnhancedActivities to true when both hasReplies and threadedRepliesV2 are enabled', () => {
wrapper = getWrapper({
features: {
Expand Down
Loading