diff --git a/apps/server/src/core/comment/comment.service.apply-suggestion.spec.ts b/apps/server/src/core/comment/comment.service.apply-suggestion.spec.ts index bcfc2f2c..100dae67 100644 --- a/apps/server/src/core/comment/comment.service.apply-suggestion.spec.ts +++ b/apps/server/src/core/comment/comment.service.apply-suggestion.spec.ts @@ -197,17 +197,25 @@ describe('CommentService — applySuggestion', () => { expect(resolvePatch.resolvedAt).toBeInstanceOf(Date); expect(resolvePatch.resolvedById).toBe('user-1'); - // NOT deleted; broadcast an update, not a deletion. + // NOT deleted. expect(commentRepo.deleteComment).not.toHaveBeenCalled(); expect(collaborationGateway.handleYjsEvent).not.toHaveBeenCalledWith( 'deleteCommentMark', expect.anything(), expect.anything(), ); + // #496 dedup: resolveComment broadcasts `commentResolved` with the enriched + // row; finalize must NOT ALSO emit a redundant `commentUpdated`. So the + // thread receives exactly ONE resolve broadcast and no update broadcast. expect(wsService.emitCommentEvent).toHaveBeenCalledWith( 'space-1', 'page-1', - expect.objectContaining({ operation: 'commentUpdated', comment: UPDATED }), + expect.objectContaining({ operation: 'commentResolved', comment: UPDATED }), + ); + expect(wsService.emitCommentEvent).not.toHaveBeenCalledWith( + 'space-1', + 'page-1', + expect.objectContaining({ operation: 'commentUpdated' }), ); expect(auditService.log).toHaveBeenCalledWith( @@ -219,6 +227,36 @@ describe('CommentService — applySuggestion', () => { expect(result.outcome).toBe('resolved'); }); + it('re-entry: already applied+resolved WITH replies → emits commentUpdated (dedup does not over-suppress)', async () => { + // suggestionAppliedAt set → idempotent finalize; resolvedAt set → resolveComment + // is skipped, so there is NO commentResolved broadcast. The applied-stamp state + // must still reach clients via a single commentUpdated. + const { service, wsService } = makeService( + { applied: false, currentText: 'new text' }, + true, + ); + + await service.applySuggestion( + suggestionComment({ + suggestionAppliedAt: new Date(), + resolvedAt: new Date(), + }), + user(), + ); + + expect(wsService.emitCommentEvent).toHaveBeenCalledWith( + 'space-1', + 'page-1', + expect.objectContaining({ operation: 'commentUpdated', comment: UPDATED }), + ); + // Nothing resolved this time (already resolved) → no resolve broadcast. + expect(wsService.emitCommentEvent).not.toHaveBeenCalledWith( + 'space-1', + 'page-1', + expect.objectContaining({ operation: 'commentResolved' }), + ); + }); + // --- error / rejection branches ----------------------------------------- it('applied=false and currentText differs → ConflictException with currentText in payload', async () => { diff --git a/apps/server/src/core/comment/comment.service.ts b/apps/server/src/core/comment/comment.service.ts index c053ccf8..88122e96 100644 --- a/apps/server/src/core/comment/comment.service.ts +++ b/apps/server/src/core/comment/comment.service.ts @@ -577,8 +577,10 @@ export class CommentService { // Auto-resolve the thread. resolveComment handles the resolve mark, its ws // broadcast and the resolve notification. Stay defensive on re-entry. + let didResolveBroadcast = false; if (!comment.resolvedAt) { await this.resolveComment(comment, true, user, provenance); + didResolveBroadcast = true; } const updatedComment = await this.commentRepo.findById(comment.id, { @@ -586,11 +588,20 @@ export class CommentService { includeResolvedBy: true, }); - this.wsService.emitCommentEvent(comment.spaceId, comment.pageId, { - operation: 'commentUpdated', - pageId: comment.pageId, - comment: updatedComment, - }); + // #496 dedup: resolveComment already broadcast `commentResolved` carrying + // the fully-enriched row (the applied stamps were persisted above, before + // that call, so its re-read reflects them). Emitting `commentUpdated` here + // too made the client receive TWO events for one apply. Broadcast the + // update ONLY when we did NOT resolve — i.e. the rare re-entry on an + // already-resolved thread, where the applied-stamp change still needs a + // broadcast and resolveComment did not run. + if (!didResolveBroadcast) { + this.wsService.emitCommentEvent(comment.spaceId, comment.pageId, { + operation: 'commentUpdated', + pageId: comment.pageId, + comment: updatedComment, + }); + } this.auditService.log({ event: AuditEvent.COMMENT_SUGGESTION_APPLIED,