Ling0925
/
buster
mirror of https://github.com/buster-so/buster.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Refactor report sharing endpoints for improved structure and clarity 

- Exported handler functions for DELETE, GET, POST, and PUT endpoints to enhance modularity.
- Removed redundant authentication middleware from individual endpoint files, consolidating it in the index route.
- Cleaned up code formatting for better readability.
- Deleted outdated test file for report sharing endpoints.
This commit is contained in:
dal 2025-08-22 11:15:35 -06:00
parent eedf5f42f8
commit c1e1c1edac
No known key found for this signature in database
GPG Key ID: 16F4B0E1E9F61122
7 changed files with 50 additions and 450 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
apps/server/src/api/v2/reports/[id]/sharing/DELETE.ts
View File

@ -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;

5
apps/server/src/api/v2/reports/[id]/sharing/GET.ts
View File

@ -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');

25
apps/server/src/api/v2/reports/[id]/sharing/POST.ts
View File

@ -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;

39
apps/server/src/api/v2/reports/[id]/sharing/PUT.ts
View File

@ -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;

8
apps/server/src/api/v2/reports/[id]/sharing/index.ts
View File

@ -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;

396
apps/server/src/api/v2/reports/[id]/sharing/sharing.test.ts
View File

@ -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);
});
});
});

2
apps/web/src/layouts/ChatLayout/FileContainer/FileContainerHeader/ReportContainerHeaderButtons/ReportThreeDotMenu.test.tsx
View File

@ -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',