From e484c13f21db4f6beb72f0fff6734053fac742e3 Mon Sep 17 00:00:00 2001 From: iPLAYCAFE Date: Wed, 29 Jul 2026 16:44:18 +0700 Subject: [PATCH] test: align Help Center admin-only fixtures The ipc.21 baseline materialized fail-closed admin-only reads while legacy mocks and route expectations retained the previous call shape. Keep Provider runtime and the SDK adapter range unchanged while restoring deterministic CI evidence. --- .../help-center-article.query.test.ts | 3 + .../help-center-article.service.test.ts | 13 +- .../help-center-category.service.test.ts | 31 ++- .../v1/help-center/__tests__/articles.test.ts | 204 ++++++++++++++++-- 4 files changed, 236 insertions(+), 15 deletions(-) diff --git a/apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.query.test.ts b/apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.query.test.ts index 330990a08..2a0c41de7 100644 --- a/apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.query.test.ts +++ b/apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.query.test.ts @@ -75,6 +75,9 @@ let listPublicArticlesForCategory: typeof import('../help-center.article.query') beforeEach(async () => { vi.clearAllMocks() + mockArticleFindFirst.mockReset().mockResolvedValue(null) + mockArticleFindMany.mockReset().mockResolvedValue([]) + mockCategoryFindMany.mockReset().mockResolvedValue([]) const mod = await import('../help-center.article.query') listArticles = mod.listArticles diff --git a/apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.service.test.ts b/apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.service.test.ts index d7707143e..a36752112 100644 --- a/apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.service.test.ts +++ b/apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.service.test.ts @@ -39,6 +39,7 @@ function createUpdateChain() { } const mockCategoryFindFirst = vi.fn() +const mockCategoryFindMany = vi.fn() const mockArticleFindFirst = vi.fn() const mockFeedbackFindFirst = vi.fn() const mockPrincipalFindFirst = vi.fn() @@ -48,6 +49,7 @@ vi.mock('@/lib/server/db', () => ({ query: { helpCenterCategories: { findFirst: (...args: unknown[]) => mockCategoryFindFirst(...args), + findMany: (...args: unknown[]) => mockCategoryFindMany(...args), }, helpCenterArticles: { findFirst: (...args: unknown[]) => mockArticleFindFirst(...args), @@ -136,6 +138,11 @@ let recordArticleFeedback: typeof import('../help-center.article.service').recor beforeEach(async () => { vi.clearAllMocks() + mockCategoryFindFirst.mockReset().mockResolvedValue(null) + mockCategoryFindMany.mockReset().mockResolvedValue([]) + mockArticleFindFirst.mockReset().mockResolvedValue(null) + mockFeedbackFindFirst.mockReset().mockResolvedValue(null) + mockPrincipalFindFirst.mockReset().mockResolvedValue(null) insertValuesCalls.length = 0 updateSetCalls.length = 0 updateWhereCalls.length = 0 @@ -372,7 +379,7 @@ describe('createArticle slug generation (#285)', () => { updatedAt: new Date(), }, ]) - vi.mocked(db.insert).mockReturnValue(chain as never) + vi.mocked(db.insert).mockReturnValueOnce(chain as never) mockCategoryFindFirst.mockResolvedValue({ id: 'category_1', slug: 'getting-started', @@ -676,6 +683,10 @@ describe('updateArticle with position and description', () => { }) describe('recordArticleFeedback', () => { + beforeEach(() => { + mockArticleFindFirst.mockResolvedValue({ categoryId: 'category_1' }) + }) + it('inserts new feedback when no existing feedback', async () => { mockFeedbackFindFirst.mockResolvedValue(null) diff --git a/apps/web/src/lib/server/domains/help-center/__tests__/help-center-category.service.test.ts b/apps/web/src/lib/server/domains/help-center/__tests__/help-center-category.service.test.ts index 4487ad442..5fa7e9ad3 100644 --- a/apps/web/src/lib/server/domains/help-center/__tests__/help-center-category.service.test.ts +++ b/apps/web/src/lib/server/domains/help-center/__tests__/help-center-category.service.test.ts @@ -57,6 +57,23 @@ const mockCategoryFindFirst = vi.fn() const mockCategoryFindMany = vi.fn() const mockSelectFrom = vi.fn() +function mockExistingCategory(id: string, parentId: string | null = null) { + mockCategoryFindFirst.mockResolvedValueOnce({ + id, + slug: 'existing', + name: 'Existing', + description: null, + isPublic: true, + adminOnly: false, + position: 0, + parentId, + icon: null, + createdAt: new Date('2024-01-01'), + updatedAt: new Date('2024-01-01'), + deletedAt: null, + }) +} + vi.mock('@/lib/server/db', () => ({ db: { query: { @@ -126,6 +143,9 @@ let restoreCategory: typeof import('../help-center.category.service').restoreCat beforeEach(async () => { vi.clearAllMocks() + mockCategoryFindFirst.mockReset().mockResolvedValue(null) + mockCategoryFindMany.mockReset().mockResolvedValue([]) + mockSelectFrom.mockReset() insertValuesCalls.length = 0 updateSetCalls.length = 0 updateWhereCalls.length = 0 @@ -434,11 +454,13 @@ describe('createCategory slug generation (#285)', () => { describe('updateCategory slug generation (#285)', () => { it('falls back to a generic slug when an explicit empty slug is given', async () => { + mockExistingCategory('category_1') await updateCategory('category_1' as HelpCenterCategoryId, { slug: '' }) expect((updateSetCalls[0][0] as Record).slug).toBe('category') }) it('disambiguates an explicit slug that collides with another category', async () => { + mockExistingCategory('category_1') mockCategoryFindFirst .mockResolvedValueOnce({ id: 'category_other' }) .mockResolvedValueOnce(null) @@ -447,6 +469,7 @@ describe('updateCategory slug generation (#285)', () => { }) it('keeps an explicit slug that only collides with the same category', async () => { + mockExistingCategory('category_1') mockCategoryFindFirst.mockResolvedValueOnce({ id: 'category_1' }) await updateCategory('category_1' as HelpCenterCategoryId, { slug: 'faq' }) expect((updateSetCalls[0][0] as Record).slug).toBe('faq') @@ -533,6 +556,7 @@ describe('createCategory with parentId and icon', () => { describe('updateCategory with parentId and icon', () => { it('passes parentId and icon in the update set', async () => { + mockExistingCategory('category_1') mockCategoryFindMany.mockResolvedValue([ { id: 'category_parent1', parentId: null }, { id: 'category_1', parentId: null }, @@ -548,6 +572,7 @@ describe('updateCategory with parentId and icon', () => { }) it('allows clearing parentId and icon by passing null', async () => { + mockExistingCategory('category_1') mockCategoryFindMany.mockResolvedValue([{ id: 'category_1', parentId: null }]) await updateCategory('category_1' as HelpCenterCategoryId, { parentId: null, @@ -624,6 +649,7 @@ describe('createCategory hierarchy validation', () => { describe('updateCategory hierarchy validation', () => { it('rejects moving a category under itself', async () => { + mockExistingCategory('a') mockCategoryFindMany.mockResolvedValue([ { id: 'a', parentId: null }, { id: 'b', parentId: 'a' }, @@ -634,6 +660,7 @@ describe('updateCategory hierarchy validation', () => { }) it('rejects moving a category under its own descendant', async () => { + mockExistingCategory('a') mockCategoryFindMany.mockResolvedValue([ { id: 'a', parentId: null }, { id: 'b', parentId: 'a' }, @@ -645,6 +672,7 @@ describe('updateCategory hierarchy validation', () => { }) it('rejects moving a subtree such that the deepest leaf would exceed MAX_CATEGORY_DEPTH', async () => { + mockExistingCategory('b', 'a') mockCategoryFindMany.mockResolvedValue([ { id: 'a', parentId: null }, { id: 'b', parentId: 'a' }, @@ -658,6 +686,7 @@ describe('updateCategory hierarchy validation', () => { }) it('allows setting parentId to null (promoting to top-level)', async () => { + mockExistingCategory('b', 'a') mockCategoryFindMany.mockResolvedValue([ { id: 'a', parentId: null }, { id: 'b', parentId: 'a' }, @@ -823,7 +852,7 @@ describe('listCategories with showDeleted option', () => { const result = await listCategories({ showDeleted: true }) expect(result).toHaveLength(1) expect(result[0].name).toBe('Deleted Category') - expect(mockCategoryFindMany).toHaveBeenCalledTimes(1) + expect(mockCategoryFindMany).toHaveBeenCalledTimes(2) }) it('returns 0 article count for deleted categories with no deleted articles', async () => { diff --git a/apps/web/src/routes/api/v1/help-center/__tests__/articles.test.ts b/apps/web/src/routes/api/v1/help-center/__tests__/articles.test.ts index 1e2597d35..d49add5b9 100644 --- a/apps/web/src/routes/api/v1/help-center/__tests__/articles.test.ts +++ b/apps/web/src/routes/api/v1/help-center/__tests__/articles.test.ts @@ -59,12 +59,16 @@ import { Route as ArticlesListRoute } from '../articles/index' import { Route as ArticleDetailRoute } from '../articles/$articleId' import { Route as ArticleFeedbackRoute } from '../articles/$articleId.feedback' -type MockedHandler = (ctx: { request: Request; params?: Record }) => Promise +type MockedHandler = (ctx: { + request: Request + params?: Record +}) => Promise type MockedRouteShape = { options: { server: { handlers: Record } } } const listHandlers = (ArticlesListRoute as unknown as MockedRouteShape).options.server.handlers const detailHandlers = (ArticleDetailRoute as unknown as MockedRouteShape).options.server.handlers -const feedbackHandlers = (ArticleFeedbackRoute as unknown as MockedRouteShape).options.server.handlers +const feedbackHandlers = (ArticleFeedbackRoute as unknown as MockedRouteShape).options.server + .handlers // --- Helpers --- @@ -93,6 +97,11 @@ const mockAuthContext: ApiAuthContext = { importMode: false, } +const mockMemberAuthContext: ApiAuthContext = { + ...mockAuthContext, + role: 'member', +} + const mockArticle: HelpCenterArticleWithCategory = { id: 'article_1' as HelpCenterArticleId, categoryId: 'category_1' as HelpCenterCategoryId, @@ -110,7 +119,11 @@ const mockArticle: HelpCenterArticleWithCategory = { createdAt: new Date('2026-01-01'), updatedAt: new Date('2026-01-10'), deletedAt: null, - category: { id: 'category_1' as HelpCenterCategoryId, slug: 'getting-started', name: 'Getting Started' }, + category: { + id: 'category_1' as HelpCenterCategoryId, + slug: 'getting-started', + name: 'Getting Started', + }, author: { id: 'principal_1' as PrincipalId, name: 'Admin', avatarUrl: null }, } @@ -162,7 +175,22 @@ describe('GET /api/v1/help-center/articles', () => { search: 'hello', cursor: undefined, limit: 20, + includeAdminOnly: true, + }) + }) + + it('excludes admin-only articles for member-role keys', async () => { + vi.mocked(withApiKeyAuth).mockResolvedValue(mockMemberAuthContext) + vi.mocked(listArticles).mockResolvedValue({ + items: [], + nextCursor: null, + hasMore: false, }) + + const request = createRequest('GET', 'http://localhost/api/v1/help-center/articles') + await listHandlers.GET({ request }) + + expect(listArticles).toHaveBeenCalledWith(expect.objectContaining({ includeAdminOnly: false })) }) it('returns 404 when feature disabled', async () => { @@ -200,7 +228,26 @@ describe('POST /api/v1/help-center/articles', () => { expect(response.status).toBe(201) const json = await response.json() expect(json.data.id).toBe('article_1') - expect(createArticle).toHaveBeenCalledWith(body, 'principal_1', undefined) + expect(createArticle).toHaveBeenCalledWith(body, 'principal_1', undefined, { + includeAdminOnly: true, + }) + }) + + it('cannot target admin-only categories with a member-role key', async () => { + vi.mocked(withApiKeyAuth).mockResolvedValue(mockMemberAuthContext) + vi.mocked(createArticle).mockResolvedValue(mockArticle) + + const body = { + categoryId: 'category_1', + title: 'Member Article', + content: 'Member-visible content', + } + const request = createRequest('POST', 'http://localhost/api/v1/help-center/articles', body) + await listHandlers.POST({ request }) + + expect(createArticle).toHaveBeenCalledWith(body, 'principal_1', undefined, { + includeAdminOnly: false, + }) }) it('creates article attributed to authorId when provided', async () => { @@ -220,7 +267,8 @@ describe('POST /api/v1/help-center/articles', () => { expect(createArticle).toHaveBeenCalledWith( { categoryId: 'category_1', title: 'Authored Article', content: 'Content' }, 'principal_1', - 'principal_2' + 'principal_2', + { includeAdminOnly: true } ) }) @@ -314,6 +362,20 @@ describe('GET /api/v1/help-center/articles/:id', () => { name: 'Getting Started', }) expect(json.data.author).toEqual({ id: 'principal_1', name: 'Admin', avatarUrl: null }) + expect(getArticleById).toHaveBeenCalledWith('article_1', { includeAdminOnly: true }) + }) + + it('excludes admin-only articles for member-role keys', async () => { + vi.mocked(withApiKeyAuth).mockResolvedValue(mockMemberAuthContext) + vi.mocked(getArticleById).mockResolvedValue(mockArticle) + + const request = createRequest('GET', 'http://localhost/api/v1/help-center/articles/article_1') + await detailHandlers.GET({ + request, + params: { articleId: 'article_1' }, + }) + + expect(getArticleById).toHaveBeenCalledWith('article_1', { includeAdminOnly: false }) }) it('returns error for invalid ID format', async () => { @@ -360,11 +422,35 @@ describe('PATCH /api/v1/help-center/articles/:id', () => { expect(response.status).toBe(200) const json = await response.json() expect(json.data.title).toBe('Updated Title') - expect(updateArticle).toHaveBeenCalledWith('article_1', { title: 'Updated Title' }, undefined) + expect(updateArticle).toHaveBeenCalledWith('article_1', { title: 'Updated Title' }, undefined, { + includeAdminOnly: true, + }) + }) + + it('cannot update admin-only articles with a member-role key', async () => { + vi.mocked(withApiKeyAuth).mockResolvedValue(mockMemberAuthContext) + vi.mocked(updateArticle).mockResolvedValue(mockArticle) + + const request = createRequest( + 'PATCH', + 'http://localhost/api/v1/help-center/articles/article_1', + { title: 'Member update' } + ) + await detailHandlers.PATCH({ + request, + params: { articleId: 'article_1' }, + }) + + expect(updateArticle).toHaveBeenCalledWith('article_1', { title: 'Member update' }, undefined, { + includeAdminOnly: false, + }) }) it('reassigns author when authorId is provided', async () => { - const updatedArticle = { ...mockArticle, author: { id: 'principal_2' as PrincipalId, name: 'Other', avatarUrl: null } } + const updatedArticle = { + ...mockArticle, + author: { id: 'principal_2' as PrincipalId, name: 'Other', avatarUrl: null }, + } vi.mocked(updateArticle).mockResolvedValue(updatedArticle) vi.mocked(parseOptionalTypeId).mockReturnValue('principal_2' as PrincipalId) @@ -380,7 +466,9 @@ describe('PATCH /api/v1/help-center/articles/:id', () => { }) expect(response.status).toBe(200) - expect(updateArticle).toHaveBeenCalledWith('article_1', {}, 'principal_2') + expect(updateArticle).toHaveBeenCalledWith('article_1', {}, 'principal_2', { + includeAdminOnly: true, + }) }) it('returns 400 when authorId format is invalid', async () => { @@ -440,10 +528,27 @@ describe('PATCH /api/v1/help-center/articles/:id', () => { }) expect(response.status).toBe(200) - expect(publishArticle).toHaveBeenCalledWith('article_1') + expect(publishArticle).toHaveBeenCalledWith('article_1', { includeAdminOnly: true }) expect(updateArticle).not.toHaveBeenCalled() }) + it('cannot publish admin-only articles with a member-role key', async () => { + vi.mocked(withApiKeyAuth).mockResolvedValue(mockMemberAuthContext) + vi.mocked(publishArticle).mockResolvedValue(mockArticle) + + const request = createRequest( + 'PATCH', + 'http://localhost/api/v1/help-center/articles/article_1', + { publishedAt: '2026-04-01T00:00:00.000Z' } + ) + await detailHandlers.PATCH({ + request, + params: { articleId: 'article_1' }, + }) + + expect(publishArticle).toHaveBeenCalledWith('article_1', { includeAdminOnly: false }) + }) + it('unpublishes article when publishedAt is null', async () => { vi.mocked(getArticleById).mockResolvedValue(mockArticle) vi.mocked(unpublishArticle).mockResolvedValue(mockArticle) @@ -460,10 +565,44 @@ describe('PATCH /api/v1/help-center/articles/:id', () => { }) expect(response.status).toBe(200) - expect(unpublishArticle).toHaveBeenCalledWith('article_1') + expect(unpublishArticle).toHaveBeenCalledWith('article_1', { includeAdminOnly: true }) expect(updateArticle).not.toHaveBeenCalled() }) + it('cannot unpublish admin-only articles with a member-role key', async () => { + vi.mocked(withApiKeyAuth).mockResolvedValue(mockMemberAuthContext) + vi.mocked(unpublishArticle).mockResolvedValue(mockArticle) + + const request = createRequest( + 'PATCH', + 'http://localhost/api/v1/help-center/articles/article_1', + { publishedAt: null } + ) + await detailHandlers.PATCH({ + request, + params: { articleId: 'article_1' }, + }) + + expect(unpublishArticle).toHaveBeenCalledWith('article_1', { includeAdminOnly: false }) + }) + + it('excludes admin-only articles from member-role no-op lookups', async () => { + vi.mocked(withApiKeyAuth).mockResolvedValue(mockMemberAuthContext) + vi.mocked(getArticleById).mockResolvedValue(mockArticle) + + const request = createRequest( + 'PATCH', + 'http://localhost/api/v1/help-center/articles/article_1', + {} + ) + await detailHandlers.PATCH({ + request, + params: { articleId: 'article_1' }, + }) + + expect(getArticleById).toHaveBeenCalledWith('article_1', { includeAdminOnly: false }) + }) + it('requires team role', async () => { vi.mocked(withApiKeyAuth).mockRejectedValue( new ForbiddenError('FORBIDDEN', 'Team access required') @@ -522,7 +661,23 @@ describe('DELETE /api/v1/help-center/articles/:id', () => { }) expect(response.status).toBe(204) - expect(deleteArticle).toHaveBeenCalledWith('article_1') + expect(deleteArticle).toHaveBeenCalledWith('article_1', { includeAdminOnly: true }) + }) + + it('cannot delete admin-only articles with a member-role key', async () => { + vi.mocked(withApiKeyAuth).mockResolvedValue(mockMemberAuthContext) + vi.mocked(deleteArticle).mockResolvedValue(undefined) + + const request = createRequest( + 'DELETE', + 'http://localhost/api/v1/help-center/articles/article_1' + ) + await detailHandlers.DELETE({ + request, + params: { articleId: 'article_1' }, + }) + + expect(deleteArticle).toHaveBeenCalledWith('article_1', { includeAdminOnly: false }) }) it('requires admin role', async () => { @@ -568,7 +723,28 @@ describe('POST /api/v1/help-center/articles/:id/feedback', () => { expect(response.status).toBe(200) const json = await response.json() expect(json.data.success).toBe(true) - expect(recordArticleFeedback).toHaveBeenCalledWith('article_1', true, 'principal_1') + expect(recordArticleFeedback).toHaveBeenCalledWith('article_1', true, 'principal_1', { + includeAdminOnly: true, + }) + }) + + it('cannot record feedback for admin-only articles with a member-role key', async () => { + vi.mocked(withApiKeyAuth).mockResolvedValue(mockMemberAuthContext) + vi.mocked(recordArticleFeedback).mockResolvedValue(undefined) + + const request = createRequest( + 'POST', + 'http://localhost/api/v1/help-center/articles/article_1/feedback', + { helpful: true } + ) + await feedbackHandlers.POST({ + request, + params: { articleId: 'article_1' }, + }) + + expect(recordArticleFeedback).toHaveBeenCalledWith('article_1', true, 'principal_1', { + includeAdminOnly: false, + }) }) it('records helpful=false feedback', async () => { @@ -586,7 +762,9 @@ describe('POST /api/v1/help-center/articles/:id/feedback', () => { }) expect(response.status).toBe(200) - expect(recordArticleFeedback).toHaveBeenCalledWith('article_1', false, 'principal_1') + expect(recordArticleFeedback).toHaveBeenCalledWith('article_1', false, 'principal_1', { + includeAdminOnly: true, + }) }) it('returns 400 for invalid body (missing helpful field)', async () => {