Enhance report sharing functionality with response type integration

- Updated DELETE, GET, and POST handlers to return structured response types (ShareDeleteResponse, ShareGetResponse, SharePostResponse) for better consistency and clarity.
- Refactored permission checks and report existence validation to streamline logic.
- Improved optimistic UI updates in the frontend for sharing reports, ensuring a smoother user experience.
- Added response schemas for sharing operations to the shared reports module.
This commit is contained in:
dal 2025-08-22 11:46:30 -06:00
parent 250c02db3b
commit db775397d9
No known key found for this signature in database
GPG Key ID: 16F4B0E1E9F61122
9 changed files with 146 additions and 51 deletions

View File

@ -5,6 +5,7 @@ import {
removeAssetPermission, removeAssetPermission,
} from '@buster/database'; } from '@buster/database';
import type { User } from '@buster/database'; import type { User } from '@buster/database';
import type { ShareDeleteResponse } from '@buster/server-shared/reports';
import type { ShareDeleteRequest } from '@buster/server-shared/share'; import type { ShareDeleteRequest } from '@buster/server-shared/share';
import { ShareDeleteRequestSchema } from '@buster/server-shared/share'; import { ShareDeleteRequestSchema } from '@buster/server-shared/share';
import { zValidator } from '@hono/zod-validator'; import { zValidator } from '@hono/zod-validator';
@ -15,13 +16,7 @@ export async function deleteReportSharingHandler(
reportId: string, reportId: string,
emails: ShareDeleteRequest, emails: ShareDeleteRequest,
user: User user: User
): Promise<{ success: boolean; removed: string[]; notFound: string[] }> { ): Promise<ShareDeleteResponse> {
// Check if report exists
const report = await getReport({ reportId, userId: user.id });
if (!report) {
throw new HTTPException(404, { message: 'Report not found' });
}
// Check if user has permission to modify sharing for the report // Check if user has permission to modify sharing for the report
const permissionCheck = await checkAssetPermission({ const permissionCheck = await checkAssetPermission({
assetId: reportId, assetId: reportId,
@ -39,6 +34,12 @@ export async function deleteReportSharingHandler(
}); });
} }
// Get the report to verify it exists and get owner info
const report = await getReport({ reportId, userId: user.id });
if (!report) {
throw new HTTPException(404, { message: 'Report not found' });
}
// Find users by emails // Find users by emails
const userMap = await findUsersByEmails(emails); const userMap = await findUsersByEmails(emails);

View File

@ -1,22 +1,13 @@
import { checkAssetPermission, getReport, listAssetPermissions } from '@buster/database'; import { checkAssetPermission, getReport, listAssetPermissions } from '@buster/database';
import type { User } from '@buster/database'; import type { User } from '@buster/database';
import type { ShareGetResponse } from '@buster/server-shared/reports';
import { Hono } from 'hono'; import { Hono } from 'hono';
import { HTTPException } from 'hono/http-exception'; import { HTTPException } from 'hono/http-exception';
interface SharePermission {
userId: string;
email: string;
name: string | null;
avatarUrl: string | null;
role: string;
createdAt: string;
updatedAt: string;
}
export async function getReportSharingHandler( export async function getReportSharingHandler(
reportId: string, reportId: string,
user: User user: User
): Promise<{ permissions: SharePermission[] }> { ): Promise<ShareGetResponse> {
// Check if report exists // Check if report exists
const report = await getReport({ reportId, userId: user.id }); const report = await getReport({ reportId, userId: user.id });
if (!report) { if (!report) {
@ -42,19 +33,8 @@ export async function getReportSharingHandler(
assetType: 'report_file', assetType: 'report_file',
}); });
// Format the permissions for the response
const formattedPermissions: SharePermission[] = permissions.map((perm) => ({
userId: perm.user?.id || '',
email: perm.user?.email || '',
name: perm.user?.name || null,
avatarUrl: perm.user?.avatarUrl || null,
role: perm.permission.role,
createdAt: perm.permission.createdAt,
updatedAt: perm.permission.updatedAt,
}));
return { return {
permissions: formattedPermissions, permissions,
}; };
} }

View File

