[Ready for Review] Add Report Issue dialog UI and wire it to Settings menu with Bugsink configured#164
[Ready for Review] Add Report Issue dialog UI and wire it to Settings menu with Bugsink configured#164eddie-wq07 wants to merge 2 commits intomainfrom
Conversation
| <DropdownMenuItem onSelect={() => NiceModal.show(ReportIssueDialog)}> | ||
| <Flag /> | ||
| <span> | ||
| Report a Bug <i>(Coming soon)</i> |
There was a problem hiding this comment.
I think we should do "Report an Issue" maybe, otherwise great work on the frontend
Backend Integration - Bug Report to BugsinkWhat this does:
Why:
What's included:
How to test:
Notes:
|
eddie-wq07
changed the title
Greptile SummaryThis PR adds the UI and partial backend wiring for a "Report Issue" feature — a
Additional style suggestions:
Confidence Score: 2/5
|
| Filename | Overview |
|---|---|
| src/server/api/routers/bug-report-router.ts | New tRPC router for bug report submission — uses publicProcedure (unauthenticated), no error handling around Sentry.captureMessage, and duplicates the Zod input schema from the frontend. |
| src/env.js | Adds required SENTRY_DSN env var — marking it required will break app startup for any developer without a local Bugsink/Sentry instance configured. |
| src/components/report-issue-dialog.tsx | New NiceModal dialog with react-hook-form and Zod validation — form state is not cleared on cancel/dismiss, and the Zod schema is duplicated from the backend router. |
| src/components/settings/settings-dropdown.tsx | Wires the new ReportIssueDialog into the Settings dropdown via NiceModal.show — straightforward integration, no issues. |
| src/instrumentation.ts | New Next.js instrumentation hook that initialises Sentry on Node.js runtime — uses a 100% trace sample rate which will be expensive in production, and doesn't guard against an unset SENTRY_DSN. |
| src/server/api/root.ts | Registers the new bugReportRouter — straightforward addition, no issues. |
| docker-compose.dev.yaml | Adds the Bugsink container for local development — hardcoded SECRET_KEY is acceptable in this dev-only file given the inline comment, but worth confirming it is never promoted to production. |
| next.config.js | Enables the instrumentationHook experimental flag required by Sentry — correct and expected change. |
Last reviewed commit: d0028dc
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }); | ||
|
|
||
| export const bugReportRouter = createTRPCRouter({ | ||
| submit: publicProcedure |
There was a problem hiding this comment.
Unauthenticated endpoint accepts arbitrary bug reports
The submit procedure uses publicProcedure, which means any unauthenticated caller — including bots or external scripts — can POST to this endpoint and inject arbitrary data into your Sentry/Bugsink instance. This opens the door to spam flooding the issue tracker and exhausting any API rate limits on the Sentry DSN.
The rest of the app uses authorizedProcedure for actions that should require a logged-in user. Since the dialog is only accessible from the Settings dropdown (which is already behind auth in the UI), using authorizedProcedure here aligns with that intent and closes the API-layer bypass:
| submit: publicProcedure | |
| submit: authorizedProcedure |
You'll also need to add the import at the top of the file:
import { authorizedProcedure } from "@/server/api/procedures";| MINIO_USE_SSL: z.enum(["true", "false"]).transform((val) => val === "true"), | ||
|
|
||
| // Sentry / Bugsink | ||
| SENTRY_DSN: z.string().url(), |
There was a problem hiding this comment.
Required SENTRY_DSN will break environments without Sentry configured
SENTRY_DSN is validated as a required z.string().url(), so the app will throw an env-validation error on startup for any developer who hasn't set up a Bugsink/Sentry instance locally. This is a regression — the app previously started fine without this variable.
Since the PR description explicitly notes that "backend submission is not included yet" and dev setups may not always have Bugsink running, this env var should be optional:
| SENTRY_DSN: z.string().url(), | |
| SENTRY_DSN: z.string().url().optional(), |
The instrumentation.ts init will need a corresponding guard:
if (process.env.NEXT_RUNTIME === "nodejs" && env.SENTRY_DSN) {
Sentry.init({ dsn: env.SENTRY_DSN, ... });
}| .mutation(async ({ input }) => { | ||
| // Send to Bugsink via Sentry SDK | ||
| Sentry.captureMessage(input.title, { | ||
| level: "info", | ||
| tags: { | ||
| type: "user_bug_report", | ||
| }, | ||
| extra: { | ||
| description: input.description, | ||
| email: input.email || "not provided", | ||
| }, | ||
| }); | ||
|
|
||
| return { success: true }; |
There was a problem hiding this comment.
No error handling around Sentry.captureMessage
Sentry.captureMessage() can throw (e.g., if the SDK was never initialised because SENTRY_DSN is unset). Currently, any exception will bubble up as an opaque tRPC internal-server error, and the client's onError toast will show a confusing message. Wrapping the call in a try/catch lets you surface a friendlier error:
try {
Sentry.captureMessage(input.title, {
level: "info",
tags: { type: "user_bug_report" },
extra: {
description: input.description,
email: input.email || "not provided",
},
});
} catch (err) {
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: "Failed to submit bug report. Please try again later.",
cause: err,
});
}| const ReportIssueSchema = z.object({ | ||
| title: z.string().min(1, "Title is required"), | ||
| description: z.string().min(1, "Description is required"), | ||
| email: z.string().email("Invalid email").optional().or(z.literal("")), | ||
| }); |
There was a problem hiding this comment.
Duplicated Zod schema between frontend and backend
ReportIssueSchema in the dialog is an exact copy of SubmitBugReportInput in bug-report-router.ts. If the validation rules ever change (e.g., a minimum length for the description), they'll need to be updated in two places and can easily drift.
Consider exporting the schema from the router file (or a shared schemas/ location) and importing it here:
// src/server/api/routers/bug-report-router.ts
export const SubmitBugReportInput = z.object({ ... });
// src/components/report-issue-dialog.tsx
import { SubmitBugReportInput } from "@/server/api/routers/bug-report-router";
type ReportIssueSchemaType = z.infer<typeof SubmitBugReportInput>;Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| onSuccess: () => { | ||
| toast.success("Issue reported successfully. Thank you!"); | ||
| modal.hide(); | ||
| form.reset(); | ||
| }, | ||
| onError: (error) => { | ||
| toast.error(`Failed to report issue: ${error.message}`); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Form data is not cleared when the dialog is dismissed without submitting
form.reset() is called only inside onSuccess. If a user partially fills the form and then closes the dialog via the Cancel button or the × icon, their draft input persists. The next time they open the dialog from the settings menu they'll see leftover text.
To clear state on every close, reset the form in the onOpenChange handler:
| onSuccess: () => { | |
| toast.success("Issue reported successfully. Thank you!"); | |
| modal.hide(); | |
| form.reset(); | |
| }, | |
| onError: (error) => { | |
| toast.error(`Failed to report issue: ${error.message}`); | |
| }, | |
| }); | |
| const submitBugReport = clientApi.bugReport.submit.useMutation({ | |
| onSuccess: () => { | |
| toast.success("Issue reported successfully. Thank you!"); | |
| modal.hide(); | |
| form.reset(); | |
| }, | |
| onError: (error) => { | |
| toast.error(`Failed to report issue: ${error.message}`); | |
| }, | |
| }); |
And update the onOpenChange callback:
onOpenChange={(open) => {
if (!open) {
form.reset();
modal.hide();
} else {
modal.show();
}
}}| if (process.env.NEXT_RUNTIME === "nodejs") { | ||
| Sentry.init({ | ||
| dsn: env.SENTRY_DSN, | ||
| tracesSampleRate: 1.0, |
There was a problem hiding this comment.
tracesSampleRate: 1.0 is too high for production
A sample rate of 1.0 means every single request is traced, which can generate a large volume of data and incur significant costs on Sentry or storage overhead on Bugsink once real traffic flows through. Consider lowering this for production or making it configurable:
| tracesSampleRate: 1.0, | |
| tracesSampleRate: env.NODE_ENV === "production" ? 0.1 : 1.0, |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
2 participants
What this PR does
ReportIssueDialogUI componentWhy
This sets up the user-facing UI for reporting issues so we can get early
feedback on wording, layout, and interaction before adding backend support.
What’s not included yet
How to test
Notes
Opening this PR early for UI/UX feedback before proceeding with backend work.