feat: add web FAQs schema and router#35
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds migrations and Drizzle snapshots, TypeScript table schemas, tRPC CRUD routes, and router wiring to implement a bilingual (IT/EN) FAQ system with audit fields and cascade deletes. ChangesFAQ Management Feature
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/routers/tg/groups.ts (1)
176-176: 💤 Low valueConsider using
> 0for cache invalidation defensively.The
=== 1check assumestelegramIduniqueness. While this is likely correct, using> 0for invalidation would be more defensive against data anomalies, while still returning=== 1for the API contract.♻️ Suggested defensive invalidation
const rows = await DB.delete(GROUPS).where(eq(GROUPS.telegramId, input.telegramId)).returning() -if (rows.length === 1) invalidateGroupsCache() +if (rows.length > 0) invalidateGroupsCache() return rows.length === 1Same pattern for
setHide:-if (rows.length === 1) invalidateGroupsCache() +if (rows.length > 0) invalidateGroupsCache() return rows.length === 1Also applies to: 193-193
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routers/tg/groups.ts` at line 176, The cache invalidation currently checks for rows.length === 1 which assumes telegramId uniqueness; change the condition to rows.length > 0 so any deleted/updated rows cause invalidateGroupsCache() to run defensively; update both occurrences where invalidateGroupsCache() is called after DB operations (the block with if (rows.length === 1) invalidateGroupsCache() and the similar check in setHide) to use > 0 while keeping the API contract that callers still expect a single-row result where appropriate.drizzle/0013_oval_dreaming_celestial.sql (1)
15-25: ⚡ Quick winIndex
web_faqs.category_idto prevent FK/cascade scan bottlenecks.
category_idis constrained by FK, but it is not indexed. On larger datasets, category reads and category deletions with cascade can trigger slower scans.💡 Suggested migration addition
ALTER TABLE "web_faqs" ADD CONSTRAINT "web_faqs_modified_by_id_tg_permissions_user_id_fk" FOREIGN KEY ("modified_by_id") REFERENCES "public"."tg_permissions"("user_id") ON DELETE no action ON UPDATE no action; +--> statement-breakpoint +CREATE INDEX "web_faqs_category_id_idx" ON "web_faqs" ("category_id");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drizzle/0013_oval_dreaming_celestial.sql` around lines 15 - 25, Add an index on web_faqs.category_id to avoid FK/cascade scan bottlenecks: create an index (preferably concurrently in production) on the column referenced by the foreign key (web_faqs.category_id) so operations involving the constraint web_faqs_category_id_web_faq_categories_id_fk run efficiently; include the index creation in the migration that introduces or follows the FK (name it clearly, e.g., idx_web_faqs_category_id) and ensure the migration handles existence checks if rerunnable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/routers/web/faqs.ts`:
- Around line 12-27: The response schema for getAllFaqs currently omits category
and faq IDs; update the output Zod schemas used in getAllFaqs to include id
fields (e.g., add id: z.string() or z.number() as appropriate) on the category
object and on each faq object so edit/delete mutations can target records; apply
the same addition in both schema blocks referenced (the top-level categories
array and the nested faqs array) and keep nullable/optional types consistent
with your DB model.
- Around line 50-187: All mutating endpoints (addFaqs, addFaqsCategory,
editFaqs, editFaqsCategory, deleteFaqs, deleteFaqsCategory) are currently
public; add explicit auth/role checks at the start of each mutation to require
an authenticated user and appropriate role (e.g., admin or faq-manager) by
inspecting the request context (ctx.user or ctx.session) and fail fast with an
authorization error (throw TRPCError with code "UNAUTHORIZED" or return { error:
"UNAUTHORIZED" }). Also ensure createdBy/modifiedBy values come from ctx.user.id
(or validate they match) instead of trusting client input. Implement the checks
consistently in each mutation before performing DB operations.
---
Nitpick comments:
In `@drizzle/0013_oval_dreaming_celestial.sql`:
- Around line 15-25: Add an index on web_faqs.category_id to avoid FK/cascade
scan bottlenecks: create an index (preferably concurrently in production) on the
column referenced by the foreign key (web_faqs.category_id) so operations
involving the constraint web_faqs_category_id_web_faq_categories_id_fk run
efficiently; include the index creation in the migration that introduces or
follows the FK (name it clearly, e.g., idx_web_faqs_category_id) and ensure the
migration handles existence checks if rerunnable.
In `@src/routers/tg/groups.ts`:
- Line 176: The cache invalidation currently checks for rows.length === 1 which
assumes telegramId uniqueness; change the condition to rows.length > 0 so any
deleted/updated rows cause invalidateGroupsCache() to run defensively; update
both occurrences where invalidateGroupsCache() is called after DB operations
(the block with if (rows.length === 1) invalidateGroupsCache() and the similar
check in setHide) to use > 0 while keeping the API contract that callers still
expect a single-row result where appropriate.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2702614c-ded4-4aad-b0d5-23d8785c3b87
📒 Files selected for processing (11)
drizzle/0013_oval_dreaming_celestial.sqldrizzle/0014_remarkable_synch.sqldrizzle/meta/0013_snapshot.jsondrizzle/meta/0014_snapshot.jsondrizzle/meta/_journal.jsonsrc/db/schema/web/faqs.tssrc/db/schema/web/index.tssrc/routers/index.tssrc/routers/tg/groups.tssrc/routers/web/faqs.tssrc/routers/web/index.ts
No description provided.