Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 20 additions & 14 deletions backend/controllers/checklist/checklist-comments-controllers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,20 @@ import { createCommentSchema } from '../../validations/checklist/checklist-schem
import { logger } from '../../config/logger.js'
import { fetchStoredFile, sendPrivateFile } from '../../services/uploads/private-files.js'

// Answers a refused delete; returns true when it did
function refuseRemove(
res: Response,
result: commentsService.RemoveResult,
what: 'comentario' | 'adjunto'
): boolean {
if (result === 'deleted') return false
if (result === 'not_found') res.status(404).json({ error: `No existe ese ${what}` })
else if (result === 'closed')
res.status(409).json({ error: 'El día está cerrado: solo un administrador puede borrar' })
else res.status(403).json({ error: `No tienes permiso para eliminar este ${what}` })
return true
}

function isAdmin(req: Request): boolean {
return req.user?.role?.toLowerCase() === 'admin'
}
Expand Down Expand Up @@ -45,17 +59,13 @@ export async function addCommentController(req: Request, res: Response): Promise
// DELETE /api/checklists/:id/steps/:stepId/comments/:commentId
export async function deleteCommentController(req: Request, res: Response): Promise<void> {
try {
const { id, commentId } = req.params
const deleted = await commentsService.removeComment(
id,
const { commentId } = req.params
const result = await commentsService.removeComment(
Number(commentId),
req.user!.id,
isAdmin(req)
)
if (!deleted) {
res.status(403).json({ error: 'No tienes permiso para eliminar este comentario' })
return
}
if (refuseRemove(res, result, 'comentario')) return
res.json({ success: true })
} catch (err) {
logger.error({ err }, '[checklist] deleteComment')
Expand Down Expand Up @@ -113,17 +123,13 @@ export async function addAttachmentController(req: Request, res: Response): Prom
// DELETE /api/checklists/:id/steps/:stepId/attachments/:attachmentId
export async function deleteAttachmentController(req: Request, res: Response): Promise<void> {
try {
const { id, attachmentId } = req.params
const deleted = await commentsService.removeAttachment(
id,
const { attachmentId } = req.params
const result = await commentsService.removeAttachment(
Number(attachmentId),
req.user!.id,
isAdmin(req)
)
if (!deleted) {
res.status(403).json({ error: 'No tienes permiso para eliminar este adjunto' })
return
}
if (refuseRemove(res, result, 'adjunto')) return
res.json({ success: true })
} catch (err) {
logger.error({ err }, '[checklist] deleteAttachment')
Expand Down
39 changes: 26 additions & 13 deletions backend/repositories/checklist/checklist-comments.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,27 @@ export async function createComment(
return rows[0]
}

export async function deleteComment(
commentId: number,
userId: string,
isAdmin: boolean
): Promise<boolean> {
// The comment with its run, to check who may delete it and whether its day is closed
export async function findComment(
commentId: number
): Promise<{ user_id: string; run_id: number; run_closed: boolean } | null> {
const [rows] = await db.execute<RowDataPacket[]>(
`SELECT user_id FROM checklist_step_comments WHERE id = ?`,
`SELECT c.user_id, c.run_id, r.reset_at IS NOT NULL AS run_closed
FROM checklist_step_comments c
JOIN checklist_runs r ON r.id = c.run_id
WHERE c.id = ?`,
[commentId]
)
if (!rows[0]) return false
if (!isAdmin && rows[0].user_id !== userId) return false
if (!rows[0]) return null
return {
user_id: rows[0].user_id,
run_id: rows[0].run_id,
run_closed: Boolean(rows[0].run_closed),
}
}

export async function deleteComment(commentId: number): Promise<void> {
await db.execute(`DELETE FROM checklist_step_comments WHERE id = ?`, [commentId])
return true
}

// ── Attachments ───────────────────────────────────────────
Expand Down Expand Up @@ -113,10 +121,15 @@ export async function createAttachment(
return rows[0]
}

export async function findAttachment(attachmentId: number): Promise<Attachment | null> {
const [rows] = await db.execute<Attachment[]>(
`SELECT a.*, u.username FROM checklist_step_attachments a
JOIN users u ON u.id = a.user_id WHERE a.id = ?`,
export async function findAttachment(
attachmentId: number
): Promise<(Attachment & { run_closed: number }) | null> {
const [rows] = await db.execute<(Attachment & { run_closed: number })[]>(
`SELECT a.*, u.username, r.reset_at IS NOT NULL AS run_closed
FROM checklist_step_attachments a
JOIN users u ON u.id = a.user_id
JOIN checklist_runs r ON r.id = a.run_id
WHERE a.id = ?`,
[attachmentId]
)
return rows[0] ?? null
Expand Down
47 changes: 33 additions & 14 deletions backend/services/checklist/checklist-comments.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,21 @@ async function resolveRun(checklistId: string): Promise<{ id: number }> {
return run
}

// Result of deleting a comment or an attachment. A closed day (its run has
// reset_at) is history: since 2026-10-11 only admin may delete there
export type RemoveResult = 'deleted' | 'not_found' | 'forbidden' | 'closed'

function canRemove(
item: { user_id: string; run_closed: boolean | number },
userId: string,
isAdmin: boolean
): RemoveResult | null {
if (isAdmin) return null
if (item.user_id !== userId) return 'forbidden'
if (item.run_closed) return 'closed'
return null
}

// ── Comments ──────────────────────────────────────────────

export async function getComments(checklistId: string, stepId: string): Promise<Comment[]> {
Expand All @@ -31,16 +46,20 @@ export async function addComment(
}

export async function removeComment(
checklistId: string,
commentId: number,
userId: string,
isAdmin: boolean
): Promise<boolean> {
const run = await resolveRun(checklistId)
const deleted = await commentsRepo.deleteComment(commentId, userId, isAdmin)
if (deleted)
await checklistRepo.logEvent(run.id, null, userId, 'comment', { deleted_comment_id: commentId })
return deleted
): Promise<RemoveResult> {
const comment = await commentsRepo.findComment(commentId)
if (!comment) return 'not_found'
const refused = canRemove(comment, userId, isAdmin)
if (refused) return refused
await commentsRepo.deleteComment(commentId)
// Logged in the comment's own run, not in today's
await checklistRepo.logEvent(comment.run_id, null, userId, 'comment', {
deleted_comment_id: commentId,
})
return 'deleted'
}

// ── Attachments ───────────────────────────────────────────
Expand Down Expand Up @@ -78,21 +97,21 @@ export async function addAttachment(
}

export async function removeAttachment(
checklistId: string,
attachmentId: number,
userId: string,
isAdmin: boolean
): Promise<boolean> {
): Promise<RemoveResult> {
const attachment = await commentsRepo.findAttachment(attachmentId)
if (!attachment) return false
if (!isAdmin && attachment.user_id !== userId) return false
if (!attachment) return 'not_found'
const refused = canRemove(attachment, userId, isAdmin)
if (refused) return refused
await CloudinaryService.deleteImage(attachment.public_id)
await commentsRepo.deleteAttachment(attachmentId)
const run = await resolveRun(checklistId)
await checklistRepo.logEvent(run.id, null, userId, 'attach', {
// Logged in the attachment's own run, not in today's
await checklistRepo.logEvent(attachment.run_id, null, userId, 'attach', {
deleted_attachment_id: attachmentId,
})
return true
return 'deleted'
}

// Stored Cloudinary URL of an attachment, for the server to serve it
Expand Down
67 changes: 67 additions & 0 deletions backend/tests/checklist/closed-day-deletes.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// tests/checklist/closed-day-deletes.test.ts
// Regression tests: the author of a comment or a photo could delete it from a
// closed day (rewriting its history), and the deletion was logged in today's
// run as if it were a new one. Decided on 2026-10-11: on a closed day only
// admin deletes; the event goes to the item's own run.

import { describe, it, expect, vi, beforeEach } from 'vitest'

vi.mock('../../repositories/checklist/checklist-comments.repository.js', () => ({
findComment: vi.fn(),
deleteComment: vi.fn(),
findAttachment: vi.fn(),
deleteAttachment: vi.fn(),
}))
vi.mock('../../repositories/checklist/checklist-repository.js', () => ({
logEvent: vi.fn(),
getOrCreateRun: vi.fn(async () => ({ id: 999 })), // today's run: must not be used
}))
vi.mock('../../services/blacklist/cloudinary-service.js', () => ({
CloudinaryService: { deleteImage: vi.fn() },
}))

const service = await import('../../services/checklist/checklist-comments.service.js')
const repo = await import('../../repositories/checklist/checklist-comments.repository.js')
const checklistRepo = await import('../../repositories/checklist/checklist-repository.js')

const closed = { user_id: 'author', run_id: 7, run_closed: true }
const open = { user_id: 'author', run_id: 8, run_closed: false }

describe('deleting comments and photos', () => {
beforeEach(() => vi.clearAllMocks())

it('the author cannot delete from a closed day', async () => {
vi.mocked(repo.findComment).mockResolvedValue(closed)
vi.mocked(repo.findAttachment).mockResolvedValue({ ...closed, public_id: 'p' } as never)

expect(await service.removeComment(1, 'author', false)).toBe('closed')
expect(await service.removeAttachment(1, 'author', false)).toBe('closed')
expect(repo.deleteComment).not.toHaveBeenCalled()
expect(repo.deleteAttachment).not.toHaveBeenCalled()
})

it('admin can, and the event goes to the closed run', async () => {
vi.mocked(repo.findComment).mockResolvedValue(closed)

expect(await service.removeComment(1, 'admin-id', true)).toBe('deleted')
expect(checklistRepo.logEvent).toHaveBeenCalledWith(7, null, 'admin-id', 'comment', {
deleted_comment_id: 1,
})
expect(checklistRepo.getOrCreateRun).not.toHaveBeenCalled()
})

it('on an open day the author deletes, someone else cannot', async () => {
vi.mocked(repo.findComment).mockResolvedValue(open)

expect(await service.removeComment(1, 'someone-else', false)).toBe('forbidden')
expect(await service.removeComment(1, 'author', false)).toBe('deleted')
expect(checklistRepo.logEvent).toHaveBeenCalledWith(8, null, 'author', 'comment', {
deleted_comment_id: 1,
})
})

it('an unknown id is not found', async () => {
vi.mocked(repo.findComment).mockResolvedValue(null)
expect(await service.removeComment(1, 'author', false)).toBe('not_found')
})
})
Loading