refactor(board): atomic delete + query limit + composable cleanup
Some checks failed
Test / vitest (push) Failing after 7m17s
Test / playwright (push) Has been skipped
Test / visual (push) Has been skipped
Test / Notify on failure (push) Successful in 1s

Delete uses findOneAndDelete with author match (no TOCTOU window);
existence check only runs on miss to distinguish 403 vs 404. Posts
list capped at 200. Drop unused resolveTagChannel and refreshParams;
route slack URL building through the composable's slackUrl helper.
This commit is contained in:
Jennie Robinson Faber 2026-04-15 12:47:53 +01:00
parent d1a1484daf
commit 28040f44f4
7 changed files with 30 additions and 54 deletions

View file

@ -86,6 +86,8 @@ const props = defineProps({
defineEmits(['edit', 'delete', 'confirm-delete', 'cancel-delete']) defineEmits(['edit', 'delete', 'confirm-delete', 'cancel-delete'])
const { slackUrl } = useBoardChannels()
const capitalizeAvatar = (str) => { const capitalizeAvatar = (str) => {
if (str.toLowerCase() === 'wtf') return 'WTF' if (str.toLowerCase() === 'wtf') return 'WTF'
return str return str
@ -148,7 +150,7 @@ const slackLinks = computed(() => {
.map((c) => ({ .map((c) => ({
id: c.slackChannelId, id: c.slackChannelId,
name: c.slackChannelName || c.name || c.slackChannelId, name: c.slackChannelName || c.name || c.slackChannelId,
url: `https://gammaspace.slack.com/archives/${c.slackChannelId}`, url: slackUrl(c.slackChannelId),
})) }))
}) })
</script> </script>

View file

@ -1,7 +1,3 @@
/**
* Board Channels Composable
* Shared state + helpers for mapping board tags to Slack channels.
*/
export function useBoardChannels() { export function useBoardChannels() {
const channels = useState('board.channels', () => []) const channels = useState('board.channels', () => [])
@ -11,15 +7,6 @@ export function useBoardChannels() {
return channels.value return channels.value
} }
function resolveTagChannel(tagSlugs = []) {
if (!tagSlugs?.length) return null
return (
channels.value.find((channel) =>
(channel.tagSlugs || []).some((slug) => tagSlugs.includes(slug))
) || null
)
}
function slackUrl(channelId) { function slackUrl(channelId) {
return `https://gammaspace.slack.com/archives/${channelId}` return `https://gammaspace.slack.com/archives/${channelId}`
} }
@ -27,7 +14,6 @@ export function useBoardChannels() {
return { return {
channels: readonly(channels), channels: readonly(channels),
fetchChannels, fetchChannels,
resolveTagChannel,
slackUrl, slackUrl,
} }
} }

View file

@ -1,7 +1,3 @@
/**
* Board Posts Composable
* Shared state + CRUD for board posts.
*/
export function useBoardPosts() { export function useBoardPosts() {
const posts = useState('board.posts', () => []) const posts = useState('board.posts', () => [])
const loading = useState('board.loading', () => false) const loading = useState('board.loading', () => false)
@ -17,29 +13,29 @@ export function useBoardPosts() {
} }
} }
async function createPost(body, refreshParams = {}) { async function createPost(body) {
const created = await $fetch('/api/board/posts', { const created = await $fetch('/api/board/posts', {
method: 'POST', method: 'POST',
body, body,
}) })
await fetchPosts(refreshParams) await fetchPosts()
return created return created
} }
async function updatePost(id, body, refreshParams = {}) { async function updatePost(id, body) {
const updated = await $fetch(`/api/board/posts/${id}`, { const updated = await $fetch(`/api/board/posts/${id}`, {
method: 'PATCH', method: 'PATCH',
body, body,
}) })
await fetchPosts(refreshParams) await fetchPosts()
return updated return updated
} }
async function deletePost(id, refreshParams = {}) { async function deletePost(id) {
const result = await $fetch(`/api/board/posts/${id}`, { const result = await $fetch(`/api/board/posts/${id}`, {
method: 'DELETE', method: 'DELETE',
}) })
await fetchPosts(refreshParams) await fetchPosts()
return result return result
} }

View file

@ -1,13 +1,11 @@
<template> <template>
<PageShell title="Bulletin Board" :subtitle="pageSubtitle"> <PageShell title="Bulletin Board" :subtitle="pageSubtitle">
<!-- Action bar -->
<div class="action-bar"> <div class="action-bar">
<button type="button" class="new-post-btn" @click="openNewForm"> <button type="button" class="new-post-btn" @click="openNewForm">
+ New Post + New Post
</button> </button>
</div> </div>
<!-- Tags Drawer Toggle -->
<div v-if="cooperativeTags.length > 0" class="tags-drawer-toggle"> <div v-if="cooperativeTags.length > 0" class="tags-drawer-toggle">
<button type="button" class="drawer-btn" @click="showTagsDrawer = !showTagsDrawer"> <button type="button" class="drawer-btn" @click="showTagsDrawer = !showTagsDrawer">
Tags... Tags...
@ -15,7 +13,6 @@
</button> </button>
</div> </div>
<!-- Tags Drawer -->
<div v-if="showTagsDrawer && cooperativeTags.length > 0" class="tags-drawer"> <div v-if="showTagsDrawer && cooperativeTags.length > 0" class="tags-drawer">
<div class="skills-bar"> <div class="skills-bar">
<span class="tag-label">Filter:</span> <span class="tag-label">Filter:</span>
@ -40,7 +37,6 @@
</div> </div>
</div> </div>
<!-- Inline form -->
<div v-if="showForm" class="form-wrapper"> <div v-if="showForm" class="form-wrapper">
<BoardPostForm <BoardPostForm
:post="editingPost" :post="editingPost"
@ -50,7 +46,6 @@
/> />
</div> </div>
<!-- Content -->
<ClientOnly> <ClientOnly>
<div v-if="loading" class="loading-state"> <div v-if="loading" class="loading-state">
<p>Loading board...</p> <p>Loading board...</p>

View file

@ -17,6 +17,7 @@ export default defineEventHandler(async (event) => {
const posts = await BoardPost.find(dbQuery) const posts = await BoardPost.find(dbQuery)
.sort({ createdAt: -1 }) .sort({ createdAt: -1 })
.limit(200)
.populate('author', 'name avatar circle board.slackHandle') .populate('author', 'name avatar circle board.slackHandle')
.lean() .lean()

View file

@ -5,16 +5,15 @@ export default defineEventHandler(async (event) => {
const member = await requireAuth(event) const member = await requireAuth(event)
const id = getRouterParam(event, 'id') const id = getRouterParam(event, 'id')
const post = await BoardPost.findById(id) const deleted = await BoardPost.findOneAndDelete({ _id: id, author: member._id })
if (!post) {
throw createError({ statusCode: 404, statusMessage: 'Post not found' })
}
if (post.author.toString() !== member._id.toString()) { if (!deleted) {
throw createError({ statusCode: 403, statusMessage: 'Not authorized to delete this post' }) const exists = await BoardPost.exists({ _id: id })
throw createError({
statusCode: exists ? 403 : 404,
statusMessage: exists ? 'Not authorized to delete this post' : 'Post not found',
})
} }
await post.deleteOne()
return { success: true } return { success: true }
}) })

View file

@ -4,9 +4,11 @@ import { setResponseStatus } from 'h3'
vi.stubGlobal('setResponseStatus', setResponseStatus) vi.stubGlobal('setResponseStatus', setResponseStatus)
// --- Mocks --- // --- Mocks ---
const { mockFind, mockFindById, mockSaveInstance } = vi.hoisted(() => ({ const { mockFind, mockFindById, mockFindOneAndDelete, mockExists, mockSaveInstance } = vi.hoisted(() => ({
mockFind: vi.fn(), mockFind: vi.fn(),
mockFindById: vi.fn(), mockFindById: vi.fn(),
mockFindOneAndDelete: vi.fn(),
mockExists: vi.fn(),
mockSaveInstance: vi.fn(), mockSaveInstance: vi.fn(),
})) }))
@ -25,6 +27,8 @@ vi.mock('../../../server/models/boardPost.js', () => {
} }
BoardPost.find = mockFind BoardPost.find = mockFind
BoardPost.findById = mockFindById BoardPost.findById = mockFindById
BoardPost.findOneAndDelete = mockFindOneAndDelete
BoardPost.exists = mockExists
return { default: BoardPost } return { default: BoardPost }
}) })
@ -63,6 +67,7 @@ const MEMBER_ID = 'member-abc'
function buildFindChain(result) { function buildFindChain(result) {
const chain = { const chain = {
sort: vi.fn().mockReturnThis(), sort: vi.fn().mockReturnThis(),
limit: vi.fn().mockReturnThis(),
populate: vi.fn().mockReturnThis(), populate: vi.fn().mockReturnThis(),
lean: vi.fn().mockResolvedValue(result), lean: vi.fn().mockResolvedValue(result),
} }
@ -276,39 +281,31 @@ describe('DELETE /api/board/posts/[id]', () => {
}) })
it('deletes own post', async () => { it('deletes own post', async () => {
const deleteOne = vi.fn().mockResolvedValue({}) mockFindOneAndDelete.mockResolvedValue({ _id: 'post-1' })
mockFindById.mockResolvedValue({
_id: 'post-1',
author: { toString: () => MEMBER_ID },
deleteOne,
})
const event = createMockEvent({ method: 'DELETE', path: '/api/board/posts/post-1' }) const event = createMockEvent({ method: 'DELETE', path: '/api/board/posts/post-1' })
event.context = { params: { id: 'post-1' } } event.context = { params: { id: 'post-1' } }
const result = await deleteHandler(event) const result = await deleteHandler(event)
expect(deleteOne).toHaveBeenCalled() expect(mockFindOneAndDelete).toHaveBeenCalledWith({ _id: 'post-1', author: MEMBER_ID })
expect(result).toEqual({ success: true }) expect(result).toEqual({ success: true })
}) })
it('rejects deleting another members post with 403', async () => { it('rejects deleting another members post with 403', async () => {
const deleteOne = vi.fn() mockFindOneAndDelete.mockResolvedValue(null)
mockFindById.mockResolvedValue({ mockExists.mockResolvedValue({ _id: 'post-1' })
_id: 'post-1',
author: { toString: () => 'someone-else' },
deleteOne,
})
const event = createMockEvent({ method: 'DELETE', path: '/api/board/posts/post-1' }) const event = createMockEvent({ method: 'DELETE', path: '/api/board/posts/post-1' })
event.context = { params: { id: 'post-1' } } event.context = { params: { id: 'post-1' } }
await expect(deleteHandler(event)).rejects.toMatchObject({ statusCode: 403 }) await expect(deleteHandler(event)).rejects.toMatchObject({ statusCode: 403 })
expect(deleteOne).not.toHaveBeenCalled()
}) })
it('returns 404 when post not found', async () => { it('returns 404 when post not found', async () => {
mockFindById.mockResolvedValue(null) mockFindOneAndDelete.mockResolvedValue(null)
mockExists.mockResolvedValue(null)
const event = createMockEvent({ method: 'DELETE', path: '/api/board/posts/missing' }) const event = createMockEvent({ method: 'DELETE', path: '/api/board/posts/missing' })
event.context = { params: { id: 'missing' } } event.context = { params: { id: 'missing' } }