@ -5,6 +5,7 @@ import {
getReport, getReport,
} from '@buster/database'; } from '@buster/database';
import type { User } from '@buster/database'; import type { User } from '@buster/database';
import type { SharePostResponse } from '@buster/server-shared/reports';
import type { SharePostRequest } from '@buster/server-shared/share'; import type { SharePostRequest } from '@buster/server-shared/share';
import { SharePostRequestSchema } from '@buster/server-shared/share'; import { SharePostRequestSchema } from '@buster/server-shared/share';
import { zValidator } from '@hono/zod-validator'; import { zValidator } from '@hono/zod-validator';
@ -15,13 +16,7 @@ export async function createReportSharingHandler(
reportId: string, reportId: string,
shareRequests: SharePostRequest, shareRequests: SharePostRequest,
user: User user: User
): Promise<{ success: boolean; shared: string[]; notFound: string[] }> { ): Promise<SharePostResponse> {
// Check if report exists
const report = await getReport({ reportId, userId: user.id });
if (!report) {
throw new HTTPException(404, { message: 'Report not found' });
}
// Check if user has permission to share the report // Check if user has permission to share the report
const permissionCheck = await checkAssetPermission({ const permissionCheck = await checkAssetPermission({
assetId: reportId, assetId: reportId,
@ -39,6 +34,12 @@ export async function createReportSharingHandler(
}); });
} }
// Get the report to verify it exists
const report = await getReport({ reportId, userId: user.id });
if (!report) {
throw new HTTPException(404, { message: 'Report not found' });
}
// Extract emails from the share requests // Extract emails from the share requests
const emails = shareRequests.map((req) => req.email); const emails = shareRequests.map((req) => req.email);
@ -69,12 +70,19 @@ export async function createReportSharingHandler(
viewer: 'can_view', // Map viewer to can_view viewer: 'can_view', // Map viewer to can_view
} as const; } as const;
const mappedRole = roleMapping[shareRequest.role];
if (!mappedRole) {
throw new HTTPException(400, {
message: `Invalid role: ${shareRequest.role} for user ${shareRequest.email}`,
});
}
permissions.push({ permissions.push({
identityId: targetUser.id, identityId: targetUser.id,
identityType: 'user' as const, identityType: 'user' as const,
assetId: reportId, assetId: reportId,
assetType: 'report_file' as const, assetType: 'report_file' as const,
role: roleMapping[shareRequest.role] || 'can_view', role: mappedRole,
createdBy: user.id, createdBy: user.id,
}); });
} }

View File

@ -19,12 +19,6 @@ export async function updateReportShareHandler(
request: ShareUpdateRequest, request: ShareUpdateRequest,
user: User & { organizationId: string } user: User & { organizationId: string }
) { ) {
// Check if report exists
const report = await getReport({ reportId, userId: user.id });
if (!report) {
throw new HTTPException(404, { message: 'Report not found' });
}
// Check if user has permission to edit asset permissions // Check if user has permission to edit asset permissions
const permissionCheck = await checkAssetPermission({ const permissionCheck = await checkAssetPermission({
assetId: reportId, assetId: reportId,
@ -42,6 +36,12 @@ export async function updateReportShareHandler(
}); });
} }
// Check if report exists
const report = await getReport({ reportId, userId: user.id });
if (!report) {
throw new HTTPException(404, { message: 'Report not found' });
}
const { publicly_accessible, public_expiry_date, public_password, workspace_sharing, users } = const { publicly_accessible, public_expiry_date, public_password, workspace_sharing, users } =
request; request;
@ -73,12 +73,19 @@ export async function updateReportShareHandler(
viewer: 'can_view', // Map viewer to can_view viewer: 'can_view', // Map viewer to can_view
} as const; } as const;
const mappedRole = roleMapping[userPermission.role];
if (!mappedRole) {
throw new HTTPException(400, {
message: `Invalid role: ${userPermission.role} for user ${userPermission.email}`,
});
}
permissions.push({ permissions.push({
identityId: targetUser.id, identityId: targetUser.id,
identityType: 'user' as const, identityType: 'user' as const,
assetId: reportId, assetId: reportId,
assetType: 'report_file' as const, assetType: 'report_file' as const,
role: roleMapping[userPermission.role] || 'can_view', role: mappedRole,
createdBy: user.id, createdBy: user.id,
}); });
} }

View File

