[#392] Create issue templates schema and router#419
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded issue template management: two new permissions, a new Template DB table (with JSONB body), and four API router procedures for creating, updating, deleting, and listing templates plus supporting schemas/types for template sub-issues. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as IssuesRouter
participant Auth as PermissionsCheck
participant DB as TemplatesTable
Client->>API: POST /createTemplate (name, body)
API->>Auth: verify `EDIT_ISSUE_TEMPLATES`
Auth-->>API: allowed / denied
alt allowed
API->>DB: INSERT Template (name, body)
DB-->>API: inserted record
API-->>Client: 200 Created (template)
else denied
API-->>Client: 403 Forbidden
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
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.
🧹 Nitpick comments (4)
packages/consts/src/permissions.ts (1)
118-127: Minor: Inconsistent title casing innamefields.Other permission entries use Title Case (e.g., "Read Issues", "Edit Forms"), but these use lowercase ("Edit issue templates", "Read issue templates"). Consider updating for consistency:
✏️ Suggested fix for consistency
EDIT_ISSUE_TEMPLATES: { idx: 22, - name: "Edit issue templates", + name: "Edit Issue Templates", desc: "Allows creating, editing, or deleting templates.", }, READ_ISSUE_TEMPLATES: { idx: 23, - name: "Read issue templates", + name: "Read Issue Templates", desc: "Grants access to issue templates.", },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/consts/src/permissions.ts` around lines 118 - 127, The permission entries EDIT_ISSUE_TEMPLATES and READ_ISSUE_TEMPLATES use inconsistent lowercased names; update their name fields to Title Case to match the rest of the file (change "Edit issue templates" to "Edit Issue Templates" and "Read issue templates" to "Read Issue Templates") so naming is consistent across permissions.packages/api/src/routers/templates.ts (2)
24-27: Consider limiting recursion depth for the nested children schema.The recursive
templateSubIssueSchemaallows unbounded nesting depth. While Zod handles this gracefully, deeply nested payloads could impact performance and storage. You might want to enforce a practical limit.🛡️ Example: Enforce max depth of 3 levels
// Helper to create depth-limited schema const createDepthLimitedSchema = (maxDepth: number): z.ZodType<TemplateSubIssue> => { if (maxDepth <= 0) { return baseTemplateSubIssueSchema as z.ZodType<TemplateSubIssue>; } return baseTemplateSubIssueSchema.extend({ children: z.array(createDepthLimitedSchema(maxDepth - 1)).optional(), }) as z.ZodType<TemplateSubIssue>; }; const templateSubIssueSchema = createDepthLimitedSchema(3);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/templates.ts` around lines 24 - 27, The recursive templateSubIssueSchema allows unbounded nesting; replace it with a depth-limited constructor to prevent deeply nested payloads impacting performance/storage: implement a helper like createDepthLimitedSchema(maxDepth) that returns baseTemplateSubIssueSchema when maxDepth <= 0 and otherwise extends baseTemplateSubIssueSchema with children: z.array(createDepthLimitedSchema(maxDepth - 1)).optional(), then assign templateSubIssueSchema = createDepthLimitedSchema(3) (or another practical limit) and update any references to templateSubIssueSchema accordingly.
116-127: Consider adding pagination forgetTemplates.Currently returns all templates without limit. This is fine for a small dataset, but as templates grow, you may want pagination to avoid large payloads.
📄 Example pagination approach
getTemplates: permProcedure .input(z.object({ limit: z.number().int().min(1).max(100).default(50), cursor: z.string().uuid().optional(), }).optional()) .query(async ({ ctx, input }) => { permissions.controlPerms.or( ["READ_ISSUE_TEMPLATES", "EDIT_ISSUE_TEMPLATES"], ctx, ); const templates = await db.query.Template.findMany({ limit: input?.limit ?? 50, orderBy: (templates, { desc }) => [desc(templates.createdAt)], // Add cursor-based pagination if needed }); return templates; }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/templates.ts` around lines 116 - 127, Add cursor/limit pagination to the getTemplates procedure: change getTemplates (permProcedure.query) to accept an optional input schema (z.object with limit:number default 50, min1 max100, and optional cursor:string/uuid) and enforce permissions via permissions.controlPerms.or as before; then pass input?.limit (or default) into db.query.Template.findMany and implement cursor-based filtering using the cursor (e.g., where createdAt < cursor timestamp or use startAfter/startFrom API of the ORM) and preserve the orderBy (desc createdAt); return the page of templates and the nextCursor to the caller.packages/db/src/schemas/knight-hacks.ts (1)
619-629: Consider adding an index oncreatedAtfor query performance.The
getTemplatesprocedure inpackages/api/src/routers/templates.tsorders results bycreatedAtdescending. Without an index, this becomes a full table scan as the template count grows.📊 Suggested index addition
export const Template = createTable("template", (t) => ({ id: t.uuid().notNull().primaryKey().defaultRandom(), name: t.text().notNull(), body: t.jsonb().notNull(), createdAt: t.timestamp().defaultNow().notNull(), updatedAt: t .timestamp() .defaultNow() .$onUpdate(() => new Date()) .notNull(), -})); +}), (table) => ({ + createdAtIdx: index("template_created_at_idx").on(table.createdAt), +}));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/db/src/schemas/knight-hacks.ts` around lines 619 - 629, Add a DB index on Template.createdAt to support the getTemplates ordering: locate the Template table definition (symbol Template from createTable) and add an index on the createdAt column (either via your schema helper e.g. createIndex for the "template" table or create a migration that adds an index named something like template_created_at_idx on createdAt) so ORDER BY createdAt DESC in getTemplates uses the index for efficient scans.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/api/src/routers/templates.ts`:
- Around line 24-27: The recursive templateSubIssueSchema allows unbounded
nesting; replace it with a depth-limited constructor to prevent deeply nested
payloads impacting performance/storage: implement a helper like
createDepthLimitedSchema(maxDepth) that returns baseTemplateSubIssueSchema when
maxDepth <= 0 and otherwise extends baseTemplateSubIssueSchema with children:
z.array(createDepthLimitedSchema(maxDepth - 1)).optional(), then assign
templateSubIssueSchema = createDepthLimitedSchema(3) (or another practical
limit) and update any references to templateSubIssueSchema accordingly.
- Around line 116-127: Add cursor/limit pagination to the getTemplates
procedure: change getTemplates (permProcedure.query) to accept an optional input
schema (z.object with limit:number default 50, min1 max100, and optional
cursor:string/uuid) and enforce permissions via permissions.controlPerms.or as
before; then pass input?.limit (or default) into db.query.Template.findMany and
implement cursor-based filtering using the cursor (e.g., where createdAt <
cursor timestamp or use startAfter/startFrom API of the ORM) and preserve the
orderBy (desc createdAt); return the page of templates and the nextCursor to the
caller.
In `@packages/consts/src/permissions.ts`:
- Around line 118-127: The permission entries EDIT_ISSUE_TEMPLATES and
READ_ISSUE_TEMPLATES use inconsistent lowercased names; update their name fields
to Title Case to match the rest of the file (change "Edit issue templates" to
"Edit Issue Templates" and "Read issue templates" to "Read Issue Templates") so
naming is consistent across permissions.
In `@packages/db/src/schemas/knight-hacks.ts`:
- Around line 619-629: Add a DB index on Template.createdAt to support the
getTemplates ordering: locate the Template table definition (symbol Template
from createTable) and add an index on the createdAt column (either via your
schema helper e.g. createIndex for the "template" table or create a migration
that adds an index named something like template_created_at_idx on createdAt) so
ORDER BY createdAt DESC in getTemplates uses the index for efficient scans.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: c520a73e-2aae-4534-9585-456e707883aa
📒 Files selected for processing (4)
packages/api/src/root.tspackages/api/src/routers/templates.tspackages/consts/src/permissions.tspackages/db/src/schemas/knight-hacks.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/api/src/routers/issues.ts`:
- Around line 299-303: The router is exposing table-owned fields by reusing
InsertTemplateSchema for public create inputs; instead define an explicit public
input schema (e.g., CreateTemplateInputSchema or TemplatePublicCreateSchema)
that only includes the allowed fields (name: z.string().min(1, "...") and body:
z.array(templateSubIssueSchema)) and use that in the tRPC route instead of
InsertTemplateSchema.extend(...); update both places that currently derive from
InsertTemplateSchema so the public write shape remains explicit and cannot
accidentally include new DB columns.
- Around line 35-41: The schema currently uses a numeric-only dateMs field which
rejects template date strings and stores the wrong key; update
baseTemplateSubIssueSchema to replace dateMs with a string-based date field
named "date" (optional) that accepts template expressions (e.g., "{-7 days}"),
and then update any usages such as createTemplate and updateTemplate to
read/write the "date" key instead of "dateMs" so the persisted template JSON
matches the feature contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 677b1b56-bce1-46f5-9550-790f3729c5a1
📒 Files selected for processing (1)
packages/api/src/routers/issues.ts
DVidal1205
left a comment
There was a problem hiding this comment.
Superb, pushing schema and merging now!
Why
To have templates to quickly make new issues
What
Closes: #392
Closes: #393
Added the template schema, a template router, and procedures to create, update, delete, and get templates.
Test Plan
Tested by checking that all new procedures could be called successfully from a server component and that the database was updated correctly.
Checklist
db:pushbefore mergingSummary by CodeRabbit