-
Notifications
You must be signed in to change notification settings - Fork 249
feat: add Dashboard public sharing functionality
#223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: add Dashboard public sharing functionality
#223
Conversation
|
@keiwanmosaddegh is attempting to deploy a commit to the Coderax's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
|
WalkthroughThis PR implements dashboard sharing functionality, adding a new database model for share configurations, API endpoints for creating and retrieving dashboard shares, password authentication for protected shares, UI components for managing share settings, and a public route for accessing shared dashboards. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Dashboard as Dashboard App
participant API as TRPC Backend
participant DB as Database
participant PublicShare as Public Share Route
User->>Dashboard: Open dashboard
Dashboard->>API: getShareByDashboardId()
API->>DB: Query ShareDashboard by dashboardId
DB-->>API: Return share config
API-->>Dashboard: Return share data
Dashboard->>Dashboard: Render DashboardShare component
User->>Dashboard: Click "Make Public" in DashboardShare
Dashboard->>Dashboard: Open ShareDashboardModal
User->>Dashboard: Submit (optional password)
Dashboard->>API: createDashboard mutation
API->>DB: Upsert shareDashboard record
DB-->>API: Confirm
API-->>Dashboard: Return updated share
Dashboard->>Dashboard: Refresh UI
User->>PublicShare: Visit /share/dashboard/[id]
PublicShare->>API: getShareDashboardById()
API->>DB: Query share + dashboard + reports
alt Share password protected
DB-->>API: Return share config
API-->>PublicShare: Return share data
PublicShare->>PublicShare: Render ShareEnterPassword
User->>PublicShare: Enter password
PublicShare->>API: signInShareDashboard mutation
API->>DB: Verify password hash
API-->>PublicShare: Set cookie, return success
PublicShare->>PublicShare: Render dashboard reports
else Share public
PublicShare->>PublicShare: Render dashboard reports
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (5)
apps/dashboard/src/components/auth/share-enter-password.tsx (1)
22-39: Consider extracting duplicate handlers to reduce code duplication.The
onSuccessandonErrorhandlers are identical in both mutation branches. Extracting them would improve maintainability.Apply this diff to refactor:
+ const onSuccess = () => { + router.refresh(); + }; + + const onError = () => { + toast.error('Incorrect password'); + }; + const mutation = type === 'dashboard' - ? api.auth.signInShareDashboard.useMutation({ - onSuccess() { - router.refresh(); - }, - onError() { - toast.error('Incorrect password'); - }, - }) - : api.auth.signInShare.useMutation({ - onSuccess() { - router.refresh(); - }, - onError() { - toast.error('Incorrect password'); - }, - }); + ? api.auth.signInShareDashboard.useMutation({ onSuccess, onError }) + : api.auth.signInShare.useMutation({ onSuccess, onError });packages/trpc/src/routers/share.ts (1)
37-58: LGTM!The
createDashboardmutation correctly implements dashboard sharing with proper password hashing and upsert logic. The implementation mirrorsshareOverviewwhich maintains consistency.Consider extracting the shared password hashing and upsert pattern into a helper function to reduce duplication between
shareOverviewandcreateDashboard, though the current implementation is acceptable given the different entities involved.apps/dashboard/src/components/dashboard/dashboard-share.tsx (2)
25-29: Add error handling to the mutation.The mutation lacks an
onErrorhandler, which means failures will occur silently without user feedback.Apply this diff to add error handling:
const mutation = api.share.createDashboard.useMutation({ + onError: handleError, onSuccess() { router.refresh(); }, });Also consider importing and using
handleErrorfrom@/trpc/clientwhich is already imported in this file.
57-64: Consider showing loading state during mutation.The dropdown menu item doesn't disable or show loading state during the mutation, which could allow duplicate submissions if the user clicks multiple times.
Consider disabling the menu item during mutation:
{data?.public && ( <DropdownMenuItem + disabled={mutation.isPending} onClick={() => { mutation.mutate({ ...data, public: false, password: null, }); }} > <LockIcon size={16} className="mr-2" /> - Make private + {mutation.isPending ? 'Making private...' : 'Make private'} </DropdownMenuItem> )}Note: Verify whether TRPC exposes
isPendingorisLoadingbased on your version.apps/dashboard/src/modals/ShareDashboardModal.tsx (1)
45-67: Consider displaying form validation errors.The form uses Zod validation but doesn't display validation errors to the user. If validation fails, users won't know what went wrong.
Consider adding error display for the password field:
+ {form.formState.errors.password && ( + <p className="text-sm text-destructive"> + {form.formState.errors.password.message} + </p> + )} <Input {...register('password')} + type="password" placeholder="Enter your password" size="large" />You'll also need to destructure
formStatefrom theuseFormhook.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/dashboard/src/app/(app)/[organizationSlug]/[projectId]/dashboards/[dashboardId]/list-reports.tsx(2 hunks)apps/dashboard/src/app/(app)/[organizationSlug]/[projectId]/dashboards/[dashboardId]/page.tsx(3 hunks)apps/dashboard/src/app/(public)/share/dashboard/[id]/page.tsx(1 hunks)apps/dashboard/src/components/auth/share-enter-password.tsx(2 hunks)apps/dashboard/src/components/dashboard/dashboard-share.tsx(1 hunks)apps/dashboard/src/modals/ShareDashboardModal.tsx(1 hunks)apps/dashboard/src/modals/index.tsx(1 hunks)packages/db/prisma/migrations/20251029161715_add_share_dashboard/migration.sql(1 hunks)packages/db/prisma/schema.prisma(3 hunks)packages/db/src/services/share.service.ts(1 hunks)packages/trpc/src/routers/auth.ts(2 hunks)packages/trpc/src/routers/share.ts(2 hunks)packages/validation/src/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
apps/dashboard/src/modals/index.tsx (1)
apps/dashboard/src/components/report-chart/metric/index.tsx (1)
Loading(34-44)
packages/db/src/services/share.service.ts (1)
packages/db/src/prisma-client.ts (1)
db(270-270)
packages/trpc/src/routers/share.ts (3)
packages/trpc/src/trpc.ts (1)
protectedProcedure(146-149)packages/validation/src/index.ts (1)
zShareDashboard(184-189)packages/db/src/prisma-client.ts (1)
db(270-270)
apps/dashboard/src/app/(app)/[organizationSlug]/[projectId]/dashboards/[dashboardId]/list-reports.tsx (3)
packages/db/src/services/reports.service.ts (1)
getReportsByDashboardId(74-82)packages/db/src/services/dashboard.service.ts (1)
IServiceDashboard(4-4)apps/dashboard/src/components/dashboard/dashboard-share.tsx (1)
DashboardShare(23-74)
apps/dashboard/src/components/dashboard/dashboard-share.tsx (3)
apps/dashboard/src/trpc/client.tsx (1)
api(9-9)apps/dashboard/src/components/ui/dropdown-menu.tsx (5)
DropdownMenu(228-228)DropdownMenuTrigger(229-229)DropdownMenuContent(230-230)DropdownMenuGroup(237-237)DropdownMenuItem(231-231)apps/dashboard/src/components/ui/button.tsx (1)
Button(180-180)
apps/dashboard/src/app/(app)/[organizationSlug]/[projectId]/dashboards/[dashboardId]/page.tsx (2)
packages/db/src/services/share.service.ts (1)
getShareByDashboardId(33-39)apps/dashboard/src/app/(app)/[organizationSlug]/[projectId]/dashboards/[dashboardId]/list-reports.tsx (1)
ListReports(49-192)
apps/dashboard/src/modals/ShareDashboardModal.tsx (5)
packages/validation/src/index.ts (1)
zShareDashboard(184-189)apps/dashboard/src/hooks/useAppParams.ts (1)
useAppParams(8-15)apps/dashboard/src/trpc/client.tsx (2)
api(9-9)handleError(27-31)apps/dashboard/src/modals/Modal/Container.tsx (2)
ModalContent(15-17)ModalHeader(26-62)apps/dashboard/src/components/button-container.tsx (1)
ButtonContainer(4-11)
apps/dashboard/src/components/auth/share-enter-password.tsx (1)
apps/dashboard/src/trpc/client.tsx (1)
api(9-9)
apps/dashboard/src/app/(public)/share/dashboard/[id]/page.tsx (6)
apps/dashboard/src/app/(app)/[organizationSlug]/[projectId]/dashboards/[dashboardId]/page.tsx (1)
Page(19-41)packages/db/src/services/share.service.ts (1)
getShareDashboardById(22-31)packages/db/src/services/organization.service.ts (1)
getOrganizationById(37-43)apps/dashboard/src/components/auth/share-enter-password.tsx (1)
ShareEnterPassword(14-101)packages/db/src/services/reports.service.ts (1)
getReportsByDashboardId(74-82)apps/dashboard/src/components/report-chart/index.tsx (1)
ReportChart(21-73)
packages/trpc/src/routers/auth.ts (5)
packages/trpc/src/trpc.ts (2)
publicProcedure(145-145)rateLimitMiddleware(17-31)packages/validation/src/index.ts (1)
zSignInShare(427-430)packages/db/src/services/share.service.ts (1)
getShareDashboardById(22-31)packages/trpc/src/errors.ts (2)
TRPCNotFoundError(9-13)TRPCAccessError(3-7)packages/auth/src/password.ts (1)
verifyPasswordHash(14-19)
🔇 Additional comments (12)
apps/dashboard/src/modals/index.tsx (1)
65-67: LGTM!The ShareDashboardModal registration follows the same pattern as other modals in the registry and is correctly configured.
packages/validation/src/index.ts (1)
184-189: LGTM!The
zShareDashboardschema correctly validates dashboard sharing configuration and follows the same pattern aszShareOverview.apps/dashboard/src/app/(app)/[organizationSlug]/[projectId]/dashboards/[dashboardId]/page.tsx (1)
22-26: LGTM!The data fetching logic correctly retrieves the share configuration in parallel with other dashboard data and properly integrates it with the ListReports component.
packages/db/src/services/share.service.ts (1)
22-39: LGTM!The new service functions follow the established pattern and correctly query the ShareDashboard model. The use of
findFirstwithincludeforgetShareDashboardByIdandfindUniqueforgetShareByDashboardIdis appropriate given the unique constraints.packages/db/prisma/schema.prisma (1)
342-354: The original review comment is based on incorrect technical claims.Prisma requires each model to be uniquely identifiable, which can be satisfied either by a primary key (@id / @@id) or by at least one unique constraint (@unique / @@unique). For relational databases @unique is sufficient.
The
id String @uniquepattern used in both ShareOverview and ShareDashboard is a valid Prisma configuration. The original review comment incorrectly claims that missing@idwill cause Prisma Client API generation failures—this is not true. Both models will function correctly as configured.This may be a stylistic or convention concern (using
@idis more standard), but it is not a critical bug preventing Prisma functionality.Likely an incorrect or invalid review comment.
apps/dashboard/src/modals/ShareDashboardModal.tsx (2)
61-61: Verify TRPC mutation property name.The code uses
mutation.isLoading, but depending on your TRPC version, this might bemutation.isPendinginstead.Please verify which property is exposed by your TRPC version by checking the type of the mutation object or referring to the TRPC documentation for your version.
24-29: Fix type mismatch in password default value.The default value for
passwordis an empty string'', but the validation schemazShareDashboardexpectsz.string().nullable(). This type mismatch could cause validation errors or unexpected behavior.Apply this diff to fix the type mismatch:
defaultValues: { public: true, - password: '', + password: null, dashboardId, organizationId, },Likely an incorrect or invalid review comment.
packages/trpc/src/routers/auth.ts (1)
367-368: LGTM!Adding an explicit
return truestatement makes the return behavior consistent with other mutations in the router.apps/dashboard/src/app/(app)/[organizationSlug]/[projectId]/dashboards/[dashboardId]/list-reports.tsx (1)
33-37: LGTM!The changes properly integrate the dashboard sharing feature by:
- Adding the
ShareDashboardtype import- Importing the
DashboardSharecomponent- Extending the props interface to accept
shareDashboard- Threading the prop through to render the share controls
The implementation is clean and follows React best practices.
Also applies to: 41-41, 46-46, 49-53, 71-71
apps/dashboard/src/app/(public)/share/dashboard/[id]/page.tsx (3)
20-39: LGTM!The share validation and password protection logic is well-implemented:
- Properly handles missing or non-public shares with 404 responses
- Cookie name pattern
shared-dashboard-${share.id}is consistent with the authentication flow inpackages/trpc/src/routers/auth.ts- Correctly delegates to
ShareEnterPasswordwithtype="dashboard"for password-protected shares
43-59: LGTM!The conditional header rendering is well-designed:
- The
searchParams.headercheck allows hiding the header (useful for iframe embeds)- Displays relevant context (organization and dashboard names)
- Includes appropriate branding for the shared view
60-77: LGTM!The reports rendering is well-structured:
- Provides a clear empty state message when no reports exist
- Uses a responsive grid layout (
grid-cols-1 md:grid-cols-2) for optimal viewing- Properly displays report names and charts using the existing
ReportChartcomponent
| <Input | ||
| {...register('password')} | ||
| placeholder="Enter your password" | ||
| size="large" | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add password masking to the input field.
The password input field is missing the type="password" attribute, which means the password will be displayed as plain text instead of being masked. This is a security and user experience issue.
Apply this diff to mask the password input:
<Input
{...register('password')}
+ type="password"
placeholder="Enter your password"
size="large"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Input | |
| {...register('password')} | |
| placeholder="Enter your password" | |
| size="large" | |
| /> | |
| <Input | |
| {...register('password')} | |
| type="password" | |
| placeholder="Enter your password" | |
| size="large" | |
| /> |
🤖 Prompt for AI Agents
In apps/dashboard/src/modals/ShareDashboardModal.tsx around lines 52 to 56, the
password Input is missing masking; update the Input props to include
type="password" (and optionally autoComplete="current-password") so the value is
rendered as a masked password while keeping the existing
{...register('password')} and other props unchanged.
| -- CreateTable | ||
| CREATE TABLE "shares_dashboards" ( | ||
| "id" TEXT NOT NULL, | ||
| "dashboardId" TEXT NOT NULL, | ||
| "organizationId" TEXT NOT NULL, | ||
| "public" BOOLEAN NOT NULL DEFAULT false, | ||
| "password" TEXT, | ||
| "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| "updatedAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP | ||
| ); | ||
|
|
||
| -- CreateIndex | ||
| CREATE UNIQUE INDEX "shares_dashboards_id_key" ON "shares_dashboards"("id"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE UNIQUE INDEX "shares_dashboards_dashboardId_key" ON "shares_dashboards"("dashboardId"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Missing PRIMARY KEY constraint on the id column.
The migration creates a UNIQUE INDEX on id but does not define it as the PRIMARY KEY. This stems from the missing @id decorator in the Prisma schema.
This issue will be resolved by fixing the schema in packages/db/prisma/schema.prisma (see comment on that file) and regenerating the migration.
🤖 Prompt for AI Agents
In
packages/db/prisma/migrations/20251029161715_add_share_dashboard/migration.sql
around lines 1-16 the "shares_dashboards" table creates a UNIQUE INDEX on "id"
but does not declare "id" as the PRIMARY KEY; update the Prisma schema
(packages/db/prisma/schema.prisma) to mark the id field with @id and then
regenerate the migration so the SQL includes a PRIMARY KEY constraint for "id"
(or if you must edit this migration directly, add a PRIMARY KEY constraint to
the "id" column and remove the incorrect separate unique index).
| signInShareDashboard: publicProcedure | ||
| .use( | ||
| rateLimitMiddleware({ | ||
| max: 3, | ||
| windowMs: 30_000, | ||
| }), | ||
| ) | ||
| .input(zSignInShare) | ||
| .mutation(async ({ input, ctx }) => { | ||
| const { password, shareId } = input; | ||
| const share = await getShareDashboardById(shareId); | ||
|
|
||
| if (!share) { | ||
| throw TRPCNotFoundError('Share not found'); | ||
| } | ||
|
|
||
| if (!share.public) { | ||
| throw TRPCNotFoundError('Share is not public'); | ||
| } | ||
|
|
||
| if (!share.password) { | ||
| throw TRPCNotFoundError('Share is not password protected'); | ||
| } | ||
|
|
||
| const validPassword = await verifyPasswordHash(share.password, password); | ||
|
|
||
| if (!validPassword) { | ||
| throw TRPCAccessError('Incorrect password'); | ||
| } | ||
|
|
||
| ctx.setCookie(`shared-dashboard-${shareId}`, '1', { | ||
| maxAge: 60 * 60 * 24 * 7, | ||
| ...COOKIE_OPTIONS, | ||
| }); | ||
|
|
||
| return true; | ||
| }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Refactor to eliminate code duplication.
The signInShareDashboard procedure is almost identical to signInShare (lines 332-368), with only two differences: the share fetching function and the cookie name. This code duplication increases maintenance burden and the risk of inconsistent behavior.
Consider extracting shared logic into a helper function:
async function signInShareHelper({
shareId,
password,
getShareById,
cookieName,
ctx,
}: {
shareId: string;
password: string;
getShareById: (id: string) => Promise<{ public: boolean; password: string | null } | null>;
cookieName: string;
ctx: any;
}) {
const share = await getShareById(shareId);
if (!share) {
throw TRPCNotFoundError('Share not found');
}
if (!share.public) {
throw TRPCNotFoundError('Share is not public');
}
if (!share.password) {
throw TRPCNotFoundError('Share is not password protected');
}
const validPassword = await verifyPasswordHash(share.password, password);
if (!validPassword) {
throw TRPCAccessError('Incorrect password');
}
ctx.setCookie(cookieName, '1', {
maxAge: 60 * 60 * 24 * 7,
...COOKIE_OPTIONS,
});
return true;
}Then both procedures can call this helper:
signInShare: publicProcedure
.use(rateLimitMiddleware({ max: 3, windowMs: 30_000 }))
.input(zSignInShare)
.mutation(async ({ input, ctx }) => {
return signInShareHelper({
shareId: input.shareId,
password: input.password,
getShareById: getShareOverviewById,
cookieName: `shared-overview-${input.shareId}`,
ctx,
});
}),
signInShareDashboard: publicProcedure
.use(rateLimitMiddleware({ max: 3, windowMs: 30_000 }))
.input(zSignInShare)
.mutation(async ({ input, ctx }) => {
return signInShareHelper({
shareId: input.shareId,
password: input.password,
getShareById: getShareDashboardById,
cookieName: `shared-dashboard-${input.shareId}`,
ctx,
});
}),🤖 Prompt for AI Agents
packages/trpc/src/routers/auth.ts lines 370-406: these lines duplicate the
sign-in logic already present for signInShare (lines ~332-368); extract the
shared flow into a helper function (e.g., signInShareHelper) that accepts
shareId, password, getShareById, cookieName and ctx, performs fetching,
existence/public/password checks, verifies the password, sets the cookie (using
COOKIE_OPTIONS) and returns true, then replace both signInShare and
signInShareDashboard mutation bodies to call this helper with the appropriate
getShareById (getShareOverviewById or getShareDashboardById) and cookieName
(`shared-overview-${shareId}` / `shared-dashboard-${shareId}`); ensure helper
has correct typing, imports (verifyPasswordHash, COOKIE_OPTIONS) and preserved
rateLimitMiddleware/input behavior.
What
In the same way the Overview view can be publicly shared, I find the need to publicly share individual dashboards to be equally or even more important. The beauty of dashboards is that one can create a custom-made view to present the KPIs that are most important to the person or a client that one is publicly sharing analytics with.
How
Following the pattern of the Overview view sharing implementation, the Dashboard sharing functionality is heavily inspired by the same patterns and structure. This is good for many reasons, including consistency & familiarity.
Summary by CodeRabbit