Add foreign key for profile default category#36
Conversation
WalkthroughProvisioning now upserts a default category (returning row when possible), falls back to SELECT by creator/name/kind if needed, resolves the category ID or throws, then upserts/updates the profile (updates email on conflict) and assigns the resolved Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Provision as ProvisionAccount
participant DB as Database
Client->>Provision: provisionAccount(req)
rect #eef7ff
Note over Provision,DB: Try insert default category (returning row)
Provision->>DB: INSERT INTO categories (...) RETURNING id
DB-->>Provision: insertedCategoryId or conflict (no row)
end
rect #f7fff0
Note over Provision,DB: Fallback resolve
alt inserted id returned
Provision-->>Provision: use insertedCategoryId
else no row returned
Provision->>DB: SELECT id FROM categories WHERE created_by = ... AND name = ... AND kind = ...
DB-->>Provision: existingCategoryId or none
end
alt no categoryId
Provision-->>Provision: throw Error("default category unresolved")
end
end
rect #fff7ee
Note over Provision,DB: Upsert profile and set FK
Provision->>DB: INSERT INTO profiles (...) ON CONFLICT(id) DO UPDATE SET email = ... RETURNING id
DB-->>Provision: profile inserted/updated
Provision->>DB: UPDATE profiles SET default_category_id = <resolvedId> WHERE id = ...
DB-->>Provision: updated / constraint error
end
Provision-->>Client: success / error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/auth/src/account/provision-account.ts (1)
77-90: ON CONFLICT predicate will not match the partial unique index if defaultCategoryKind is overridden.The partial unique index
categories_system_name_idxis hardcoded withWHERE kind = 'DEFAULT'(the literalDEFAULT_CATEGORY_KINDvalue). However, the ON CONFLICT clause useswhere: eq(categories.kind, defaultCategoryKind), which can be any string if overridden viaAccountProvisionerDependencies.defaultCategoryKind.When
defaultCategoryKinddiffers from the literalDEFAULT_CATEGORY_KIND, Postgres cannot match it to the partial index and will error: "there is no unique or exclusion constraint matching the ON CONFLICT specification".Required fixes:
- Use
ON CONFLICT ON CONSTRAINT categories_system_name_idxif your query builder supports constraint-based conflict resolution, or- Remove the
defaultCategoryKindoverride option from dependencies (enforce it as a constant), or- Add a guard rejecting non-default kinds in provisioning, or
- Add a full unique constraint on
(created_by, name, kind)instead of a partial index.
🧹 Nitpick comments (1)
packages/auth/src/account/provision-account.ts (1)
91-104: Category resolution LGTM.Insert-or-select pattern with
limit(1)is correct and RLS-safe for the owner.Optionally:
const [categoryRecord] = await tx.select(...).limit(1);improves readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/auth/src/account/provision-account.ts(3 hunks)packages/db/src/schema/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
packages/*/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/*/src/**/*.{ts,tsx}: Avoid implicit any; TypeScript is run with strict enabled
Prefer type guards or the satisfies operator over as casts where appropriate
Prefer unknown for external inputs
Use PascalCase for types and enums
Use camelCase for variables and functions
Ensure source comments are written in English
Files:
packages/db/src/schema/index.tspackages/auth/src/account/provision-account.ts
🧬 Code graph analysis (1)
packages/auth/src/account/provision-account.ts (2)
packages/db/src/schema/index.ts (2)
categories(63-111)profiles(25-59)packages/db/src/index.ts (2)
and(581-581)eq(581-581)
🔇 Additional comments (1)
packages/auth/src/account/provision-account.ts (1)
8-8: Import looks correct.
andis used in the category lookup predicate; import is appropriate.
gentamura
force-pushed
the
feature-add-default-category-fk
branch
from
f4be56a to
e1fb18a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/auth/src/account/provision-account.ts (1)
124-135: Consider removing redundant email update.The
Apply this diff to remove the redundant update:
const shouldUpdateProfile = await tx .update(profiles) .set({ defaultCategoryId, - email, }) .where(eq(profiles.id, params.userId)) .returning({ id: profiles.id });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/auth/src/account/provision-account.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
packages/*/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/*/src/**/*.{ts,tsx}: Avoid implicit any; TypeScript is run with strict enabled
Prefer type guards or the satisfies operator over as casts where appropriate
Prefer unknown for external inputs
Use PascalCase for types and enums
Use camelCase for variables and functions
Ensure source comments are written in English
Files:
packages/auth/src/account/provision-account.ts
🧬 Code graph analysis (1)
packages/auth/src/account/provision-account.ts (2)
packages/db/src/schema/index.ts (2)
profiles(25-60)categories(64-112)packages/db/src/index.ts (2)
and(581-581)eq(581-581)
🔇 Additional comments (5)
packages/auth/src/account/provision-account.ts (5)
8-8: LGTM: Import added for compound WHERE clause.The
andimport is correctly added and used at line 110 for the category resolution query.
77-88: Excellent: Profile insertion reordered to resolve circular FK constraint.Inserting the profile first without
defaultCategoryIdcorrectly breaks the circular dependency. The category can now referenceprofiles.idviacreatedBy, and the profile FK is set later after category resolution.
90-102: LGTM: Category insert with proper returning clause.The
.returning()clause correctly captures the inserted category ID. When a conflict occurs withonConflictDoNothing, the returned array will be empty, which is handled by the fallback query.
104-116: LGTM: Fallback query correctly resolves existing category.The fallback SELECT properly handles the case where
onConflictDoNothingreturns an empty array. The compound WHERE clause matches the unique constraint conditions, ensuring the correct category is retrieved.
118-120: LGTM: Appropriate error handling for category resolution failure.The guard correctly catches the unlikely case where the category cannot be resolved, providing a clear error message for debugging.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/auth/src/account/provision-account.ts (1)
90-102: Fix concurrency race: ON CONFLICT DO NOTHING + SELECT can fail under parallel provisioning.If another tx inserts the category and hasn’t committed yet, the fallback SELECT may not see it, causing a spurious “Failed to resolve default category” error. Use ON CONFLICT DO UPDATE ... RETURNING to always get the id from a single statement.
Apply this diff to eliminate the race and drop the fallback:
- const insertedCategories = await tx - .insert(categories) - .values({ - name: defaultCategoryName, - kind: defaultCategoryKind, - createdBy: params.userId, - updatedBy: params.userId, - }) - .onConflictDoNothing({ - target: [categories.createdBy, categories.name], - where: eq(categories.kind, defaultCategoryKind), - }) - .returning({ categoryId: categories.id }); - - const categoryRecord = - insertedCategories[0] ?? - ( - await tx - .select({ categoryId: categories.id }) - .from(categories) - .where( - and( - eq(categories.createdBy, params.userId), - eq(categories.name, defaultCategoryName), - eq(categories.kind, defaultCategoryKind), - ), - ) - .limit(1) - )[0]; + const [categoryRecord] = await tx + .insert(categories) + .values({ + name: defaultCategoryName, + kind: defaultCategoryKind, + createdBy: params.userId, + updatedBy: params.userId, + }) + .onConflictDoUpdate({ + target: [categories.createdBy, categories.name], + where: eq(categories.kind, defaultCategoryKind), + // Touch a column to ensure UPDATE path and RETURNING always yield a row. + set: { updatedBy: params.userId }, + }) + .returning({ categoryId: categories.id });If you adopt this, remove the now-unused
andimport. For example:-import { - and, - categories, +import { + categories, createRlsClient, DEFAULT_CATEGORY_KIND, DEFAULT_CATEGORY_NAME, eq, profiles, } from "@listee/db";Also applies to: 104-118
🧹 Nitpick comments (2)
packages/auth/src/account/provision-account.ts (2)
83-88: Good: upsert profile email on conflict.Prevents stale emails on re-provision. Consider normalizing to lowercase when setting email to avoid future unique collisions from differing cases.
126-133: Rename result variable and (optionally) avoid no-op updates.
shouldUpdateProfileis an array of updated rows; rename for clarity.- Optional: guard the UPDATE so you only write when
defaultCategoryIdchanges.Apply this minimal rename:
- const shouldUpdateProfile = await tx + const updatedProfiles = await tx .update(profiles) .set({ defaultCategoryId, email, }) .where(eq(profiles.id, params.userId)) .returning({ id: profiles.id }); - if (shouldUpdateProfile.length === 0) { + if (updatedProfiles.length === 0) { throw new Error("Failed to update profile with default category"); }Optional guard idea (pseudo with intent): add extra predicates so the UPDATE runs only when values differ.
Also applies to: 135-137
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/auth/src/account/provision-account.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
packages/*/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/*/src/**/*.{ts,tsx}: Avoid implicit any; TypeScript is run with strict enabled
Prefer type guards or the satisfies operator over as casts where appropriate
Prefer unknown for external inputs
Use PascalCase for types and enums
Use camelCase for variables and functions
Ensure source comments are written in English
Files:
packages/auth/src/account/provision-account.ts
🧬 Code graph analysis (1)
packages/auth/src/account/provision-account.ts (2)
packages/db/src/schema/index.ts (2)
profiles(25-60)categories(64-112)packages/db/src/index.ts (2)
and(581-581)eq(581-581)
1 participant
Summary
profiles.default_category_idTesting
Summary by CodeRabbit
Bug Fixes
Database Schema