@ -274,6 +274,57 @@ export const useShareReport = () => {
return useMutation({ return useMutation({
mutationFn: shareReport, mutationFn: shareReport,
onMutate: async (variables) => {
// Cancel any outgoing refetches
await queryClient.cancelQueries({
queryKey: reportsQueryKeys.reportsGetReport(variables.id).queryKey
});
// Snapshot the previous value
const previousReport = queryClient.getQueryData<GetReportResponse>(
reportsQueryKeys.reportsGetReport(variables.id).queryKey
);
// Optimistically update the report with new permissions
queryClient.setQueryData(
reportsQueryKeys.reportsGetReport(variables.id).queryKey,
(old: GetReportResponse | undefined) => {
if (!old) return old;
return create(old, (draft) => {
// Add new permissions optimistically
variables.params.forEach((shareRequest) => {
const exists = draft.individual_permissions?.some(
(p) => p.email === shareRequest.email
);
if (!exists) {
draft.individual_permissions = [
...(draft.individual_permissions || []),
{
id: `temp-${shareRequest.email}`,
email: shareRequest.email,
name: shareRequest.name || shareRequest.email,
role: shareRequest.role,
avatar_url: shareRequest.avatar_url || null
}
];
}
});
});
}
);
// Return a context object with the snapshotted value
return { previousReport };
},
onError: (err, variables, context) => {
// If the mutation fails, use the context returned from onMutate to roll back
if (context?.previousReport) {
queryClient.setQueryData(
reportsQueryKeys.reportsGetReport(variables.id).queryKey,
context.previousReport
);
}
},
onSuccess: (_, { id }) => { onSuccess: (_, { id }) => {
// Invalidate the report cache to get updated sharing info // Invalidate the report cache to get updated sharing info
queryClient.invalidateQueries({ queryClient.invalidateQueries({

View File

@ -5,6 +5,8 @@ import type {
GetReportsListRequest, GetReportsListRequest,
GetReportsListResponse, GetReportsListResponse,
GetReportResponse, GetReportResponse,
ShareDeleteResponse,
SharePostResponse,
UpdateReportRequest, UpdateReportRequest,
UpdateReportResponse UpdateReportResponse
} from '@buster/server-shared/reports'; } from '@buster/server-shared/reports';
@ -66,14 +68,18 @@ export const updateReport = async ({
* Share a report with users * Share a report with users
*/ */
export const shareReport = async ({ id, params }: { id: string; params: SharePostRequest }) => { export const shareReport = async ({ id, params }: { id: string; params: SharePostRequest }) => {
return mainApiV2.post<string>(`/reports/${id}/sharing`, params).then((res) => res.data); return mainApiV2
.post<SharePostResponse>(`/reports/${id}/sharing`, params)
.then((res) => res.data);
}; };
/** /**
* Remove sharing permissions from a report * Remove sharing permissions from a report
*/ */
export const unshareReport = async ({ id, data }: { id: string; data: ShareDeleteRequest }) => { export const unshareReport = async ({ id, data }: { id: string; data: ShareDeleteRequest }) => {
return mainApiV2.delete<string>(`/reports/${id}/sharing`, { data }).then((res) => res.data); return mainApiV2
.delete<ShareDeleteResponse>(`/reports/${id}/sharing`, { data })
.then((res) => res.data);
}; };
/** /**

View File

@ -98,7 +98,7 @@ export const ShareMenuContentEmbedFooter = ({
} else if (assetType === 'report') { } else if (assetType === 'report') {
await onShareReport(payload); await onShareReport(payload);
} }
openSuccessMessage('Succuessfully published'); openSuccessMessage('Successfully published');
}); });
return ( return (

View File

@ -382,9 +382,7 @@ describe('ReportThreeDotMenu', () => {
'Add to collection', 'Add to collection',
'Add to favorites', // Or 'Remove from favorites' based on state 'Add to favorites', // Or 'Remove from favorites' based on state
'Version history', 'Version history',
'Request verification',
'Refresh report', 'Refresh report',
'Duplicate',
'Download as PDF' 'Download as PDF'
]; ];

View File

@ -1,4 +1,4 @@
import type { z } from 'zod'; import { z } from 'zod';
import { PaginatedResponseSchema } from '../type-utilities/pagination'; import { PaginatedResponseSchema } from '../type-utilities/pagination';
import { ReportIndividualResponseSchema, ReportListItemSchema } from './reports.types'; import { ReportIndividualResponseSchema, ReportListItemSchema } from './reports.types';
@ -6,7 +6,51 @@ export const GetReportsListResponseSchema = PaginatedResponseSchema(ReportListIt
export const UpdateReportResponseSchema = ReportIndividualResponseSchema; export const UpdateReportResponseSchema = ReportIndividualResponseSchema;
export const ShareUpdateResponseSchema = ReportIndividualResponseSchema; export const ShareUpdateResponseSchema = ReportIndividualResponseSchema;
// Sharing operation response schemas
export const SharePostResponseSchema = z.object({
success: z.boolean(),
shared: z.array(z.string()),
notFound: z.array(z.string()),
});
export const ShareDeleteResponseSchema = z.object({
success: z.boolean(),
removed: z.array(z.string()),
notFound: z.array(z.string()),
});
// For GET sharing endpoint - matches AssetPermissionWithUser from database
export const ShareGetResponseSchema = z.object({
permissions: z.array(
z.object({
permission: z.object({
identityId: z.string(),
identityType: z.string(),
assetId: z.string(),
assetType: z.string(),
role: z.string(),
createdAt: z.string(),
updatedAt: z.string(),
createdBy: z.string(),
updatedBy: z.string(),
deletedAt: z.string().nullable(),
}),
user: z
.object({
id: z.string(),
email: z.string(),
name: z.string().nullable(),
avatarUrl: z.string().nullable(),
})
.nullable(),
})
),
});
export type GetReportsListResponse = z.infer<typeof GetReportsListResponseSchema>; export type GetReportsListResponse = z.infer<typeof GetReportsListResponseSchema>;
export type UpdateReportResponse = z.infer<typeof UpdateReportResponseSchema>; export type UpdateReportResponse = z.infer<typeof UpdateReportResponseSchema>;
export type GetReportResponse = z.infer<typeof ReportIndividualResponseSchema>; export type GetReportResponse = z.infer<typeof ReportIndividualResponseSchema>;
export type ShareUpdateResponse = z.infer<typeof ShareUpdateResponseSchema>; export type ShareUpdateResponse = z.infer<typeof ShareUpdateResponseSchema>;
export type SharePostResponse = z.infer<typeof SharePostResponseSchema>;
export type ShareDeleteResponse = z.infer<typeof ShareDeleteResponseSchema>;
export type ShareGetResponse = z.infer<typeof ShareGetResponseSchema>;