Add more apis - #102
Add more apis#102
Conversation
…tion - Merge `/api/v1/employees/add` route into main `/api/v1/employees` route - Add comprehensive error handling and validation for employee creation - Update API documentation to reflect new endpoint structure - Improve response formatting with consistent success/error patterns - Add detailed validation error responses in documentation
- Implement GET /api/v1/employees/:id route to fetch individual employee details - Add detailed API documentation for the new endpoint in MDX - Include robust error handling and consistent response formatting - Update API key generation prefix from 'bubba_' to 'comp_' - Provide comprehensive example requests and responses in documentation
- Modify GET and DELETE routes to correctly handle Promise-based params - Resolve potential type and async handling issues in employee endpoints - Ensure consistent parameter resolution for employee ID retrieval
- Update revalidation settings to enable focus and reconnect revalidation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes update employee data fetching and management across several modules. In the frontend hook, SWR options now trigger revalidation on focus and reconnect. The API endpoints have been modified: new GET and DELETE methods have been added, the old POST endpoint removed, and a new POST endpoint with structured validation introduced. Additionally, the API key prefix was updated, and documentation expanded to reflect these endpoint changes. A new runtime configuration for Node.js has been established across multiple files. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Auth
participant DB
Client->>API: POST /api/v1/employees
API->>Auth: Verify API key & retrieve OrgID
Auth-->>API: OrgID confirmed
API->>API: Validate request body (employeeCreateSchema)
API->>DB: Create employee record
DB-->>API: Employee created
API-->>Client: Return success response with employee details
sequenceDiagram
participant Client
participant API
participant Auth
participant DB
Client->>API: GET /api/v1/employees/:id
API->>Auth: Verify API key & retrieve OrgID
Auth-->>API: OrgID confirmed
API->>DB: Retrieve employee record
DB-->>API: Return employee data or error
API-->>Client: Send employee data or error response
Client->>API: DELETE /api/v1/employees/:id
API->>Auth: Verify API key & retrieve OrgID
Auth-->>API: OrgID confirmed
API->>DB: Delete employee record
DB-->>API: Confirmation or error
API-->>Client: Return deletion result
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
apps/app/src/app/[locale]/(app)/(dashboard)/people/hooks/useEmployees.ts (1)
55-57: Revalidate on focus and reconnect.
EnablingrevalidateOnFocusandrevalidateOnReconnectensures fresher data whenever the application regains focus or reconnects. This can cause extra network requests, but is appropriate if the latest data is a priority.apps/app/src/app/api/v1/employees/[id]/route.ts (2)
22-88: Robust error handling for GET.
Fetching the organization ID, checking for employee existence, and returning 404 or 500 status codes are well-handled. Consider validating theidparameter to confirm it’s a valid format (e.g., UUID) before querying.
107-165: Safe deletion logic.
The code properly checks ownership viaorganizationIdand returns appropriate 404 if the employee is not found. Consider a soft-delete or an audit trail depending on compliance requirements.apps/app/src/app/api/v1/employees/route.ts (1)
211-224: Consider adding a check for duplicate employee emailsThe current implementation doesn't check if an employee with the same email already exists in the organization before creating a new one. This could lead to duplicate records.
// Add after line 210, before creating the employee + // Check if employee with this email already exists + const existingEmployee = await db.employee.findFirst({ + where: { + email: validatedData.email, + organizationId: organizationId!, + }, + }); + + if (existingEmployee) { + return NextResponse.json( + { + success: false, + error: "Employee with this email already exists", + }, + { status: 400 } + ); + }packages/docs/api-reference/v1/employees.mdx (1)
199-200: Consider securing API keys in curl examplesThe static analysis tool flagged these lines for potentially exposing API keys in curl examples. While these are placeholders, it's a good security practice to emphasize that these are examples only.
Consider adding a note near the examples mentioning:
+> **Note**: Replace `YOUR_API_KEY` with your actual API key. Never share or expose your API keys in client-side code or public repositories.Also applies to: 329-330, 416-417
🧰 Tools
🪛 Gitleaks (8.21.2)
199-200:
(curl-auth-header)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
apps/app/src/app/[locale]/(app)/(dashboard)/people/hooks/useEmployees.ts(3 hunks)apps/app/src/app/api/v1/employees/[id]/route.ts(1 hunks)apps/app/src/app/api/v1/employees/add/route.ts(0 hunks)apps/app/src/app/api/v1/employees/route.ts(2 hunks)apps/app/src/lib/api-key.ts(1 hunks)packages/docs/api-reference/v1/employees.mdx(8 hunks)
💤 Files with no reviewable changes (1)
- apps/app/src/app/api/v1/employees/add/route.ts
🧰 Additional context used
🪛 Gitleaks (8.21.2)
packages/docs/api-reference/v1/employees.mdx
199-200:
(curl-auth-header)
329-330:
(curl-auth-header)
416-417:
(curl-auth-header)
🔇 Additional comments (19)
apps/app/src/lib/api-key.ts (1)
14-14:Details
❓ Verification inconclusive
Confirm references to the old prefix.
This change updates the prefix frombubba_tocomp_. Ensure that any dependent code, documentation, or database entries referencing the old prefix is also updated, if required for backward compatibility.
🏁 Script executed:
#!/bin/bash # Check for references to `bubba_` across the repository rg -A 3 'bubba_'Length of output: 16
Action Required: Verify External Dependencies on 'bubba_'
The updated prefix in
apps/app/src/lib/api-key.ts(line 14) is now correctly set tocomp_. Our repository-wide search usingrgdid not reveal any occurrences of the oldbubba_prefix in the code. However, please ensure that any external dependencies—such as documentation, test data, configuration files, or database entries—that reference the old prefix have been reviewed and updated for backward compatibility.
- Verify that associated documentation and configuration files no longer refer to
bubba_.- Confirm that any database initialization or legacy data migration processes have been updated accordingly.
apps/app/src/app/[locale]/(app)/(dashboard)/people/hooks/useEmployees.ts (2)
15-15: Good use of typed function parameters.
The use ofEmployeesInputin the function signature clarifies usage and improves maintainability.
104-104: Check dependency array completeness.
Make sure thataddEmployeedoes not rely on other variables or states outside ofrevalidateEmployees. If additional dependencies are used inside the callback, they should be included in the dependency array to avoid stale closures.apps/app/src/app/api/v1/employees/[id]/route.ts (3)
1-4: Imports look appropriate.
All required modules (database, Next.js server utilities, API key utilities) are correctly imported.
5-21: Well-documented endpoint.
The JSDoc for the GET route accurately describes the path, headers, and expected responses. Keeping this documentation up-to-date ensures consumer clarity.
90-106: Clear DELETE endpoint documentation.
The doc block for the DELETE route clearly outlines requirements and responses, making it easy for API consumers to understand usage.apps/app/src/app/api/v1/employees/route.ts (5)
16-25: Schema validation implementation looks goodThe employee creation schema is well-structured with appropriate validations:
- Required fields have minimum length validations
- Email validation ensures proper format
- Good use of optional fields with sensible defaults
- Proper nullable fields where appropriate
30-31: Good type derivation from Zod schemaUsing Zod's type inference to derive the
EmployeeCreateInputtype from the schema is a good practice that ensures type safety and consistency between validation and usage.
157-179: Well-documented API endpointThe JSDoc comments for the POST endpoint are comprehensive and clear, detailing:
- Endpoint path and purpose
- Required headers
- Expected request body fields with descriptions
- All possible response status codes and formats
This level of documentation is excellent for maintainability.
180-210: API authentication and validation flow looks goodThe implementation properly:
- Extracts and validates the API key to get organization ID
- Returns appropriate error responses for invalid authentication
- Parses and validates the request body against the schema
- Returns detailed validation errors when validation fails
225-240: Consistent error response formatThe error response format is consistent with the rest of the API, including the
success: falseflag and appropriate HTTP status codes. The console error logging is also helpful for debugging.packages/docs/api-reference/v1/employees.mdx (8)
2-3: Consistent string format in frontmatterThe update from single quotes to double quotes in the frontmatter maintains consistency with other documentation files.
8-8: Enhanced clarity in endpoint descriptionThe updated description clearly communicates the expanded functionality of the API, highlighting that users can now create and delete employees.
31-37: Comprehensive endpoint overviewThe added list of available endpoints provides a clear overview of all functionality, making it easier for developers to understand what operations are supported.
96-109: Well-structured error response documentationThe error response documentation clearly explains the possible error scenarios and their response format, which is crucial for API consumers to handle errors properly.
137-229: Complete documentation for single employee retrievalThe GET endpoint for retrieving a single employee is thoroughly documented with:
- Clear endpoint description
- Required parameters
- Response structure
- Error scenarios
- Example request and response
This level of detail is excellent for API consumers.
🧰 Tools
🪛 Gitleaks (8.21.2)
199-200:
(curl-auth-header)
232-232: Updated API path for employee creationThe documentation now correctly shows the updated path for creating employees, which aligns with the implementation in the route.ts file.
362-377: Helpful validation error exampleThe added example of a validation error response helps API consumers understand what to expect when their request doesn't meet the schema requirements.
379-452: Complete DELETE endpoint documentationThe DELETE endpoint documentation is comprehensive and follows the same structure as other endpoints, ensuring consistency throughout the documentation.
🧰 Tools
🪛 Gitleaks (8.21.2)
416-417:
(curl-auth-header)
| "success": false, | ||
| "error": "Validation failed", | ||
| "details": { | ||
| "email": { | ||
| "_errors": ["Valid email is required"] | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Review DELETE validation error example
The validation error example for the DELETE endpoint seems odd since DELETE operations typically don't require an email field. This example might be more appropriate for the POST endpoint.
🌐 Web query:
Do REST DELETE endpoints typically include request bodies that could be validated?
Error during web search
Fix DELETE Endpoint Validation Example
The current validation error example in the DELETE endpoint is confusing since DELETE requests typically do not include a body, and thus validating fields like "email" is unexpected. Please consider either removing the email validation from this example or moving it to a POST endpoint example where such validation is more appropriate.
- File:
packages/docs/api-reference/v1/employees.mdx(lines 444–451)
- Set runtime to 'nodejs' for `/api/v1/employees` and `/api/v1/employees/[id]` routes - Ensure compatibility with database operations and server-side logic - Improve route performance and functionality by using Node.js runtime
- Delete redundant validation error response example - Simplify documentation by removing unnecessary JSON snippet - Maintain clean and concise API reference documentation
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
apps/app/src/app/api/v1/employees/[id]/route.ts (1)
127-142: Consider optimizing database operations.Currently, you're doing two database operations: first to check if the employee exists, then to delete it. You could potentially optimize this with a single operation by using Prisma's delete with where clause and handling the case where no records are deleted.
- // Check if the employee exists and belongs to the organization - const existingEmployee = await db.employee.findFirst({ - where: { - id: employeeId, - organizationId: organizationId!, - }, - }); - - if (!existingEmployee) { - return NextResponse.json( - { - success: false, - error: "Employee not found", - }, - { status: 404 } - ); - } - - // Delete the employee - await db.employee.delete({ - where: { - id: employeeId, - }, - }); + // Try to delete the employee that belongs to the organization + const deleteResult = await db.employee.deleteMany({ + where: { + id: employeeId, + organizationId: organizationId!, + }, + }); + + // If no records were deleted, the employee wasn't found + if (deleteResult.count === 0) { + return NextResponse.json( + { + success: false, + error: "Employee not found", + }, + { status: 404 } + ); + }apps/app/src/app/api/v1/employees/route.ts (2)
93-94: Avoid usinganytype.Using
anytype for thewhereclause reduces type safety. Consider defining a more specific type for the query conditions.- // Build the where clause - const where: any = { + // Build the where clause + // Define a type that includes all possible filter properties + type EmployeeWhereClause = { + organizationId: string; + isActive?: boolean; + department?: string; + OR?: Array<{ + name?: { contains: string; mode: string }; + email?: { contains: string; mode: string }; + }>; + }; + + const where: EmployeeWhereClause = { organizationId: organizationId!, };
229-232: Consider adding resource location in response headers.For the POST endpoint, it's a REST best practice to include the location of the newly created resource in the response headers.
+ // Create the response with success status and formatted employee data + const response = NextResponse.json({ + success: true, + data: formattedEmployee, + }); + + // Add the location header pointing to the new resource + response.headers.set('Location', `/api/v1/employees/${employee.id}`); + + return response; - return NextResponse.json({ - success: true, - data: formattedEmployee, - });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/app/src/app/api/v1/employees/[id]/route.ts(1 hunks)apps/app/src/app/api/v1/employees/route.ts(3 hunks)
🔇 Additional comments (11)
apps/app/src/app/api/v1/employees/[id]/route.ts (5)
5-6: Good practice to specify the runtime.Setting the runtime to Node.js is appropriate for database operations.
8-24: Well-documented API endpoint.The documentation is thorough, covering all inputs, possible responses, and status codes, which is great for API consumers.
42-57: Proper database filtering using both employee ID and organization ID.This is a good security practice - ensuring the employee belongs to the organization associated with the API key.
71-75: Proper date formatting for JSON response.Converting Date objects to ISO strings is necessary for proper JSON serialization.
93-109: Well-documented DELETE endpoint.The documentation clearly explains the endpoint's purpose, required headers, path parameters, and possible responses.
apps/app/src/app/api/v1/employees/route.ts (6)
6-7: Appropriate runtime specification.Setting the runtime to Node.js is correct for database operations.
19-28: Well-defined validation schema.The schema provides clear validation rules with descriptive error messages, which is excellent for API usability.
33-34: Good type definition for enhanced type safety.Inferring the type from the schema ensures type consistency between validation and usage.
160-182: Comprehensive API documentation.The documentation clearly details all request parameters, body fields, and possible responses.
193-213: Robust validation with detailed error reporting.The implementation properly validates input data and returns structured validation errors, which is great for API consumers.
223-227: Consistent date formatting.The date formatting is consistent with other endpoints, which ensures a uniform API response structure.
| export async function DELETE( | ||
| request: NextRequest, | ||
| { params }: { params: Promise<{ id: string }> } | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix the params type in DELETE method.
Similar to the GET method, the params parameter is incorrectly typed as a Promise.
export async function DELETE(
request: NextRequest,
- { params }: { params: Promise<{ id: string }> }
+ { params }: { params: { id: string } }
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function DELETE( | |
| request: NextRequest, | |
| { params }: { params: Promise<{ id: string }> } | |
| ) { | |
| export async function DELETE( | |
| request: NextRequest, | |
| { params }: { params: { id: string } } | |
| ) { |
| try { | ||
| const employeeId = (await params).id; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary await for params.
Since params should not be a Promise (as noted in the previous comment), you don't need to await it.
- const employeeId = (await params).id;
+ const employeeId = params.id;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const employeeId = (await params).id; | |
| try { | |
| - const employeeId = (await params).id; | |
| + const employeeId = params.id; |
| // Delete the employee | ||
| await db.employee.delete({ | ||
| where: { | ||
| id: employeeId, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Security issue: Missing organization ID in deletion criteria.
The deletion query only filters by employee ID, not by organization ID. This could potentially allow deletion of employees from other organizations if the ID is known.
- // Delete the employee
- await db.employee.delete({
- where: {
- id: employeeId,
- },
- });
+ // Delete the employee, ensuring it belongs to the correct organization
+ await db.employee.delete({
+ where: {
+ id: employeeId,
+ organizationId: organizationId!,
+ },
+ });Note: If you implement the optimization from the previous comment, this issue would be resolved automatically.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Delete the employee | |
| await db.employee.delete({ | |
| where: { | |
| id: employeeId, | |
| }, | |
| }); | |
| // Delete the employee, ensuring it belongs to the correct organization | |
| await db.employee.delete({ | |
| where: { | |
| id: employeeId, | |
| organizationId: organizationId!, | |
| }, | |
| }); |
| export async function GET( | ||
| request: NextRequest, | ||
| { params }: { params: Promise<{ id: string }> } | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix the params type - it shouldn't be a Promise.
The type for params is defined as Promise<{ id: string }>, which is unusual. Typically, route parameters in Next.js are not promises.
export async function GET(
request: NextRequest,
- { params }: { params: Promise<{ id: string }> }
+ { params }: { params: { id: string } }
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function GET( | |
| request: NextRequest, | |
| { params }: { params: Promise<{ id: string }> } | |
| ) { | |
| export async function GET( | |
| request: NextRequest, | |
| { params }: { params: { id: string } } | |
| ) { |
| try { | ||
| const employeeId = (await params).id; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary await for params in DELETE method.
Since params should not be a Promise, you don't need to await it here either.
- const employeeId = (await params).id;
+ const employeeId = params.id;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const employeeId = (await params).id; | |
| try { | |
| - const employeeId = (await params).id; | |
| + const employeeId = params.id; |
| // Create the employee using the organization ID from the API key | ||
| const employee = await db.employee.create({ | ||
| data: { | ||
| ...validatedData, | ||
| organizationId: organizationId!, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add check for duplicate employees.
There's no check for whether an employee with the same email already exists in the organization. This could lead to duplicate employee records.
+ // Check if an employee with the same email already exists in this organization
+ const existingEmployee = await db.employee.findFirst({
+ where: {
+ email: validatedData.email,
+ organizationId: organizationId!,
+ },
+ });
+
+ if (existingEmployee) {
+ return NextResponse.json(
+ {
+ success: false,
+ error: "An employee with this email already exists",
+ },
+ { status: 400 }
+ );
+ }
// Create the employee using the organization ID from the API key
const employee = await db.employee.create({
data: {
...validatedData,
organizationId: organizationId!,
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Create the employee using the organization ID from the API key | |
| const employee = await db.employee.create({ | |
| data: { | |
| ...validatedData, | |
| organizationId: organizationId!, | |
| }, | |
| }); | |
| // Check if an employee with the same email already exists in this organization | |
| const existingEmployee = await db.employee.findFirst({ | |
| where: { | |
| email: validatedData.email, | |
| organizationId: organizationId!, | |
| }, | |
| }); | |
| if (existingEmployee) { | |
| return NextResponse.json( | |
| { | |
| success: false, | |
| error: "An employee with this email already exists", | |
| }, | |
| { status: 400 } | |
| ); | |
| } | |
| // Create the employee using the organization ID from the API key | |
| const employee = await db.employee.create({ | |
| data: { | |
| ...validatedData, | |
| organizationId: organizationId!, | |
| }, | |
| }); |
- Add `runtime = "nodejs"` export to auth configuration files - Ensure consistent runtime environment for authentication-related modules - Improve compatibility with server-side authentication logic
- Bump nanoid to version 5.1.0 in multiple packages - Update @tiptap/extension-bold to version 2.11.5 - Add @types/react version 19.0.10 to project dependencies
…ware - Add `runtime = "nodejs"` to middleware, API routes, and server actions - Ensure consistent runtime environment for server-side modules - Update runtime configuration in multiple files to improve compatibility
- Add `experimental.nodeMiddleware: true` to Next.js configuration - Enhance server-side middleware compatibility - Prepare for advanced Node.js runtime features in the application
…timizations - Add ANTHROPIC_API_KEY to global environment variables - Simplify Turbo configuration JSON structure - Remove unnecessary whitespace and formatting
- Remove `experimental.nodeMiddleware` from Next.js configuration - Remove explicit `runtime = "nodejs"` from middleware - Simplify runtime configuration and remove unnecessary experimental settings
- Install `ai` package version 3.4.33 - Update package.json and lock files with new dependency - Standardize dependency version specifications
- Remove unnecessary task configurations in turbo.json - Simplify build and development task dependencies - Remove cached tasks for clean, database operations, and integrations - Optimize global configuration for more efficient build process
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/ui/package.json (1)
123-123: Exact Version Update for @tiptap/extension-boldLocking
@tiptap/extension-boldto version2.11.5ensures that the UI components have a stable and predictable behavior. If you intentionally want to avoid automatic patch updates within the major version, this change is appropriate. Otherwise, consider whether a semver range might be more flexible for future improvements.apps/app/src/app/api/generate/route.ts (1)
131-139: Consider using a configurable model selectionThe model is hardcoded to "gpt-4o-mini" which could limit flexibility as OpenAI releases new models or updates existing ones.
Consider using environment variables or configuration settings to determine which model to use:
- model: openai("gpt-4o-mini"), + model: openai(env.OPENAI_MODEL || "gpt-4o-mini"),This allows for easier model swapping without code changes, especially useful for testing performance, costs, or capabilities of different models.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
bun.lockbis excluded by!**/bun.lockbyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
apps/app/src/actions/runtime-config.ts(1 hunks)apps/app/src/app/api/auth/[...nextauth]/route.ts(1 hunks)apps/app/src/app/api/chat/route.ts(1 hunks)apps/app/src/app/api/generate/route.ts(2 hunks)apps/app/src/app/api/v1/route-config.ts(1 hunks)apps/app/src/auth/config.ts(1 hunks)apps/app/src/auth/index.ts(1 hunks)apps/app/src/auth/org.ts(1 hunks)apps/app/src/auth/stripe.ts(1 hunks)apps/app/src/middleware.ts(1 hunks)package.json(1 hunks)packages/docs/api-reference/v1/employees.mdx(8 hunks)packages/kv/package.json(1 hunks)packages/notifications/package.json(1 hunks)packages/ui/package.json(1 hunks)turbo.json(2 hunks)
✅ Files skipped from review due to trivial changes (5)
- apps/app/src/auth/config.ts
- apps/app/src/actions/runtime-config.ts
- packages/kv/package.json
- apps/app/src/middleware.ts
- apps/app/src/app/api/v1/route-config.ts
🔇 Additional comments (36)
package.json (1)
33-34: New Dependency Additions for Enhanced FunctionalityThe addition of
@types/react(v19.0.10) andai(v3.4.33) looks good. These dependencies should help improve type support for React and integrate AI functionalities. Ensure that any new usage of these packages is well documented and tested across the application.packages/notifications/package.json (1)
15-15: Locking Dependency Version for nanoidUpdating the
nanoiddependency from a caret version (^5.0.7) to an exact version (5.1.0) helps ensure consistent behavior by avoiding unintentional updates. Confirm that this version lock aligns with your overall dependency management strategy.turbo.json (6)
3-3: Global Dependencies SimplificationThe
"globalDependencies"field is now defined as a single-line array containing"**/.env", which simplifies the configuration. Confirm that this pattern meets your needs for tracking all relevant environment files.
35-36: New Environment Variables AddedThe build task now includes
"AWS_SECRET_ACCESS_KEY"and"ANTHROPIC_API_KEY". Make sure these keys are securely managed in your CI/CD pipeline and that their inclusion aligns with your deployment strategy.
38-40: Build Task Configuration UpdateThe
"build"task now explicitly defines its"inputs","dependsOn", and"outputs". This streamlined configuration improves maintainability. Verify that the new dependency ("^build") and the specified outputs correctly reflect your intended build order and caching strategy.
46-47: Dev Task Dependency AdjustmentThe
"dev"task now lists"inputs": ["$TURBO_DEFAULT$", ".env"]and depends on"^db:generate". This change ensures that database generation is completed before the development server starts. Ensure that this new dependency order fits your local development workflow.
52-52: Lint Task Dependency UpdateThe
"lint"task has been updated to depend on"^topo". Please verify that this dependency correctly represents the required precondition for linting in your project.
55-56: Typecheck Task Configuration EnhancementThe
"typecheck"task now depends on"^topo"and explicitly outputs to"node_modules/.cache/tsbuildinfo.json". This update should help streamline incremental builds and caching. Confirm that this configuration aligns with your TypeScript build process.apps/app/src/auth/org.ts (1)
4-5: Good addition of runtime specification.Adding the Node.js runtime specification is appropriate for this file since it contains server-side code that interacts with the database and Stripe API.
This explicit runtime declaration ensures that the code runs in a Node.js environment rather than using Edge runtime, which is important for compatibility with the external dependencies and database transactions being used.
apps/app/src/app/api/auth/[...nextauth]/route.ts (1)
3-3: Appropriate runtime specification for NextAuth route.The explicit Node.js runtime declaration is necessary for NextAuth to function properly, as it relies on Node.js features for authentication processes.
This follows Next.js best practices to specify the runtime environment for API routes, especially those handling authentication.
apps/app/src/auth/index.ts (1)
5-6: Consistent runtime specification added.Good addition of the Node.js runtime specification, keeping consistency with other authentication-related files.
This ensures that all authentication components operate in the same runtime environment and helps prevent potential issues that could arise from mixed runtime contexts.
apps/app/src/app/api/chat/route.ts (1)
4-4: Appropriate runtime for OpenAI integration.Adding the Node.js runtime specification is essential for this API route that interacts with OpenAI's services.
The Node.js runtime provides better compatibility with the OpenAI SDK and ensures proper handling of streaming responses, which might not be fully supported in Edge runtime.
apps/app/src/auth/stripe.ts (1)
3-3:Details
❓ Verification inconclusive
Change from edge runtime to Node.js runtime
This addition specifies Node.js as the runtime environment for the Stripe-related functions. This aligns with the pattern observed across multiple files in this PR, ensuring consistent runtime configuration.
🏁 Script executed:
#!/bin/bash # Check for other files that might need runtime configuration but don't have it # This helps ensure consistency across the codebase echo "Checking for Stripe-related files that might need runtime configuration:" fd -e ts -e js --exclude node_modules "stripe" | xargs grep -l "import.*Stripe" | xargs grep -L "runtime.*=.*nodejs" || echo "All Stripe-related files have runtime configuration." echo -e "\nChecking other API route files that might need runtime configuration:" fd -e ts --exclude node_modules "route\.ts$" | xargs grep -L "runtime" || echo "All API route files have runtime configuration."Length of output: 583
Runtime configuration verified for Stripe functions—please review consistency on API routes
- The change in apps/app/src/auth/stripe.ts (adding
export const runtime = "nodejs";) correctly specifies the Node.js runtime for Stripe-related functions.- However, our checks indicate two API route files currently lack any runtime configuration:
apps/portal/src/app/api/auth/[...all]/route.tsapps/web/src/app/api/waitlist/route.ts- Please verify whether these files should explicitly configure a runtime (e.g.,
export const runtime = "nodejs";) to maintain consistency across the codebase.apps/app/src/app/api/generate/route.ts (3)
16-16: Formatting adjustmentThis is a minor formatting change that improves code consistency.
28-28:Details
✅ Verification successful
Removed trailing comma
The trailing comma after the template string
novel_ratelimit_${ip}has been removed. This is a minor formatting change.
🏁 Script executed:
#!/bin/bash # Check the project's preferred style for trailing commas to ensure consistency echo "Checking ESLint configuration for trailing comma rules:" fd -t f -e json -e js -e cjs ".eslintrc" | xargs grep -l "comma" | xargs grep -A 5 -B 5 "comma" || echo "No specific trailing comma rules found in ESLint config." echo -e "\nChecking Prettier configuration for trailing comma settings:" fd -t f -e json -e js -e cjs ".prettierrc" | xargs grep "trailingComma" || echo "No specific trailing comma rules found in Prettier config."Length of output: 669
Approved: Trailing Comma Removal Verified
After checking the ESLint and Prettier configurations, no explicit trailing comma rules were enforced. Therefore, the removal of the trailing comma from the template string
novel_ratelimit_${ip}in apps/app/src/app/api/generate/route.ts is a minor, acceptable formatting change.
- File:
apps/app/src/app/api/generate/route.ts(Line 28)- Change: Removed trailing comma after the template string.
This change is approved and aligns with the project's formatting practices.
8-8:Details
✅ Verification successful
Runtime change from edge to Node.js
You've changed the runtime from "edge" to "nodejs". This is a significant change that could affect performance characteristics and available functionality.
Edge functions generally provide lower latency and are optimized for performance but have more constraints, while Node.js runtime offers a broader API surface and potentially more flexibility.
🏁 Script executed:
#!/bin/bash # Check if there's any edge-specific code in this file that might not work with Node.js runtime echo "Checking for edge-specific functions that might not be compatible with Node.js runtime:" grep -E 'Response.json|waitUntil|NextRequest|NextFetchEvent|context.env' apps/app/src/app/api/generate/route.ts echo -e "\nChecking for dependencies that might be used differently between edge and Node.js runtimes:" grep -E 'import.*from "(vercel|next)/edge"|import \{.*\} from "next"' apps/app/src/app/api/generate/route.ts echo -e "\nChecking if this file uses cache directives that might behave differently:" grep -E 'export const revalidate|export const fetchCache|export const dynamic' apps/app/src/app/api/generate/route.tsLength of output: 893
Runtime configuration update verified
After reviewing the file at
apps/app/src/app/api/generate/route.ts, the verification scripts did not reveal any edge-specific functions, dependencies, or cache directives. This suggests that the change from'edge'to'nodejs'is isolated to the runtime setting and should be safe in this context.
- The script checks for patterns like
Response.json,waitUntil, and similar edge-specific calls returned no matches.- No edge-related imports (e.g., from
next/edgeorvercel/edge) were found.- There were no cache-related directives that might behave differently between the runtimes.
Please continue to monitor performance and functionality in production to ensure that the runtime change meets the desired criteria.
packages/docs/api-reference/v1/employees.mdx (20)
2-3: Clarify Title and Description FormattingThe title and description now use double quotes, which improves consistency and readability in the documentation.
8-8: Update Introduction for Employee EndpointsThe introductory paragraph clearly explains that the endpoints allow users to list, create, and delete employees. This update improves the clarity of what operations are available.
31-37: Review Endpoint ListingsThe endpoints list now explicitly details the four key operations (GET all, GET by ID, POST, DELETE) with concise descriptions. Ensure that these endpoints remain in sync with the actual API implementation.
47-48: Enhance Query Parameter DescriptionThe description for filtering by active status is now succinct and clear, indicating that if the parameter is not provided, all employees will be returned.
96-109: Standardized Error Response Format for List EmployeesThe error response section now uniformly documents the structure (including
success,error, anddetails), which will help API consumers understand how to handle failures.
137-148: Clear Documentation for Get Employee EndpointThe GET endpoint for retrieving a single employee by ID is well documented. The parameter details and usage instructions are clear, aiding developers in correctly invoking this endpoint.
149-184: Comprehensive Employee Object SchemaThe response section for a GET employee request thoroughly details the employee object schema within an expandable section. This structured approach makes it easy for developers to review all the available fields.
186-195: Consistent Error Response for Get EmployeeThe error responses for retrieving an employee follow the same standard as other endpoints, ensuring consistency across the API documentation.
196-200: Clear Example Request for Get EmployeeThe provided cURL example for fetching an employee is well formatted and includes the necessary Authorization header.
203-219: Accurate Example Response for Get EmployeeThe example response clearly outlines the structure of a successful employee retrieval, offering practical guidance for API consumers.
232-234: Detailed Description for Add Employee EndpointThe POST endpoint’s description now explicitly states that it is used to add a new employee. This improves clarity and aligns with the updated documentation structure.
329-339: Well-Formatted Example Request for Add EmployeeThe cURL example for creating a new employee is precise and demonstrates the correct headers and JSON payload structure. This example will help developers quickly integrate with the API.
343-360: Comprehensive Example Response for Add EmployeeThe example response for a successful employee creation includes all the relevant fields, providing a clear template for what to expect when using the endpoint.
362-377: Clear Validation Error ExampleThe validation error response example is detailed and shows how field-specific errors are returned. This structured error information aids in debugging and proper error handling on the client side.
379-390: Delete Employee Endpoint Documentation & Addressing Past Review FeedbackThe DELETE endpoint documentation is now concise; it specifies that only the employee ID is required, appropriately omitting any email validation. This resolves previous concerns related to unnecessary validation on DELETE requests.
391-402: Clear Success Response for Delete EmployeeThe DELETE response section provides a simple and clear confirmation message upon successful deletion, making it easy for API consumers to interpret the outcome.
403-412: Standardized Error Format for Delete EmployeeError responses for the DELETE endpoint are consistent with those for other endpoints, ensuring a uniform experience throughout the API.
413-418: Accurate Example Request for Delete EmployeeThe DELETE request example correctly demonstrates how to remove an employee, including proper use of the Authorization header. No issues noted here.
421-430: Clear Example Response for Delete EmployeeThe example response for a successful deletion is straightforward and confirms that the operation was executed correctly.
431-438: Concise Example Error Response for Delete EmployeeThe error response example efficiently communicates the scenario when an employee is not found, which is helpful for developers handling error cases.
- Update bun.lock with latest package versions - Modify workspace dependencies from `workspace:*` to specific package paths - Bump various package versions including @NangoHQ, @mui, @opentelemetry, and uploadthing - Synchronize yarn.lock with latest dependency configurations
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
turbo.json (1)
6-8: 💡 Verification agent🧩 Analysis chain
Revisit Removal of Deprecated Tasks
According to the high-level summary, tasks such as"clean-react","db:push","clean", and"integrations:dev"were intended to be removed. These tasks still appear in the configuration (lines 6–8, 48–56), and"clean-react"is even referenced in thedependsOnof thebuildtask. Please verify whether these tasks should be removed and, if so, update both the configuration and any related dependency references.Also applies to: 48-56
Action Required: Remove Deprecated Tasks from turbo.json
After verifying the configuration, it appears that the deprecated tasks—specifically"clean-react", along with"db:push","clean", and"integrations:dev", which were slated for removal per the high-level summary—are still present. Notably,"clean-react"is referenced in thedependsOnof thebuildtask.
- Remove the
"clean-react"block (lines 6–8) and the related block on lines 48–56.- Update any dependency references (e.g., in the
dependsOnarray of thebuildtask) to remove the reference to"clean-react".Please make these changes to align the configuration with the intended removal of deprecated tasks.
🏁 Script executed:
#!/bin/bash # Display the sections of turbo.json that mention the deprecated tasks. echo "Lines 1–10 of turbo.json:" sed -n '1,10p' turbo.json echo "" echo "Lines 45–60 of turbo.json:" sed -n '45,60p' turbo.json # Additionally, search for deprecated tasks across the file to confirm references. echo "" echo "Searching for deprecated tasks ('clean-react', 'db:push', 'clean', 'integrations:dev') in turbo.json:" rg -n 'clean-react|db:push|clean|integrations:dev' turbo.jsonLength of output: 1287
Deprecated Task Removal: Action Required
Our verification confirms that the following deprecated tasks remain present in
turbo.json:
- "clean-react" is defined at lines 6–8 and is referenced in the
"dependsOn"array of thebuildtask (line 42).- "db:push," "clean," and "integrations:dev" are defined between lines 48–56.
To align with the intended removal as outlined in the high-level summary, please remove these task definitions from the configuration and update any dependency references accordingly.
🧹 Nitpick comments (1)
turbo.json (1)
38-39: Update Build Task Environment Variables
Thebuildtask now includes the sensitive environment variableANTHROPIC_API_KEYalongsideAWS_SECRET_ACCESS_KEY. Please ensure these keys are managed securely (e.g., via secret management or environment-specific configurations) to avoid inadvertent exposure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
bun.lockis excluded by!**/*.lockbun.lockbis excluded by!**/bun.lockbyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
turbo.json(3 hunks)
🔇 Additional comments (5)
turbo.json (5)
3-3: Consolidate Global Dependencies Format
TheglobalDependenciesfield is now expressed in a concise, single-line array which improves readability.
57-59: Simplify Dev Task Dependencies
Thedevtask now clearly specifies its inputs (["$TURBO_DEFAULT$", ".env"]) and depends solely on"^db:generate", which streamlines dependency management.
64-64: Lint Task Dependency Update
The update to have thelinttask depend on"^topo"improves the clarity of task sequencing.
67-68: Update Typecheck Task Configuration
Thetypechecktask now correctly depends on"^topo"and outputs the TypeScript build information tonode_modules/.cache/tsbuildinfo.json, which should help optimize caching and incremental builds.
41-43:Details
❓ Verification inconclusive
Refactor Build Task Configuration Fields
Reformatting theinputs,dependsOn, andoutputsfields into a single-line format enhances clarity. However, note that"clean-react"is still referenced in thedependsOnarray—even though the change summary indicates that this task should be removed. Please confirm whether this dependency should be updated.
Action Required: Confirm 'clean-react' Dependency Removal
The configuration inturbo.json(lines 41–43) has been reformatted into a single-line style for clarity. However, we still see"clean-react"listed in thedependsOnarray even though the change summary indicates that this task should be removed. Please run the attached shell script to search for any remaining references to"clean-react"in the codebase. If no other usage is found, update the configuration by removing this dependency.
🏁 Script executed:
#!/bin/bash # Verify if "clean-react" is referenced in the repository to determine if it should be removed. echo "Searching for references to \"clean-react\" in the repository..." rg --ignore-case '"clean-react"' -n .Length of output: 620
Action Required: Confirm the Removal or Retention of "clean-react"
After reformatting the fields inturbo.json, the"dependsOn"array still contains the"clean-react"dependency on line 42. A search in the repository shows that"clean-react"is also defined in the package scripts (e.g., inapps/app/package.jsonandapps/web/package.json) and configured inturbo.json(line 6). Since the change summary suggests that this task should be removed, please verify if its removal is intentional. If it is, update the configuration accordingly in all relevant places.
- Review in
turbo.json: Remove"clean-react"from the"dependsOn"array (line 42) and adjust its configuration if no longer needed.- Review Package Scripts: Confirm that any scripts or tasks relying on
"clean-react"inapps/app/package.jsonandapps/web/package.jsonare updated or removed as intended.
…H-49, GH-52, GH-75, GH-102, GH-272) (#3573) * fix(api): derive soa organizationId from session, not request body Every SOA handler now overwrites dto.organizationId with the trusted @organizationId() session value before calling the service, closing the cross-tenant read/tamper/destroy gap in save-answer, auto-fill, create-document, ensure-setup, approve, decline, and submit-for-approval. Refs GH-36 * fix(api): scope task automations to task and organization automationId lookups are now verified against the task in the URL and the caller's organization via a shared verifyAutomationAccess helper, so an automationId from another org's task 404s on read, update, delete, runs, versions, and publish instead of leaking or mutating it. Refs GH-46 * fix(api): harden public trust-portal access endpoints reclaimAccess no longer returns the access link/token in the response body and returns an identical generic message whether or not a grant exists, removing the unauthenticated token disclosure and the email-enumeration oracle; the link is only emailed to the requester. findPublishedTrustByRouteId no longer auto-creates a published Trust row or flips drafts to published: public endpoints only resolve rows that are already published and 404 otherwise, so an unauthenticated caller can no longer force-publish an organization's trust portal. Refs GH-42, GH-272 * fix(app): require session and org match in task-automation actions Every exported server action now resolves the caller's session and active organization and fails closed when unauthenticated. Actions that take orgId must match the session's active org; S3 keys must be prefixed with the active org; automationId-only actions verify ownership through the automation's task before proxying to the enterprise API. Refs GH-52 * fix(app): require auth and run ownership before minting trigger tokens healAndSetAccessToken and createAccessToken now require a session with an active organization and only mint a Trigger.dev run-read token when the run id is recorded against that organization (onboarding job, knowledge base document, or remediation batch). Refs GH-102 * ci(device-agent): restrict release workflow to protected branches The device-agent release pipeline runs branch-controlled build scripts with Apple and SSL.com code-signing secrets in scope, so push triggers are now limited to main and release (manual staging builds remain via workflow_dispatch), and the secret-bearing jobs are gated behind the staging/production GitHub environments. Refs GH-49, GH-75
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores