From b89951cf5dcd6f76ec003671dddd2c65efe0d2bf Mon Sep 17 00:00:00 2001 From: Mathias Jakobsen Date: Mon, 7 Jul 2025 12:12:35 +0100 Subject: [PATCH] Merge pull request #26814 from overleaf/mj-review-panel-comment-collapse [web] Fix review panel check for comments in active document GitOrigin-RevId: fc0c35bc1d2f253c133dec5dcea7f81f68d723a9 --- .../utils/has-active-range.ts | 5 +- .../review-panel/review-panel.spec.tsx | 195 ++++++++++++++++-- 2 files changed, 185 insertions(+), 15 deletions(-) diff --git a/services/web/frontend/js/features/review-panel-new/utils/has-active-range.ts b/services/web/frontend/js/features/review-panel-new/utils/has-active-range.ts index 0934aef59f..7b636c69a3 100644 --- a/services/web/frontend/js/features/review-panel-new/utils/has-active-range.ts +++ b/services/web/frontend/js/features/review-panel-new/utils/has-active-range.ts @@ -15,8 +15,9 @@ export const hasActiveRange = ( return true } - for (const thread of Object.values(threads)) { - if (!thread.resolved) { + for (const comment of ranges.comments) { + const thread = threads[comment.op.t] + if (thread && !thread.resolved) { return true } } diff --git a/services/web/test/frontend/features/review-panel/review-panel.spec.tsx b/services/web/test/frontend/features/review-panel/review-panel.spec.tsx index 58ac3e443d..9cccd5093b 100644 --- a/services/web/test/frontend/features/review-panel/review-panel.spec.tsx +++ b/services/web/test/frontend/features/review-panel/review-panel.spec.tsx @@ -8,6 +8,19 @@ import { mockScope } from '../source-editor/helpers/mock-scope' import { TestContainer } from '../source-editor/helpers/test-container' import { docId } from '../source-editor/helpers/mock-doc' +const userData = { + avatar_text: 'User', + email: USER_EMAIL, + hue: 180, + id: USER_ID, + isSelf: true, + first_name: 'Test', + last_name: 'User', +} + +const resolvedThreadId = 'resolved-thread-id' +const unresolvedThreadId = 'unresolved-thread-id' + describe('', function () { beforeEach(function () { window.metaAttributesCache.set('ol-preventCompileOnLoad', true) @@ -23,19 +36,6 @@ describe('', function () { }, ]) - const userData = { - avatar_text: 'User', - email: USER_EMAIL, - hue: 180, - id: USER_ID, - isSelf: true, - first_name: 'Test', - last_name: 'User', - } - - const resolvedThreadId = 'resolved-thread-id' - const unresolvedThreadId = 'unresolved-thread-id' - cy.intercept('GET', '/project/*/threads', { // Resolved comment thread [resolvedThreadId]: { @@ -623,6 +623,175 @@ describe('', function () { }) }) +describe(' in mini mode', function () { + function render({ comments = [], changes = [], threads = {} }: any) { + window.metaAttributesCache.set('ol-preventCompileOnLoad', true) + + cy.interceptEvents() + + cy.intercept('GET', '/project/*/changes/users', [ + { + id: USER_ID, + email: USER_EMAIL, + first_name: 'Test', + last_name: 'User', + }, + ]) + + const getChanges = cy.stub().as('getChanges').returns([]) + const removeChangeIds = cy.stub().as('removeChangeIds') + + const scope = mockScope(undefined, { + docOptions: { + rangesOptions: { + comments, + changes, + getChanges, + removeChangeIds, + }, + }, + projectFeatures: { trackChangesVisible: true }, + }) + + cy.intercept('GET', '/project/*/ranges', [ + { + id: docId, + ranges: { + changes, + comments, + docId, + }, + }, + ]) + + cy.intercept('GET', '/project/*/threads', threads) + + cy.intercept('POST', `/project/*/doc/${docId}/metadata`, {}) + + cy.wrap(scope).as('scope') + + cy.mount( + + + + + + ) + // Wait for editor + cy.get('.cm-content').should('have.css', 'opacity', '1') + + // Toggle the review panel twice to ensure data is loaded + cy.findByText('contentLine 0').type('{command}jj', { + scrollBehavior: false, + }) + cy.findByText('contentLine 1').type('{ctrl}jj', { scrollBehavior: false }) + } + + it("doesn't render mini when no comments or changes are present in project", function () { + render({ + comments: [], + changes: [], + threads: {}, + }) + cy.get('.review-panel-mini').should('not.exist') + }) + + it("doesn't render mini when no comments or changes are present in document", function () { + render({ + comments: [], + changes: [], + threads: { + 'random-unrelated-thread': { + messages: [ + { + content: 'a comment', + id: 'random-unrelated-thread-1', + timestamp: new Date('2025-01-01T01:00:00.000Z'), + user: userData, + user_id: USER_ID, + }, + ], + }, + }, + }) + cy.get('.review-panel-mini').should('not.exist') + }) + + it("doesn't render mini when a resolved comments is present in document", function () { + render({ + comments: [ + { + id: resolvedThreadId, + op: { p: 161, c: 'Your introduction', t: resolvedThreadId }, + }, + ], + changes: [], + threads: { + [resolvedThreadId]: { + resolved: true, + resolved_at: new Date('2025-01-02T00:00:00.000Z').toISOString(), + resolved_by_user_id: USER_ID, + resolved_by_user: userData, + messages: [ + { + content: 'a comment', + id: `${resolvedThreadId}-1`, + timestamp: new Date('2025-01-01T01:00:00.000Z'), + user: userData, + user_id: USER_ID, + }, + ], + }, + }, + }) + cy.get('.review-panel-mini').should('not.exist') + }) + + it('renders mini when a unresolved comments is present in document', function () { + render({ + comments: [ + { + id: unresolvedThreadId, + op: { p: 161, c: 'Your introduction', t: unresolvedThreadId }, + }, + ], + changes: [], + threads: { + [unresolvedThreadId]: { + messages: [ + { + content: 'a comment', + id: `${unresolvedThreadId}-1`, + timestamp: new Date('2025-01-01T01:00:00.000Z'), + user: userData, + user_id: USER_ID, + }, + ], + }, + }, + }) + cy.get('.review-panel-mini').should('exist') + }) + + it('renders mini when a tracked change is present in document', function () { + render({ + comments: [], + changes: [ + { + metadata: { + user_id: USER_ID, + ts: new Date('2025-01-01T00:00:00.000Z'), + }, + id: 'inserted-op-id', + op: { p: 166, t: 'inserted-op-id', i: 'introduction' }, + }, + ], + threads: {}, + }) + cy.get('.review-panel-mini').should('exist') + }) +}) + describe(' for free users', function () { function mountEditor(ownerId = USER_ID) { const scope = mockScope(undefined, {