diff --git a/apps/server/src/api/v2/reports/[id]/sharing/DELETE.ts b/apps/server/src/api/v2/reports/[id]/sharing/DELETE.ts index 4154b3f81..53bcd3945 100644 --- a/apps/server/src/api/v2/reports/[id]/sharing/DELETE.ts +++ b/apps/server/src/api/v2/reports/[id]/sharing/DELETE.ts @@ -10,9 +10,8 @@ import { ShareDeleteRequestSchema } from '@buster/server-shared/share'; import { zValidator } from '@hono/zod-validator'; import { Hono } from 'hono'; import { HTTPException } from 'hono/http-exception'; -import { requireAuth } from '../../../../../middleware/auth'; -async function deleteReportSharingHandler( +export async function deleteReportSharingHandler( reportId: string, emails: ShareDeleteRequest, user: User @@ -78,20 +77,18 @@ async function deleteReportSharingHandler( }; } -const app = new Hono() - .use('*', requireAuth) - .delete('/', zValidator('json', ShareDeleteRequestSchema), async (c) => { - const reportId = c.req.param('id'); - const emails = c.req.valid('json'); - const user = c.get('busterUser'); +const app = new Hono().delete('/', zValidator('json', ShareDeleteRequestSchema), async (c) => { + const reportId = c.req.param('id'); + const emails = c.req.valid('json'); + const user = c.get('busterUser'); - if (!reportId) { - throw new HTTPException(400, { message: 'Report ID is required' }); - } + if (!reportId) { + throw new HTTPException(400, { message: 'Report ID is required' }); + } - const result = await deleteReportSharingHandler(reportId, emails, user); + const result = await deleteReportSharingHandler(reportId, emails, user); - return c.json(result); - }); + return c.json(result); +}); export default app; diff --git a/apps/server/src/api/v2/reports/[id]/sharing/GET.ts b/apps/server/src/api/v2/reports/[id]/sharing/GET.ts index 5729d92fc..521be1aa0 100644 --- a/apps/server/src/api/v2/reports/[id]/sharing/GET.ts +++ b/apps/server/src/api/v2/reports/[id]/sharing/GET.ts @@ -2,7 +2,6 @@ import { checkAssetPermission, getReport, listAssetPermissions } from '@buster/d import type { User } from '@buster/database'; import { Hono } from 'hono'; import { HTTPException } from 'hono/http-exception'; -import { requireAuth } from '../../../../../middleware/auth'; interface SharePermission { userId: string; @@ -14,7 +13,7 @@ interface SharePermission { updatedAt: string; } -async function getReportSharingHandler( +export async function getReportSharingHandler( reportId: string, user: User ): Promise<{ permissions: SharePermission[] }> { @@ -59,7 +58,7 @@ async function getReportSharingHandler( }; } -const app = new Hono().use('*', requireAuth).get('/', async (c) => { +const app = new Hono().get('/', async (c) => { const reportId = c.req.param('id'); const user = c.get('busterUser'); diff --git a/apps/server/src/api/v2/reports/[id]/sharing/POST.ts b/apps/server/src/api/v2/reports/[id]/sharing/POST.ts index 048ac8319..f1f5f4bd1 100644 --- a/apps/server/src/api/v2/reports/[id]/sharing/POST.ts +++ b/apps/server/src/api/v2/reports/[id]/sharing/POST.ts @@ -10,9 +10,8 @@ import { SharePostRequestSchema } from '@buster/server-shared/share'; import { zValidator } from '@hono/zod-validator'; import { Hono } from 'hono'; import { HTTPException } from 'hono/http-exception'; -import { requireAuth } from '../../../../../middleware/auth'; -async function createReportSharingHandler( +export async function createReportSharingHandler( reportId: string, shareRequests: SharePostRequest, user: User @@ -92,20 +91,18 @@ async function createReportSharingHandler( }; } -const app = new Hono() - .use('*', requireAuth) - .post('/', zValidator('json', SharePostRequestSchema), async (c) => { - const reportId = c.req.param('id'); - const shareRequests = c.req.valid('json'); - const user = c.get('busterUser'); +const app = new Hono().post('/', zValidator('json', SharePostRequestSchema), async (c) => { + const reportId = c.req.param('id'); + const shareRequests = c.req.valid('json'); + const user = c.get('busterUser'); - if (!reportId) { - throw new HTTPException(400, { message: 'Report ID is required' }); - } + if (!reportId) { + throw new HTTPException(400, { message: 'Report ID is required' }); + } - const result = await createReportSharingHandler(reportId, shareRequests, user); + const result = await createReportSharingHandler(reportId, shareRequests, user); - return c.json(result); - }); + return c.json(result); +}); export default app; diff --git a/apps/server/src/api/v2/reports/[id]/sharing/PUT.ts b/apps/server/src/api/v2/reports/[id]/sharing/PUT.ts index 9b5615820..1dde93b1a 100644 --- a/apps/server/src/api/v2/reports/[id]/sharing/PUT.ts +++ b/apps/server/src/api/v2/reports/[id]/sharing/PUT.ts @@ -12,10 +12,9 @@ import { type ShareUpdateRequest, ShareUpdateRequestSchema } from '@buster/serve import { zValidator } from '@hono/zod-validator'; import { Hono } from 'hono'; import { HTTPException } from 'hono/http-exception'; -import { requireAuth } from '../../../../../middleware/auth'; import { getReportHandler } from '../GET'; -async function updateReportShareHandler( +export async function updateReportShareHandler( reportId: string, request: ShareUpdateRequest, user: User & { organizationId: string } @@ -108,29 +107,27 @@ async function updateReportShareHandler( return updatedReport; } -const app = new Hono() - .use('*', requireAuth) - .put('/', zValidator('json', ShareUpdateRequestSchema), async (c) => { - const reportId = c.req.param('id'); - const request = c.req.valid('json'); - const user = c.get('busterUser'); +const app = new Hono().put('/', zValidator('json', ShareUpdateRequestSchema), async (c) => { + const reportId = c.req.param('id'); + const request = c.req.valid('json'); + const user = c.get('busterUser'); - if (!reportId) { - throw new HTTPException(404, { message: 'Report not found' }); - } + if (!reportId) { + throw new HTTPException(404, { message: 'Report not found' }); + } - const userOrg = await getUserOrganizationId(user.id); + const userOrg = await getUserOrganizationId(user.id); - if (!userOrg) { - throw new HTTPException(403, { message: 'User is not associated with an organization' }); - } + if (!userOrg) { + throw new HTTPException(403, { message: 'User is not associated with an organization' }); + } - const updatedReport: ShareUpdateResponse = await updateReportShareHandler(reportId, request, { - ...user, - organizationId: userOrg.organizationId, - }); - - return c.json(updatedReport); + const updatedReport: ShareUpdateResponse = await updateReportShareHandler(reportId, request, { + ...user, + organizationId: userOrg.organizationId, }); + return c.json(updatedReport); +}); + export default app; diff --git a/apps/server/src/api/v2/reports/[id]/sharing/index.ts b/apps/server/src/api/v2/reports/[id]/sharing/index.ts index b4b24e1df..659e3fcbc 100644 --- a/apps/server/src/api/v2/reports/[id]/sharing/index.ts +++ b/apps/server/src/api/v2/reports/[id]/sharing/index.ts @@ -1,9 +1,15 @@ import { Hono } from 'hono'; +import { requireAuth } from '../../../../../middleware/auth'; import DELETE from './DELETE'; import GET from './GET'; import POST from './POST'; import PUT from './PUT'; -const app = new Hono().route('/', GET).route('/', POST).route('/', PUT).route('/', DELETE); +const app = new Hono() + .use('*', requireAuth) + .route('/', GET) + .route('/', POST) + .route('/', PUT) + .route('/', DELETE); export default app; diff --git a/apps/server/src/api/v2/reports/[id]/sharing/sharing.test.ts b/apps/server/src/api/v2/reports/[id]/sharing/sharing.test.ts deleted file mode 100644 index 190c7faa5..000000000 --- a/apps/server/src/api/v2/reports/[id]/sharing/sharing.test.ts +++ /dev/null @@ -1,396 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; - -// Mock database functions -vi.mock('@buster/database', () => ({ - getReport: vi.fn(), - checkAssetPermission: vi.fn(), - findUsersByEmails: vi.fn(), - bulkCreateAssetPermissions: vi.fn(), - listAssetPermissions: vi.fn(), - removeAssetPermission: vi.fn(), - updateReport: vi.fn(), - getUserOrganizationId: vi.fn(), -})); - -// Mock middleware -vi.mock('../../../../../middleware/auth', () => ({ - requireAuth: vi.fn((c, next) => next()), -})); - -// Mock the GET handler -vi.mock('../GET', () => ({ - getReportHandler: vi.fn(), -})); - -import { - bulkCreateAssetPermissions, - checkAssetPermission, - findUsersByEmails, - getReport, - getUserOrganizationId, - listAssetPermissions, - removeAssetPermission, - updateReport, -} from '@buster/database'; -import { testClient } from 'hono/testing'; -import { getReportHandler } from '../GET'; -import DELETE from './DELETE'; -import GET from './GET'; -import POST from './POST'; -import PUT from './PUT'; - -describe('Report Sharing Endpoints', () => { - const mockUser = { - id: 'user-123', - name: 'Test User', - email: 'test@example.com', - avatarUrl: null, - }; - - const mockReport = { - id: 'report-123', - name: 'Test Report', - content: 'Report content', - created_by_id: 'user-123', - created_at: '2024-01-01T00:00:00Z', - updated_at: '2024-01-01T00:00:00Z', - }; - - beforeEach(() => { - vi.clearAllMocks(); - }); - - describe('POST /reports/:id/sharing', () => { - it('should create sharing permissions for valid users', async () => { - const app = POST; - const client = testClient(app, { - set: { busterUser: mockUser }, - param: { id: 'report-123' }, - }); - - vi.mocked(getReport).mockResolvedValue(mockReport as any); - vi.mocked(checkAssetPermission).mockResolvedValue({ - hasAccess: true, - role: 'owner', - accessPath: 'direct', - }); - vi.mocked(findUsersByEmails).mockResolvedValue( - new Map([ - [ - 'user2@example.com', - { - id: 'user-456', - email: 'user2@example.com', - name: 'User 2', - avatarUrl: null, - }, - ], - ]) as any - ); - vi.mocked(bulkCreateAssetPermissions).mockResolvedValue([]); - - const response = await client.index.$post({ - json: [ - { - email: 'user2@example.com', - role: 'can_view', - }, - ], - }); - - expect(response.status).toBe(200); - const data = await response.json(); - expect(data).toEqual({ - success: true, - shared: ['user2@example.com'], - notFound: [], - }); - }); - - it('should return 403 if user lacks permission', async () => { - const app = POST; - const client = testClient(app, { - set: { busterUser: mockUser }, - param: { id: 'report-123' }, - }); - - vi.mocked(getReport).mockResolvedValue(mockReport as any); - vi.mocked(checkAssetPermission).mockResolvedValue({ - hasAccess: true, - role: 'can_view', - accessPath: 'direct', - }); - - const response = await client.index.$post({ - json: [ - { - email: 'user2@example.com', - role: 'can_view', - }, - ], - }); - - expect(response.status).toBe(403); - }); - }); - - describe('GET /reports/:id/sharing', () => { - it('should list sharing permissions', async () => { - const app = GET; - const client = testClient(app, { - set: { busterUser: mockUser }, - param: { id: 'report-123' }, - }); - - vi.mocked(getReport).mockResolvedValue(mockReport as any); - vi.mocked(checkAssetPermission).mockResolvedValue({ - hasAccess: true, - role: 'can_view', - accessPath: 'direct', - }); - vi.mocked(listAssetPermissions).mockResolvedValue([ - { - permission: { - identityId: 'user-456', - identityType: 'user', - assetId: 'report-123', - assetType: 'report_file', - role: 'can_edit', - createdAt: '2024-01-01T00:00:00Z', - updatedAt: '2024-01-01T00:00:00Z', - deletedAt: null, - createdBy: 'user-123', - updatedBy: 'user-123', - }, - user: { - id: 'user-456', - email: 'user2@example.com', - name: 'User 2', - avatarUrl: null, - }, - }, - ] as any); - - const response = await client.index.$get(); - - expect(response.status).toBe(200); - const data = await response.json(); - expect(data).toEqual({ - permissions: [ - { - userId: 'user-456', - email: 'user2@example.com', - name: 'User 2', - avatarUrl: null, - role: 'can_edit', - createdAt: '2024-01-01T00:00:00Z', - updatedAt: '2024-01-01T00:00:00Z', - }, - ], - }); - }); - - it('should return 403 if user lacks view permission', async () => { - const app = GET; - const client = testClient(app, { - set: { busterUser: mockUser }, - param: { id: 'report-123' }, - }); - - vi.mocked(getReport).mockResolvedValue(mockReport as any); - vi.mocked(checkAssetPermission).mockResolvedValue({ - hasAccess: false, - }); - - const response = await client.index.$get(); - - expect(response.status).toBe(403); - }); - }); - - describe('DELETE /reports/:id/sharing', () => { - it('should remove sharing permissions', async () => { - const app = DELETE; - const client = testClient(app, { - set: { busterUser: mockUser }, - param: { id: 'report-123' }, - }); - - vi.mocked(getReport).mockResolvedValue(mockReport as any); - vi.mocked(checkAssetPermission).mockResolvedValue({ - hasAccess: true, - role: 'owner', - accessPath: 'direct', - }); - vi.mocked(findUsersByEmails).mockResolvedValue( - new Map([ - [ - 'user2@example.com', - { - id: 'user-456', - email: 'user2@example.com', - name: 'User 2', - avatarUrl: null, - }, - ], - ]) as any - ); - vi.mocked(removeAssetPermission).mockResolvedValue({} as any); - - const response = await client.index.$delete({ - json: ['user2@example.com'], - }); - - expect(response.status).toBe(200); - const data = await response.json(); - expect(data).toEqual({ - success: true, - removed: ['user2@example.com'], - notFound: [], - }); - }); - - it('should not remove owner permissions', async () => { - const app = DELETE; - const client = testClient(app, { - set: { busterUser: mockUser }, - param: { id: 'report-123' }, - }); - - vi.mocked(getReport).mockResolvedValue(mockReport as any); - vi.mocked(checkAssetPermission).mockResolvedValue({ - hasAccess: true, - role: 'owner', - accessPath: 'direct', - }); - vi.mocked(findUsersByEmails).mockResolvedValue( - new Map([ - [ - 'test@example.com', - { - id: 'user-123', // Same as creator - email: 'test@example.com', - name: 'Test User', - avatarUrl: null, - }, - ], - ]) as any - ); - - const response = await client.index.$delete({ - json: ['test@example.com'], - }); - - expect(response.status).toBe(200); - const data = await response.json(); - expect(data).toEqual({ - success: true, - removed: [], // Owner not removed - notFound: [], - }); - expect(removeAssetPermission).not.toHaveBeenCalled(); - }); - }); - - describe('PUT /reports/:id/sharing', () => { - it('should update sharing settings and permissions', async () => { - const app = PUT; - const client = testClient(app, { - set: { busterUser: mockUser }, - param: { id: 'report-123' }, - }); - - vi.mocked(getReport).mockResolvedValue(mockReport as any); - vi.mocked(checkAssetPermission).mockResolvedValue({ - hasAccess: true, - role: 'owner', - accessPath: 'direct', - }); - vi.mocked(getUserOrganizationId).mockResolvedValue({ - userId: 'user-123', - organizationId: 'org-123', - } as any); - vi.mocked(findUsersByEmails).mockResolvedValue( - new Map([ - [ - 'user2@example.com', - { - id: 'user-456', - email: 'user2@example.com', - name: 'User 2', - avatarUrl: null, - }, - ], - ]) as any - ); - vi.mocked(bulkCreateAssetPermissions).mockResolvedValue([]); - vi.mocked(updateReport).mockResolvedValue(undefined); - vi.mocked(getReportHandler).mockResolvedValue({ - ...mockReport, - publicly_accessible: true, - permission: 'owner', - workspace_sharing: null, - individual_permissions: [], - public_expiry_date: null, - public_password: null, - workspace_member_count: 1, - collections: [], - version_number: 1, - versions: [], - created_by_name: 'Test User', - created_by_avatar: null, - } as any); - - const response = await client.index.$put({ - json: { - publicly_accessible: true, - users: [ - { - email: 'user2@example.com', - role: 'can_edit', - }, - ], - }, - }); - - expect(response.status).toBe(200); - const data = await response.json(); - expect(data).toHaveProperty('publicly_accessible', true); - expect(updateReport).toHaveBeenCalledWith( - expect.objectContaining({ - reportId: 'report-123', - userId: 'user-123', - publicly_accessible: true, - }), - false - ); - }); - - it('should return 403 if user lacks permission', async () => { - const app = PUT; - const client = testClient(app, { - set: { busterUser: mockUser }, - param: { id: 'report-123' }, - }); - - vi.mocked(getReport).mockResolvedValue(mockReport as any); - vi.mocked(checkAssetPermission).mockResolvedValue({ - hasAccess: true, - role: 'can_view', - accessPath: 'direct', - }); - vi.mocked(getUserOrganizationId).mockResolvedValue({ - userId: 'user-123', - organizationId: 'org-123', - } as any); - - const response = await client.index.$put({ - json: { - publicly_accessible: true, - }, - }); - - expect(response.status).toBe(403); - }); - }); -}); diff --git a/apps/web/src/layouts/ChatLayout/FileContainer/FileContainerHeader/ReportContainerHeaderButtons/ReportThreeDotMenu.test.tsx b/apps/web/src/layouts/ChatLayout/FileContainer/FileContainerHeader/ReportContainerHeaderButtons/ReportThreeDotMenu.test.tsx index 31f191655..b390dad3e 100644 --- a/apps/web/src/layouts/ChatLayout/FileContainer/FileContainerHeader/ReportContainerHeaderButtons/ReportThreeDotMenu.test.tsx +++ b/apps/web/src/layouts/ChatLayout/FileContainer/FileContainerHeader/ReportContainerHeaderButtons/ReportThreeDotMenu.test.tsx @@ -378,7 +378,7 @@ describe('ReportThreeDotMenu', () => { // Check for all expected menu items const expectedItems = [ 'Edit with AI', - 'Share metric', // Note: This might be a bug in the original code - says "metric" for reports + 'Share report', 'Add to collection', 'Add to favorites', // Or 'Remove from favorites' based on state 'Version history',