Skip to content

Commit d9bc6ef

Browse files
committed
fix(help-center): address codex/code-quality review on PR #132
- getCategoryDepth in category-tree.ts: the \`while (current && ...)\` had a useless truthiness check — \`current\` is provably defined after the early \`!start\` return. Drop the redundant guard and type \`current\` as \`T\` instead of \`T | undefined\`. (github-code-quality) - Article canonical URL was built as \`\${baseUrl}/\${categorySlug}/\${articleSlug}\`, missing the \`/hc\` prefix — wrong for og:url and rel=canonical after the help center moved under the portal layout. Prepend \`/hc\`. (codex P2) - restoreCategory now refuses to restore a child whose parent is still deleted, raising a new \`PARENT_DELETED\` ValidationError. Without this, restoring a child first left it pointing at a hidden parent and the category silently dropped out of the sidebar tree (which only roots from parentId === null). Admins must restore the ancestor chain from the top down. New tests cover both the refusal path and the happy path where the parent is already active. (codex P2)
1 parent 8ef7a55 commit d9bc6ef

4 files changed

Lines changed: 61 additions & 3 deletions

File tree

apps/web/src/lib/server/domains/help-center/__tests__/help-center-service.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,6 +1125,47 @@ describe('restoreCategory', () => {
11251125
code: 'RESTORE_EXPIRED',
11261126
})
11271127
})
1128+
1129+
it('refuses to restore a child under a still-deleted parent', async () => {
1130+
const recentDeletedAt = new Date(Date.now() - 5 * 24 * 60 * 60 * 1000)
1131+
mockCategoryFindFirst.mockResolvedValueOnce({
1132+
id: 'category_child' as HelpCenterCategoryId,
1133+
slug: 'child',
1134+
name: 'Child Category',
1135+
parentId: 'category_parent' as HelpCenterCategoryId,
1136+
deletedAt: recentDeletedAt,
1137+
createdAt: new Date('2024-01-01'),
1138+
updatedAt: new Date('2024-01-01'),
1139+
})
1140+
mockCategoryFindFirst.mockResolvedValueOnce({
1141+
id: 'category_parent' as HelpCenterCategoryId,
1142+
deletedAt: recentDeletedAt,
1143+
})
1144+
await expect(restoreCategory('category_child' as HelpCenterCategoryId)).rejects.toMatchObject({
1145+
code: 'PARENT_DELETED',
1146+
})
1147+
})
1148+
1149+
it('restores a child when its parent is already active', async () => {
1150+
const recentDeletedAt = new Date(Date.now() - 5 * 24 * 60 * 60 * 1000)
1151+
mockCategoryFindFirst.mockResolvedValueOnce({
1152+
id: 'category_child' as HelpCenterCategoryId,
1153+
slug: 'child',
1154+
name: 'Child Category',
1155+
parentId: 'category_parent' as HelpCenterCategoryId,
1156+
deletedAt: recentDeletedAt,
1157+
createdAt: new Date('2024-01-01'),
1158+
updatedAt: new Date('2024-01-01'),
1159+
})
1160+
mockCategoryFindFirst.mockResolvedValueOnce({
1161+
id: 'category_parent' as HelpCenterCategoryId,
1162+
deletedAt: null,
1163+
})
1164+
const { db } = await import('@/lib/server/db')
1165+
vi.mocked(db.update).mockReturnValueOnce(makeRestoredCategoryChain() as never)
1166+
const result = await restoreCategory('category_child' as HelpCenterCategoryId)
1167+
expect(result.deletedAt).toBeNull()
1168+
})
11281169
})
11291170

11301171
// ============================================================================

apps/web/src/lib/server/domains/help-center/category-tree.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ export function getCategoryDepth<T extends CategoryLike>(flat: T[], id: string):
7070
const start = byId.get(id)
7171
if (!start) return 0
7272
let depth = 0
73-
let current: T | undefined = start
73+
let current: T = start
7474
const seen = new Set<string>()
75-
while (current && current.parentId !== null) {
75+
while (current.parentId !== null) {
7676
if (seen.has(current.id)) break
7777
seen.add(current.id)
7878
const parent = byId.get(current.parentId)

apps/web/src/lib/server/domains/help-center/help-center.service.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,23 @@ export async function restoreCategory(id: HelpCenterCategoryId): Promise<HelpCen
336336
)
337337
}
338338

339+
// Refuse to restore a child under a still-deleted parent — it would keep
340+
// a non-null parentId to a hidden ancestor and drop out of the active
341+
// sidebar tree (which only roots from parentId === null). Admins must
342+
// restore the ancestor chain first.
343+
if (category.parentId) {
344+
const parent = await db.query.helpCenterCategories.findFirst({
345+
where: eq(helpCenterCategories.id, category.parentId),
346+
columns: { id: true, deletedAt: true },
347+
})
348+
if (parent?.deletedAt) {
349+
throw new ValidationError(
350+
'PARENT_DELETED',
351+
'Restore the parent category first, then restore this one.'
352+
)
353+
}
354+
}
355+
339356
const [restored] = await db
340357
.update(helpCenterCategories)
341358
.set({ deletedAt: null, updatedAt: new Date() })

apps/web/src/routes/_portal/hc/$categorySlug/$articleSlug.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export const Route = createFileRoute('/_portal/hc/$categorySlug/$articleSlug')({
4646

4747
const baseUrl =
4848
((portalMatch?.context as Record<string, any> | undefined)?.baseUrl as string) ?? ''
49-
const canonicalUrl = `${baseUrl}/${params.categorySlug}/${params.articleSlug}`
49+
const canonicalUrl = `${baseUrl}/hc/${params.categorySlug}/${params.articleSlug}`
5050

5151
return {
5252
meta: [

0 commit comments

Comments
 (0)