cud handler #1061
cud handler #1061
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Preview Screenshots⏳ Preview screenshots are being captured... Workspace and dev browser links will appear here once the preview environment is ready. Generated by cmux preview system |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughCentralized CRUD handlers were introduced via a new CUD infrastructure and applied to internal email-theme routes; route handlers now delegate create/read/update/delete operations to typed Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Route as API Route Handler
participant Cud as CUD Handlers
participant Env as Env Config / Overrides
rect rgb(200,220,255)
Note over Client,Env: CREATE flow
Client->>Route: POST /email-themes (display_name)
Route->>Cud: adminCreate(tenancy, {display_name, id?})
Cud->>Cud: validate & generate id
Cud->>Env: apply override (add theme TSX)
Env-->>Cud: confirmation
Cud-->>Route: created theme {id, display_name}
Route-->>Client: 200 + theme
end
rect rgb(220,240,220)
Note over Client,Env: READ flow
Client->>Route: GET /email-themes or /email-themes/[id]
Route->>Cud: adminList/adminRead(tenancy, params)
Cud->>Env: read config/themes
Env-->>Cud: theme data
Cud-->>Route: theme(s)
Route-->>Client: 200 + theme(s)
end
rect rgb(255,240,200)
Note over Client,Env: UPDATE flow
Client->>Route: PATCH /email-themes/[id] (tsx_source)
Route->>Cud: adminUpdate(tenancy, params, data)
Cud->>Cud: validate ids, render preview
Cud->>Env: apply override (update TSX)
Env-->>Cud: confirmation
Cud->>Cud: fetch updated item
Cud-->>Route: updated theme
Route-->>Client: 200 + updated theme
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Greptile OverviewGreptile SummaryThis PR refactors email theme management to use a new generic CUD (Create/Update/Delete) handler pattern, consolidating CRUD operations into reusable abstractions. Key Changes:
Issues Found:
Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Client
participant RouteHandler
participant CudHandler
participant EmailThemesCud
participant Database
Note over Client,Database: Email Theme Update Flow (PATCH)
Client->>RouteHandler: PATCH /email-themes/:id
RouteHandler->>CudHandler: adminUpdate(tenancy, id, data)
CudHandler->>CudHandler: validate params & data
CudHandler->>EmailThemesCud: onUpdate(auth, params, data)
alt Empty data (Read/List)
EmailThemesCud->>EmailThemesCud: getThemeOrThrow(tenancy, id)
EmailThemesCud-->>CudHandler: return theme from tenancy.config
else Update operation
EmailThemesCud->>EmailThemesCud: validate tsx_source via renderEmailWithTemplate
EmailThemesCud->>Database: overrideEnvironmentConfigOverride
Database-->>EmailThemesCud: config updated
Note over EmailThemesCud: Uses stale auth.tenancy.config
EmailThemesCud->>EmailThemesCud: construct updatedThemes from stale config
EmailThemesCud-->>CudHandler: return items with updated tsx_source
end
CudHandler->>CudHandler: validate output
CudHandler-->>RouteHandler: return result
RouteHandler->>RouteHandler: extract theme by id
RouteHandler-->>Client: return display_name
Note over Client,Database: Email Theme Create Flow (POST)
Client->>RouteHandler: POST /email-themes
RouteHandler->>CudHandler: adminCreate(tenancy, data)
CudHandler->>EmailThemesCud: onCreate(auth, data)
EmailThemesCud->>Database: overrideEnvironmentConfigOverride
EmailThemesCud-->>CudHandler: return created id
CudHandler->>EmailThemesCud: onUpdate with create-forward data
EmailThemesCud-->>CudHandler: return created theme
CudHandler-->>RouteHandler: return result
RouteHandler-->>Client: return id
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/backend/src/route-handlers/cud-handler.tsx (1)
323-378: Consider using an ES6 Map for direct calls accumulation.Per coding guidelines, prefer ES6 Maps over records where possible. The
directCallsarray is later spread intoObject.fromEntries, but the overall pattern could benefit from using a Map for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsx(3 hunks)apps/backend/src/app/api/latest/internal/email-themes/cud.tsx(1 hunks)apps/backend/src/app/api/latest/internal/email-themes/route.tsx(3 hunks)apps/backend/src/route-handlers/cud-handler.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Always add new E2E tests when changing the API or SDK interface
For blocking alerts and errors, never use toast; use alerts instead as they are less easily missed by the user
NEVER try-catch-all, NEVER void a promise, and NEVER .catch(console.error); use loading indicators and async callbacks instead, or use runAsynchronously/runAsynchronouslyWithAlert for error handling
Use ES6 maps instead of records wherever you can
Files:
apps/backend/src/app/api/latest/internal/email-themes/cud.tsxapps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsxapps/backend/src/route-handlers/cud-handler.tsxapps/backend/src/app/api/latest/internal/email-themes/route.tsx
**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,css}: Keep hover/click transitions snappy and fast; avoid fade-in delays on hover. Apply transitions after action completion instead, like smooth fade-out when hover ends
Use hover-exit transitions instead of hover-enter transitions; for example, use 'transition-colors hover:transition-none' instead of fade-in on hover
Files:
apps/backend/src/app/api/latest/internal/email-themes/cud.tsxapps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsxapps/backend/src/route-handlers/cud-handler.tsxapps/backend/src/app/api/latest/internal/email-themes/route.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
NEVER use Next.js dynamic functions if you can avoid them; prefer using client components and hooks like usePathname instead of await params to keep pages static
Files:
apps/backend/src/app/api/latest/internal/email-themes/cud.tsxapps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsxapps/backend/src/route-handlers/cud-handler.tsxapps/backend/src/app/api/latest/internal/email-themes/route.tsx
{.env*,**/*.{ts,tsx,js}}
📄 CodeRabbit inference engine (AGENTS.md)
Prefix environment variables with STACK_ (or NEXT_PUBLIC_STACK_ if public) so changes are picked up by Turborepo and improves readability
Files:
apps/backend/src/app/api/latest/internal/email-themes/cud.tsxapps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsxapps/backend/src/route-handlers/cud-handler.tsxapps/backend/src/app/api/latest/internal/email-themes/route.tsx
apps/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Check existing apps for inspiration when implementing new apps or pages; update apps-frontend.tsx and apps-config.ts to add new apps
Files:
apps/backend/src/app/api/latest/internal/email-themes/cud.tsxapps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsxapps/backend/src/route-handlers/cud-handler.tsxapps/backend/src/app/api/latest/internal/email-themes/route.tsx
🧬 Code graph analysis (1)
apps/backend/src/app/api/latest/internal/email-themes/route.tsx (2)
apps/backend/src/app/api/latest/internal/email-themes/cud.tsx (1)
internalEmailThemesCudHandlers(68-203)packages/stack-shared/src/utils/errors.tsx (1)
StatusError(152-261)
🪛 Biome (2.1.2)
apps/backend/src/route-handlers/cud-handler.tsx
[error] 134-134: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: lint_and_build (latest)
- GitHub Check: all-good
- GitHub Check: Vercel Agent Review
- GitHub Check: docker
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test-with-custom-base-port
- GitHub Check: restart-dev-and-test
🔇 Additional comments (15)
apps/backend/src/app/api/latest/internal/email-themes/route.tsx (2)
8-44: LGTM!The POST handler correctly delegates theme creation to the centralized CUD handler, generating a UUID upfront and passing the necessary tenancy context and error types.
46-83: LGTM!The GET handler properly uses
adminListand maps the result items to the expected response shape, filtering to only includeidanddisplay_namefields.apps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsx (2)
6-42: LGTM!The GET handler correctly delegates to
adminReadand returns the theme data with the proper field names.
44-89: LGTM!The PATCH handler properly wraps the update data in an array as expected by the batch update API and includes defensive error handling if the updated theme cannot be found in the result.
apps/backend/src/app/api/latest/internal/email-themes/cud.tsx (4)
15-35: LGTM!The helper types and functions are well-structured.
getThemeOrThrowproperly usesObject.prototype.hasOwnProperty.callto safely check for theme existence.
37-66: LGTM!The CRUD schema definition is well-organized with appropriate field types and hidden documentation flags.
72-87: LGTM!The
onCreatehandler correctly generates a UUID if not provided and initializes the theme withLightEmailThemeas the default TSX source.
177-202: LGTM!The
onDeletehandler properly validates theme existence before deletion and returns the remaining themes after the operation.apps/backend/src/route-handlers/cud-handler.tsx (7)
1-44: LGTM!The imports and type definitions are well-structured, providing clear typing for batch operations and CUD route handlers.
126-138: Static analysis false positive -.when()conditional is valid Yup syntax.The Biome warning about "Do not add then to an object" is a false positive. The
thenhere is part of Yup's.when()conditional validation API, not a property being added to a plain object. The schema correctly defines conditional pagination requirements.
93-159: LGTM!The
createCudHandlersfunction signature and helper functions (getBatchSchema,getListOutputSchema,getSchemas) are well-designed. The lazy batch schema properly handles both single items and arrays.
161-259: LGTM!The operations mapping and access type filtering logic is sound. The
invokeListandinvokeReadfunctions properly validate params, handle the prepare hook, and route to the appropriate operation handlers.
366-371: VerifyallowedErrorTypescheck handles inheritance correctly.The
instanceofcheck works for direct instances, but ensure that subclasses of allowed error types are also properly handled if that's the intended behavior.
388-415: LGTM!The
CudHandlerInvocationErrorclass provides clear error wrapping with cause chain, and thevalidatefunction properly handles Yup validation errors with descriptive messages.
417-589: Good test coverage for CUD handler operations.The inline vitest tests cover list, read, update, create, and delete operations with proper assertions on call counts and response shapes.
apps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/backend/src/app/api/latest/internal/email-themes/cud.tsx`:
- Around line 179-194: The onDelete handler currently sends the whole
emails.themes object to overrideEnvironmentConfigOverride; change it to delete
each theme using dot-notation keys consistent with onCreate/onUpdate: for each
id validated by getThemeOrThrow in onDelete, call
overrideEnvironmentConfigOverride with environmentConfigOverrideOverride mapping
"emails.themes.${id}" to null (or the API's deletion sentinel) instead of
sending the full themes object, mirroring the pattern used in onCreate/onUpdate
to avoid the inconsistent update style.
♻️ Duplicate comments (1)
apps/backend/src/app/api/latest/internal/email-themes/cud.tsx (1)
160-166: Stale tenancy data after config persistence - fragile pattern.After
overrideEnvironmentConfigOverridepersists changes to the database (lines 151-158), the code still reads from the in-memoryauth.tenancy.config.emails.themeswhich hasn't been refreshed. The merge logic at lines 162-165 partially compensates by overlaying the newtsxSourcevalues, but this is fragile.The current approach works because you're manually merging the updated values, but it's error-prone if more fields need updating in the future.
Consider building response purely from resolvedUpdates
- const updatedThemes = { - ...auth.tenancy.config.emails.themes, - ...Object.fromEntries(resolvedUpdates.map((u) => { - const current = getThemeOrThrow(auth.tenancy, u.id!); - return [u.id!, { ...current, tsxSource: u.tsx_source }]; - })), - }; - - const items: ThemeRead[] = typedEntries(updatedThemes).map(([id, theme]) => ({ - id, - display_name: theme.displayName, - tsx_source: theme.tsxSource, - })); + // Capture display names before persisting (from stale tenancy) + const displayNames = new Map(resolvedUpdates.map((u) => { + const current = getThemeOrThrow(auth.tenancy, u.id!); + return [u.id!, current.displayName] as const; + })); + + // Build response from the updates we just applied + const updatedItems: ThemeRead[] = resolvedUpdates.map((u) => ({ + id: u.id!, + display_name: displayNames.get(u.id!) ?? '', + tsx_source: u.tsx_source, + })); + + // For list operations, include unchanged themes from stale tenancy + const unchangedItems: ThemeRead[] = paramsId ? [] : typedEntries(auth.tenancy.config.emails.themes) + .filter(([id]) => !resolvedUpdates.some((u) => u.id === id)) + .map(([id, theme]) => ({ + id, + display_name: theme.displayName, + tsx_source: theme.tsxSource, + })); + + const items = [...updatedItems, ...unchangedItems];
🧹 Nitpick comments (5)
apps/backend/src/route-handlers/cud-handler.tsx (4)
129-136: Biome error:thenproperty on object can cause issues with Promise-like behavior.The
.when('is_paginated', { ... then: (schema) => ... })syntax triggers a Biome lint error because adding athenproperty to an object can cause it to be treated as a thenable, leading to unexpected behavior in async contexts.This is a yup-specific API pattern, so the warning is a false positive in this context. However, consider suppressing it explicitly to document the intentional usage.
Suppress the lint warning
pagination: yupObject({ next_cursor: yupString().nullable().defined().meta({ openapiField: { description: "The cursor to fetch the next page of results. null if there is no next page.", exampleValue: 'b3d396b8-c574-4c80-97b3-50031675ceb2' } }), + // biome-ignore lint/suspicious/noThenProperty: yup's .when() API uses `then` as a key, not as Promise thenable }).when('is_paginated', { is: true, then: (schema) => schema.defined(), otherwise: (schema) => schema.optional(), }),
335-371: Consider reducinganyusage inmakeDirectInvoke.The destructured parameters and casts use
anyextensively without explanatory comments. Per coding guidelines,anyshould be accompanied by a comment explaining why it's necessary.Add comments explaining `any` usage
const makeDirectInvoke = (entry: (typeof accessTypeEntries) extends Map<unknown, infer V> ? V : never, accessType: "client" | "server" | "admin", directOperation: CudOperation) => { + // Using `any` here because the function signature is dynamically generated based on operation type + // and access level. Type safety is enforced at the schema validation layer via yup. return async ({ user, project, branchId, tenancy, data, query, allowedErrorTypes, ...params }: any) => {
384-384: Return type cast toanyloses type safety.The
as anycast at the end ofcreateCudHandlersbypasses TypeScript's type checking for the returned object. While the complex type signature makes this difficult to avoid, consider adding a comment explaining the limitation.- ) as any; + // Type assertion needed because Object.fromEntries loses the precise typing + // of the dynamically constructed handler map. Type safety is enforced via + // the CudHandlers<...> return type annotation. + ) as any;
513-518: Test helper uses type assertion without tenancy verification.The
makeAuthhelper casts incomplete mock objects toany, which could mask issues if the realSmartRequestAuthshape changes.Consider using a more complete mock or
Partial<SmartRequestAuth>with explicit type assertions only where necessary.apps/backend/src/app/api/latest/internal/email-themes/cud.tsx (1)
90-111: TheonUpdatefunction handles multiple responsibilities.This function handles three distinct cases: listing (empty data), create-forwarding (data with
display_name), and actual updates. While functional, this overloading makes the logic harder to follow and maintain.Consider documenting these cases with comments, or refactoring to separate concerns if the CUD handler design allows it.
onUpdate: async ({ auth, params, data }) => { const paramsId = params.id; + // Case 1: Read/List operation (no update data) if (data.length === 0) { if (paramsId) { const theme = getThemeOrThrow(auth.tenancy, paramsId);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsxapps/backend/src/app/api/latest/internal/email-themes/cud.tsxapps/backend/src/route-handlers/cud-handler.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: For blocking alerts and errors, never usetoast, as they are easily missed by the user. Instead, use alerts
Keep hover/click transitions snappy and fast without pre-transition delays (e.g., no fade-in when hovering a button). Apply transitions after the action, like smooth fade-out when hover ends
NEVER try-catch-all, NEVER void a promise, and NEVER .catch(console.error). Use loading indicators for async operations. UserunAsynchronouslyorrunAsynchronouslyWithAlertinstead of general try-catch error handling
When creating hover transitions, avoid hover-enter transitions and use only hover-exit transitions (e.g.,transition-colors hover:transition-none)
Don't useDate.now()for measuring elapsed (real) time; instead useperformance.now()
Use ES6 maps instead of records wherever possible
Files:
apps/backend/src/route-handlers/cud-handler.tsxapps/backend/src/app/api/latest/internal/email-themes/cud.tsxapps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: NEVER use Next.js dynamic functions if you can avoid them. Prefer using client components to keep pages static (e.g., useusePathnameinstead ofawait params)
Code defensively using?? throwErr(...)instead of non-null assertions, with good error messages explicitly stating violated assumptions
Try to avoid theanytype. When usingany, leave a comment explaining why and how the type system fails or how errors would still be caught
Files:
apps/backend/src/route-handlers/cud-handler.tsxapps/backend/src/app/api/latest/internal/email-themes/cud.tsxapps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsx
{.env*,**/*.{ts,tsx,js,jsx}}
📄 CodeRabbit inference engine (AGENTS.md)
All environment variables should be prefixed with
STACK_(orNEXT_PUBLIC_STACK_if public) to ensure Turborepo picks up changes and improve readability
Files:
apps/backend/src/route-handlers/cud-handler.tsxapps/backend/src/app/api/latest/internal/email-themes/cud.tsxapps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsx
🧠 Learnings (2)
📚 Learning: 2026-01-13T18:14:29.974Z
Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T18:14:29.974Z
Learning: Applies to packages/stack-shared/src/config/schema.ts : Whenever making backwards-incompatible changes to the config schema, update the migration functions in `packages/stack-shared/src/config/schema.ts`
Applied to files:
apps/backend/src/app/api/latest/internal/email-themes/cud.tsx
📚 Learning: 2026-01-08T20:30:36.983Z
Learnt from: nams1570
Repo: stack-auth/stack-auth PR: 1091
File: apps/backend/src/lib/email-rendering.tsx:49-54
Timestamp: 2026-01-08T20:30:36.983Z
Learning: In apps/backend/src/lib/email-rendering.tsx, the nodeModules constant intentionally pins React and react-dom to version 19.1.1 (rather than matching the project's React version) because pinning provides more reliability for email rendering in the sandbox environment.
Applied to files:
apps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsx
🧬 Code graph analysis (2)
apps/backend/src/route-handlers/cud-handler.tsx (8)
packages/stack-shared/src/crud.tsx (2)
CrudTypeOf(117-121)CrudSchema(55-68)apps/backend/src/route-handlers/smart-request.tsx (1)
SmartRequestAuth(23-30)packages/stack-shared/src/interface/crud/users.ts (1)
UsersCrud(111-111)packages/stack-shared/src/interface/crud/projects.ts (1)
ProjectsCrud(194-194)apps/backend/src/lib/tenancies.tsx (2)
Tenancy(53-53)getSoleTenancyFromProjectBranch(65-72)packages/stack-shared/src/schema-fields.ts (6)
yupArray(213-216)yupObject(247-251)yupBoolean(195-198)yupString(187-190)yupNumber(191-194)yupValidate(120-169)packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(69-85)packages/stack-shared/src/utils/arrays.tsx (1)
typedIncludes(3-5)
apps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsx (3)
apps/backend/src/app/api/latest/internal/email-themes/route.tsx (1)
GET(46-83)apps/backend/src/app/api/latest/internal/email-themes/cud.tsx (1)
internalEmailThemesCudHandlers(58-205)packages/stack-shared/src/utils/errors.tsx (2)
StatusError(152-261)StackAssertionError(69-85)
🪛 Biome (2.1.2)
apps/backend/src/route-handlers/cud-handler.tsx
[error] 133-133: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Vercel Agent Review
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: E2E Tests (Node 22.x, Freestyle mock)
- GitHub Check: restart-dev-and-test
- GitHub Check: build (22.x)
- GitHub Check: E2E Tests (Node 22.x, Freestyle prod)
- GitHub Check: restart-dev-and-test-with-custom-base-port
- GitHub Check: all-good
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: lint_and_build (latest)
- GitHub Check: docker
- GitHub Check: setup-tests-with-custom-base-port
🔇 Additional comments (7)
apps/backend/src/route-handlers/cud-handler.tsx (2)
364-369: Error wrapping logic may suppress useful error types.The catch block wraps errors in
CudHandlerInvocationErrorunless they matchallowedErrorTypesor areStackAssertionError. This could hide important error context from callers who expect specific error types not in their allowed list.Consider whether other common error types (e.g.,
KnownErrors.*) should also propagate unwrapped by default, or document this behavior clearly for consumers.
92-103: Good implementation of a centralized CUD handler factory.The
createCudHandlersfunction provides a well-structured abstraction for generating CRUD handlers with consistent validation, access control, and response formatting. The type parameterization enables strong typing for consumers.apps/backend/src/app/api/latest/internal/email-themes/[id]/route.tsx (2)
6-6: Good simplification of GET handler.Directly exporting
internalEmailThemesCudHandlers.readHandleris cleaner than wrapping it in a custom handler. This addresses the previous review feedback.
31-44: Post-update verification is defensive but could indicate a design gap.The pattern of calling
adminUpdatethen searching for the updated item in the result suggests the CUD handler's update response format may not be ideal for single-item updates. Consider whether the CUD handler could return the single updated item directly for single-item operations.That said, the current implementation is safe and handles the edge case appropriately.
apps/backend/src/app/api/latest/internal/email-themes/cud.tsx (3)
74-88:onCreateimplementation looks correct.Generates UUID if not provided, persists the new theme with default
LightEmailThemesource, and returns the ID. The use ofoverrideEnvironmentConfigOverridewith dot-notation path is appropriate.
139-149: Good validation pattern: render templates before persisting.Validating each TSX source by calling
renderEmailWithTemplatebefore persisting changes prevents invalid templates from being saved. This is a solid defensive approach.
21-35: Helper functions are clean and focused.
listThemesandgetThemeOrThrowprovide clear abstractions for reading theme data from tenancy config. The 404 error for missing themes is appropriate.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary by CodeRabbit
Refactor
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.