From ec542a924b05e6fba67e11941c709e286c68a4f0 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 3 Jul 2026 19:09:23 +0300 Subject: [PATCH] feat(comment): store suggestedText + POST /comments/apply-suggestion (#315 phase 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server side of agent comment suggestions. - CreateCommentDto gains optional suggestedText (<=2000). CommentService.create accepts it ONLY for a top-level inline comment with a non-empty selection, requires it be non-empty and differ from selection (else BadRequest), and stores it. - POST /comments/apply-suggestion (ApplySuggestionDto { commentId }): authorizes with validateCanEdit (applying edits page text) BEFORE any structural check or mutation, then CommentService.applySuggestion: - runs the phase-3 collab event applyCommentSuggestion on `page.` to atomically check-and-replace the marked text, returning { applied, currentText }; - applied → stamp suggestion_applied_at/by, auto-resolve the thread, ws commentUpdated, audit COMMENT_SUGGESTION_APPLIED; - already-applied (DB) → idempotent success (no re-apply), self-healing the resolve if it was missed — satisfies the issue's double-click / two-user race requirement; - collab verdict applied:false && currentText===suggestedText → idempotent success (crash between doc mutation and DB write); - text changed → 409 ConflictException carrying currentText; - gateway undefined/throw → hard error, never a silent success. - audit-events: COMMENT_SUGGESTION_APPLIED. Tests: create validation (reply/no-selection/equal-to-selection rejected; valid stored) + applySuggestion verdict branches incl. both idempotent paths. jest src/core/comment: 33 passed. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/server/src/common/events/audit-events.ts | 1 + .../src/core/comment/comment.controller.ts | 37 +++ .../comment.service.apply-suggestion.spec.ts | 245 ++++++++++++++++++ .../comment/comment.service.behavior.spec.ts | 93 +++++++ ...mment.service.provenance-broadcast.spec.ts | 3 + .../src/core/comment/comment.service.spec.ts | 1 + .../src/core/comment/comment.service.ts | 192 +++++++++++++- .../core/comment/dto/apply-suggestion.dto.ts | 6 + .../core/comment/dto/create-comment.dto.ts | 18 +- 9 files changed, 594 insertions(+), 2 deletions(-) create mode 100644 apps/server/src/core/comment/comment.service.apply-suggestion.spec.ts create mode 100644 apps/server/src/core/comment/dto/apply-suggestion.dto.ts diff --git a/apps/server/src/common/events/audit-events.ts b/apps/server/src/common/events/audit-events.ts index d8be76f8..b7cfabe5 100644 --- a/apps/server/src/common/events/audit-events.ts +++ b/apps/server/src/common/events/audit-events.ts @@ -51,6 +51,7 @@ export const AuditEvent = { COMMENT_UPDATED: 'comment.updated', COMMENT_RESOLVED: 'comment.resolved', COMMENT_REOPENED: 'comment.reopened', + COMMENT_SUGGESTION_APPLIED: 'comment.suggestion_applied', // Page PAGE_CREATED: 'page.created', diff --git a/apps/server/src/core/comment/comment.controller.ts b/apps/server/src/core/comment/comment.controller.ts index 96e8d5d2..f04f32e5 100644 --- a/apps/server/src/core/comment/comment.controller.ts +++ b/apps/server/src/core/comment/comment.controller.ts @@ -14,6 +14,7 @@ import { CommentService } from './comment.service'; import { CreateCommentDto } from './dto/create-comment.dto'; import { UpdateCommentDto } from './dto/update-comment.dto'; import { ResolveCommentDto } from './dto/resolve-comment.dto'; +import { ApplySuggestionDto } from './dto/apply-suggestion.dto'; import { PageIdDto, CommentIdDto } from './dto/comments.input'; import { AuthUser } from '../../common/decorators/auth-user.decorator'; import { AuthWorkspace } from '../../common/decorators/auth-workspace.decorator'; @@ -197,6 +198,42 @@ export class CommentController { return updated; } + @HttpCode(HttpStatus.OK) + @Post('apply-suggestion') + async applySuggestion( + @Body() dto: ApplySuggestionDto, + @AuthUser() user: User, + @AuthWorkspace() workspace: Workspace, + @AuthProvenance() provenance: AuthProvenanceData, + ) { + const comment = await this.commentRepo.findById(dto.commentId, { + includeCreator: true, + includeResolvedBy: true, + }); + if (!comment) { + throw new NotFoundException('Comment not found'); + } + + const page = await this.pageRepo.findById(comment.pageId); + if (!page || page.deletedAt) { + throw new NotFoundException('Page not found'); + } + + // Authorize BEFORE revealing any structural detail about the comment + // (metadata-disclosure hygiene). Applying a suggestion rewrites the page + // text, so require edit access (NOT just comment access). Running this + // first means a cross-workspace user with a guessed comment UUID gets a + // uniform 403 regardless of the comment's type or suggestion state — it can + // never distinguish those before the access check. The structural 400s + // (top-level / has-a-suggested-edit) are re-checked by the service below. + await this.pageAccessService.validateCanEdit(page, user); + + // The service re-validates the comment's state, returns idempotent success + // for an already-applied suggestion, and lets ConflictException (409, with + // currentText in the payload) propagate untouched. + return this.commentService.applySuggestion(comment, user, provenance); + } + @HttpCode(HttpStatus.OK) @Post('delete') async delete(@Body() input: CommentIdDto, @AuthUser() user: User, @AuthWorkspace() workspace: Workspace) { 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 new file mode 100644 index 00000000..aa11f41e --- /dev/null +++ b/apps/server/src/core/comment/comment.service.apply-suggestion.spec.ts @@ -0,0 +1,245 @@ +import { + BadRequestException, + ConflictException, + InternalServerErrorException, +} from '@nestjs/common'; +import { CommentService } from './comment.service'; +import { AuditEvent, AuditResource } from '../../common/events/audit-events'; + +/** + * Focused coverage for CommentService.applySuggestion (comment.service.ts). + * The service is constructed directly with jest-mocked deps (the @InjectQueue + * tokens can't be resolved by Test.createTestingModule — see the sibling specs). + * + * The collaboration gateway verdict is the pivot of the whole flow, so each test + * pins a specific { applied, currentText } and asserts the DB persistence, + * auto-resolve, audit, ws broadcast, and error mapping that follow from it. + */ +describe('CommentService — applySuggestion', () => { + const UPDATED = { id: 'c-1', __updated: true } as any; + + function makeService(verdict: unknown) { + const commentRepo: any = { + // Both the applied-stamp re-read and resolveComment's re-read go through + // findById; return a recognizable enriched row. + findById: jest.fn(async () => UPDATED), + updateComment: jest.fn(async () => undefined), + }; + const pageRepo: any = {}; + const wsService: any = { emitCommentEvent: jest.fn() }; + const collaborationGateway: any = { + handleYjsEvent: jest.fn(async () => verdict), + }; + const generalQueue: any = { add: jest.fn(() => Promise.resolve()) }; + const notificationQueue: any = { add: jest.fn(async () => undefined) }; + const auditService: any = { log: jest.fn() }; + + const service = new CommentService( + commentRepo, + pageRepo, + wsService, + collaborationGateway, + generalQueue, + notificationQueue, + auditService, + ); + + return { + service, + commentRepo, + wsService, + collaborationGateway, + auditService, + }; + } + + const suggestionComment = (over?: Partial): any => ({ + id: 'c-1', + pageId: 'page-1', + spaceId: 'space-1', + workspaceId: 'ws-1', + creatorId: 'user-1', + parentCommentId: null, + selection: 'old text', + suggestedText: 'new text', + suggestionAppliedAt: null, + resolvedAt: null, + ...over, + }); + const user = (over?: Partial): any => ({ id: 'user-1', ...over }); + + // Pull the updateComment patch that carries the applied stamps. + const appliedPatch = (commentRepo: any) => + commentRepo.updateComment.mock.calls + .map((c: any[]) => c[0]) + .find((patch: any) => 'suggestionAppliedAt' in patch); + + it('applied=true → replaces text, persists applied stamps, auto-resolves, audits, returns updated', async () => { + const { service, commentRepo, wsService, collaborationGateway, auditService } = + makeService({ applied: true, currentText: 'new text' }); + + const result = await service.applySuggestion(suggestionComment(), user()); + + // The atomic replace was requested against the exact marked text. + expect(collaborationGateway.handleYjsEvent).toHaveBeenCalledWith( + 'applyCommentSuggestion', + 'page.page-1', + expect.objectContaining({ + commentId: 'c-1', + expectedText: 'old text', + newText: 'new text', + user: expect.objectContaining({ id: 'user-1' }), + }), + ); + + // Applied stamps persisted. + const patch = appliedPatch(commentRepo); + expect(patch.suggestionAppliedAt).toBeInstanceOf(Date); + expect(patch.suggestionAppliedById).toBe('user-1'); + + // Auto-resolved: resolveComment writes a resolvedAt/resolvedById patch too. + const resolvePatch = commentRepo.updateComment.mock.calls + .map((c: any[]) => c[0]) + .find((p: any) => 'resolvedAt' in p); + expect(resolvePatch.resolvedAt).toBeInstanceOf(Date); + expect(resolvePatch.resolvedById).toBe('user-1'); + + // Audit + broadcast + return. + expect(auditService.log).toHaveBeenCalledWith( + expect.objectContaining({ + event: AuditEvent.COMMENT_SUGGESTION_APPLIED, + resourceType: AuditResource.COMMENT, + resourceId: 'c-1', + spaceId: 'space-1', + metadata: { pageId: 'page-1' }, + }), + ); + expect(wsService.emitCommentEvent).toHaveBeenCalledWith( + 'space-1', + 'page-1', + expect.objectContaining({ operation: 'commentUpdated', comment: UPDATED }), + ); + expect(result).toBe(UPDATED); + }); + + it('applied=false but currentText === suggestedText → idempotent success (no 409)', async () => { + const { service, commentRepo, auditService } = makeService({ + applied: false, + currentText: 'new text', + }); + + const result = await service.applySuggestion(suggestionComment(), user()); + + // The stamps are still persisted (reconciling a crash between the doc + // mutation and the DB write) and the call succeeds. + const patch = appliedPatch(commentRepo); + expect(patch.suggestionAppliedAt).toBeInstanceOf(Date); + expect(patch.suggestionAppliedById).toBe('user-1'); + expect(auditService.log).toHaveBeenCalledTimes(1); + expect(result).toBe(UPDATED); + }); + + it('applied=false and currentText differs → ConflictException with currentText in payload', async () => { + const { service, commentRepo, auditService } = makeService({ + applied: false, + currentText: 'someone else edited this', + }); + + const err = await service + .applySuggestion(suggestionComment(), user()) + .catch((e) => e); + + expect(err).toBeInstanceOf(ConflictException); + expect(err.getResponse()).toMatchObject({ + currentText: 'someone else edited this', + }); + // No persistence and no audit on a conflict. + expect(appliedPatch(commentRepo)).toBeUndefined(); + expect(auditService.log).not.toHaveBeenCalled(); + }); + + it('already-applied AND already-resolved → idempotent success, no collab call, no re-resolve (#315 double-click)', async () => { + const { service, collaborationGateway, commentRepo, auditService } = + makeService({ applied: true, currentText: 'new text' }); + + const result = await service.applySuggestion( + suggestionComment({ + suggestionAppliedAt: new Date(), + resolvedAt: new Date(), + resolvedById: 'user-1', + }), + user(), + ); + + // Idempotent SUCCESS, not a 409. The suggestion is already applied, so the + // collaborative document is never touched again and nothing is re-stamped + // or re-resolved. + expect(result).toBe(UPDATED); + expect(collaborationGateway.handleYjsEvent).not.toHaveBeenCalled(); + expect(commentRepo.updateComment).not.toHaveBeenCalled(); + // Same success shape as the applied path (broadcast + audit). + expect(auditService.log).toHaveBeenCalledTimes(1); + }); + + it('already-applied but NOT resolved (crash window) → idempotent success, self-heals resolve, no re-apply', async () => { + const { service, collaborationGateway, commentRepo } = makeService({ + applied: true, + currentText: 'new text', + }); + + const result = await service.applySuggestion( + suggestionComment({ suggestionAppliedAt: new Date(), resolvedAt: null }), + user(), + ); + + expect(result).toBe(UPDATED); + + // The suggestion is NOT re-applied to the document… + expect(collaborationGateway.handleYjsEvent).not.toHaveBeenCalledWith( + 'applyCommentSuggestion', + expect.anything(), + expect.anything(), + ); + // …but the open thread is self-healed to resolved via resolveComment, which + // writes the resolve patch and updates the resolve mark. + const resolvePatch = commentRepo.updateComment.mock.calls + .map((c: any[]) => c[0]) + .find((p: any) => 'resolvedAt' in p); + expect(resolvePatch.resolvedAt).toBeInstanceOf(Date); + expect(resolvePatch.resolvedById).toBe('user-1'); + expect(collaborationGateway.handleYjsEvent).toHaveBeenCalledWith( + 'resolveCommentMark', + 'page.page-1', + expect.objectContaining({ commentId: 'c-1', resolved: true }), + ); + // The applied stamps are NOT re-written (already stamped). + expect(appliedPatch(commentRepo)).toBeUndefined(); + }); + + it('rejects a comment with no suggestedText', async () => { + const { service, collaborationGateway } = makeService({ + applied: true, + currentText: 'x', + }); + + await expect( + service.applySuggestion( + suggestionComment({ suggestedText: null }), + user(), + ), + ).rejects.toThrow(BadRequestException); + expect(collaborationGateway.handleYjsEvent).not.toHaveBeenCalled(); + }); + + it('gateway returning undefined → hard error, not a silent success', async () => { + const { service, commentRepo, auditService } = makeService(undefined); + + await expect( + service.applySuggestion(suggestionComment(), user()), + ).rejects.toThrow(InternalServerErrorException); + + // Nothing persisted, nothing audited. + expect(appliedPatch(commentRepo)).toBeUndefined(); + expect(auditService.log).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/server/src/core/comment/comment.service.behavior.spec.ts b/apps/server/src/core/comment/comment.service.behavior.spec.ts index 91572496..00f2aaaf 100644 --- a/apps/server/src/core/comment/comment.service.behavior.spec.ts +++ b/apps/server/src/core/comment/comment.service.behavior.spec.ts @@ -60,6 +60,7 @@ describe('CommentService — behavior', () => { }; const generalQueue: any = { add: jest.fn(() => Promise.resolve()) }; const notificationQueue: any = { add: jest.fn(async () => undefined) }; + const auditService: any = { log: jest.fn() }; const service = new CommentService( commentRepo, @@ -68,14 +69,17 @@ describe('CommentService — behavior', () => { collaborationGateway, generalQueue, notificationQueue, + auditService, ); return { service, commentRepo, wsService, + collaborationGateway, generalQueue, notificationQueue, + auditService, }; } @@ -181,6 +185,95 @@ describe('CommentService — behavior', () => { }); }); + describe('create — suggested edit validation & storage', () => { + it('rejects a suggestedText on a reply (not a top-level comment)', async () => { + const parentComment = { + id: 'parent-1', + pageId: 'page-1', + parentCommentId: null, + }; + const { service, commentRepo } = makeService({ parentComment }); + + await expect( + service.create( + { page: page(), workspaceId: 'ws-1', user: user() }, + { + content: JSON.stringify(docMentioning()), + parentCommentId: 'parent-1', + selection: 'hello world', + suggestedText: 'goodbye world', + } as any, + ), + ).rejects.toThrow(BadRequestException); + + expect(commentRepo.insertComment).not.toHaveBeenCalled(); + }); + + it('rejects a suggestedText without a selection', async () => { + const { service, commentRepo } = makeService(); + + await expect( + service.create( + { page: page(), workspaceId: 'ws-1', user: user() }, + { + content: JSON.stringify(docMentioning()), + suggestedText: 'new text', + } as any, + ), + ).rejects.toThrow(BadRequestException); + + expect(commentRepo.insertComment).not.toHaveBeenCalled(); + }); + + it('rejects a suggestedText identical to the selection (no-op)', async () => { + const { service, commentRepo } = makeService(); + + await expect( + service.create( + { page: page(), workspaceId: 'ws-1', user: user() }, + { + content: JSON.stringify(docMentioning()), + selection: 'same text', + // Only differs by surrounding whitespace → still a no-op after trim. + suggestedText: ' same text ', + } as any, + ), + ).rejects.toThrow(BadRequestException); + + expect(commentRepo.insertComment).not.toHaveBeenCalled(); + }); + + it('stores a valid suggestedText (trimmed) on the inserted row', async () => { + const { service, commentRepo } = makeService(); + + await service.create( + { page: page(), workspaceId: 'ws-1', user: user() }, + { + content: JSON.stringify(docMentioning()), + selection: 'old text', + type: 'inline', + suggestedText: ' new text ', + } as any, + ); + + const insertArg = commentRepo.insertComment.mock.calls[0][0]; + expect(insertArg.suggestedText).toBe('new text'); + expect(insertArg.selection).toBe('old text'); + }); + + it('leaves suggestedText null for an ordinary comment', async () => { + const { service, commentRepo } = makeService(); + + await service.create( + { page: page(), workspaceId: 'ws-1', user: user() }, + { content: JSON.stringify(docMentioning()) } as any, + ); + + const insertArg = commentRepo.insertComment.mock.calls[0][0]; + expect(insertArg.suggestedText).toBeNull(); + }); + }); + describe('resolveComment — provenance & resolve notifications', () => { it('stamps resolvedSource:"agent" when an agent resolves', async () => { const { service, commentRepo } = makeService(); diff --git a/apps/server/src/core/comment/comment.service.provenance-broadcast.spec.ts b/apps/server/src/core/comment/comment.service.provenance-broadcast.spec.ts index 2d962f1a..e2c828d5 100644 --- a/apps/server/src/core/comment/comment.service.provenance-broadcast.spec.ts +++ b/apps/server/src/core/comment/comment.service.provenance-broadcast.spec.ts @@ -61,6 +61,8 @@ describe('CommentService — broadcast carries the agent avatar stack', () => { const generalQueue: any = { add: jest.fn(() => Promise.resolve()) }; const notificationQueue: any = { add: jest.fn(async () => undefined) }; + const auditService: any = { log: jest.fn() }; + const service = new CommentService( commentRepo, pageRepo, @@ -68,6 +70,7 @@ describe('CommentService — broadcast carries the agent avatar stack', () => { collaborationGateway, generalQueue, notificationQueue, + auditService, ); return { service, commentRepo, wsService }; diff --git a/apps/server/src/core/comment/comment.service.spec.ts b/apps/server/src/core/comment/comment.service.spec.ts index 9384a2b8..102bb5fd 100644 --- a/apps/server/src/core/comment/comment.service.spec.ts +++ b/apps/server/src/core/comment/comment.service.spec.ts @@ -14,6 +14,7 @@ describe('CommentService', () => { {} as any, // collaborationGateway {} as any, // generalQueue {} as any, // notificationQueue + {} as any, // auditService ); }); diff --git a/apps/server/src/core/comment/comment.service.ts b/apps/server/src/core/comment/comment.service.ts index 98244b6a..3a251b58 100644 --- a/apps/server/src/core/comment/comment.service.ts +++ b/apps/server/src/core/comment/comment.service.ts @@ -1,7 +1,10 @@ import { BadRequestException, + ConflictException, ForbiddenException, + Inject, Injectable, + InternalServerErrorException, Logger, NotFoundException, } from '@nestjs/common'; @@ -26,6 +29,11 @@ import { AuthProvenanceData, agentSourceFields, } from '../../common/decorators/auth-provenance.decorator'; +import { AuditEvent, AuditResource } from '../../common/events/audit-events'; +import { + AUDIT_SERVICE, + IAuditService, +} from '../../integrations/audit/audit.service'; @Injectable() export class CommentService { @@ -40,6 +48,7 @@ export class CommentService { private generalQueue: Queue, @InjectQueue(QueueName.NOTIFICATION_QUEUE) private notificationQueue: Queue, + @Inject(AUDIT_SERVICE) private auditService: IAuditService, ) {} async findById(commentId: string) { @@ -78,15 +87,50 @@ export class CommentService { } } + const selection = createCommentDto?.selection?.substring(0, 250) ?? null; + + // A suggested edit rewrites the exact text under an inline comment mark, so + // it is only meaningful on a top-level inline comment that carries a + // selection, and only if the suggestion actually changes that text. + let suggestedText: string | null = null; + if ( + createCommentDto.suggestedText !== undefined && + createCommentDto.suggestedText !== null + ) { + if (createCommentDto.parentCommentId) { + throw new BadRequestException( + 'A suggested edit can only be attached to a top-level comment, not a reply', + ); + } + if (!selection || selection.trim().length === 0) { + throw new BadRequestException( + 'A suggested edit requires an inline comment with a non-empty text selection', + ); + } + const trimmed = createCommentDto.suggestedText.trim(); + if (trimmed.length === 0) { + throw new BadRequestException('A suggested edit cannot be empty'); + } + // A no-op suggestion (identical to the selection) is meaningless and would + // make "apply" indistinguishable from "already applied". + if (trimmed === selection.trim()) { + throw new BadRequestException( + 'A suggested edit must differ from the selected text', + ); + } + suggestedText = trimmed; + } + const inserted = await this.commentRepo.insertComment({ pageId: page.id, content: commentContent, - selection: createCommentDto?.selection?.substring(0, 250) ?? null, + selection, type: createCommentDto.type ?? 'page', parentCommentId: createCommentDto?.parentCommentId, creatorId: user.id, workspaceId: workspaceId, spaceId: page.spaceId, + suggestedText, // Agent-edit provenance: the user stays creatorId; this only annotates the // source. Normal user requests leave the column default ('user'). ...agentSourceFields(provenance, 'createdSource', 'aiChatId'), @@ -299,6 +343,152 @@ export class CommentService { return updatedComment; } + /** + * Apply the suggested edit carried by a top-level inline comment: atomically + * replace the text under the comment mark in the collaborative document with + * the comment's suggestedText, then stamp the applied fields and auto-resolve + * the thread. The controller authorizes (validateCanEdit); this re-checks the + * comment's own state so the invariant holds regardless of caller. + */ + async applySuggestion( + comment: Comment, + user: User, + provenance?: AuthProvenanceData, + ): Promise { + // Structural guards. + if (comment.parentCommentId) { + throw new BadRequestException( + 'Only a top-level comment can carry a suggested edit', + ); + } + if (!comment.suggestedText) { + throw new BadRequestException('This comment has no suggested edit to apply'); + } + // State guards. Order matters — the already-applied check precedes the + // resolved check because an applied comment is normally also resolved. + // + // Already applied → IDEMPOTENT SUCCESS (issue #315 DoD: double-click / + // two-user race → idempotent "already applied", NOT a 409). The suggestion + // is already in the document, so do NOT call the collab gateway again. + // finalizeAppliedSuggestion re-fetches/broadcasts the same success shape as + // the applied branch and, when the thread is still open (the rare "applied + // but not resolved" crash window), self-heals it via resolveComment. + if (comment.suggestionAppliedAt) { + return this.finalizeAppliedSuggestion(comment, user, provenance); + } + // Not-yet-applied on a resolved thread → reject. The client hides the apply + // button once a thread is resolved; this is the defensive server check. + if (comment.resolvedAt) { + throw new BadRequestException( + 'Cannot apply a suggested edit on a resolved comment thread', + ); + } + + // Derive the document name the same way create()/resolveComment() do for + // the comment marks: `page.${pageId}`. + const documentName = `page.${comment.pageId}`; + + let verdict: { applied: boolean; currentText: string | null } | undefined; + try { + verdict = await this.collaborationGateway.handleYjsEvent( + 'applyCommentSuggestion', + documentName, + { + commentId: comment.id, + expectedText: comment.selection, + newText: comment.suggestedText, + user, + }, + ); + } catch (error) { + // A throwing gateway (or the phase-3 fallback failing) is a hard error — + // never silently succeed, the document may or may not have changed. + this.logger.error( + `Failed to apply suggested edit for comment ${comment.id}`, + error, + ); + throw new InternalServerErrorException('Failed to apply the suggested edit'); + } + + if (!verdict) { + // Should not happen given the phase-3 fallback; treat as a hard error + // rather than assuming success. + throw new InternalServerErrorException('Failed to apply the suggested edit'); + } + + if (verdict.applied === true) { + return this.finalizeAppliedSuggestion(comment, user, provenance); + } + + // Idempotent branch: the mutation didn't run now, but the text under the + // mark is ALREADY the suggested text (double-click, two-user race, or a + // crash between the doc mutation and the DB write). Reconcile the DB / + // resolved state and report success — do NOT 409. + if ( + verdict.applied === false && + verdict.currentText === comment.suggestedText + ) { + return this.finalizeAppliedSuggestion(comment, user, provenance); + } + + // The commented text changed since the suggestion was made. Surface the + // current text so the client can tell the user what it is now. + throw new ConflictException({ + message: + 'The commented text changed since this suggestion was made; it was not applied.', + currentText: verdict.currentText, + }); + } + + /** + * Persist the applied stamps (idempotently), auto-resolve the thread and + * broadcast + audit the applied suggestion. Shared by the applied and the + * idempotent "already-applied" branches of applySuggestion. + */ + private async finalizeAppliedSuggestion( + comment: Comment, + user: User, + provenance?: AuthProvenanceData, + ): Promise { + if (!comment.suggestionAppliedAt) { + await this.commentRepo.updateComment( + { + suggestionAppliedAt: new Date(), + suggestionAppliedById: user.id, + }, + comment.id, + ); + } + + // Auto-resolve the thread. resolveComment handles the resolve mark, its ws + // broadcast and the resolve notification. The guard above guarantees the + // thread was open when we entered, but stay defensive on re-entry. + if (!comment.resolvedAt) { + await this.resolveComment(comment, true, user, provenance); + } + + const updatedComment = await this.commentRepo.findById(comment.id, { + includeCreator: true, + includeResolvedBy: true, + }); + + this.wsService.emitCommentEvent(comment.spaceId, comment.pageId, { + operation: 'commentUpdated', + pageId: comment.pageId, + comment: updatedComment, + }); + + this.auditService.log({ + event: AuditEvent.COMMENT_SUGGESTION_APPLIED, + resourceType: AuditResource.COMMENT, + resourceId: comment.id, + spaceId: comment.spaceId, + metadata: { pageId: comment.pageId }, + }); + + return updatedComment; + } + private async queueCommentNotification( content: any, oldMentionIds: string[], diff --git a/apps/server/src/core/comment/dto/apply-suggestion.dto.ts b/apps/server/src/core/comment/dto/apply-suggestion.dto.ts new file mode 100644 index 00000000..0c93f19e --- /dev/null +++ b/apps/server/src/core/comment/dto/apply-suggestion.dto.ts @@ -0,0 +1,6 @@ +import { IsUUID } from 'class-validator'; + +export class ApplySuggestionDto { + @IsUUID() + commentId: string; +} diff --git a/apps/server/src/core/comment/dto/create-comment.dto.ts b/apps/server/src/core/comment/dto/create-comment.dto.ts index c82ae187..b556c627 100644 --- a/apps/server/src/core/comment/dto/create-comment.dto.ts +++ b/apps/server/src/core/comment/dto/create-comment.dto.ts @@ -1,4 +1,12 @@ -import { IsIn, IsJSON, IsObject, IsOptional, IsString, IsUUID } from 'class-validator'; +import { + IsIn, + IsJSON, + IsObject, + IsOptional, + IsString, + IsUUID, + MaxLength, +} from 'class-validator'; import { z } from 'zod'; const yjsIdSchema = z.object({ @@ -43,4 +51,12 @@ export class CreateCommentDto { anchor: any; head: any; }; + + // Optional suggested replacement for the selected text (a "suggested edit"). + // Only valid on a top-level inline comment that carries a non-empty selection; + // enforced in CommentService.create. + @IsOptional() + @IsString() + @MaxLength(2000) + suggestedText?: string; }