Mariano/stuff#45
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces a new GitHub Actions workflow to automate database migrations and Prisma Client generation when specific schema or migration files are modified. Additionally, it adds a new server-side action for publishing organization policies, complete with validation and error handling. On the frontend, new and updated React components integrate the policy publishing functionality, including UI elements like publish buttons and notifications. Minor layout adjustments and cleanup of unused imports have also been applied. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions
participant Repo as Repository
participant DB as Database
Dev->>GH: Push commit with schema/migration changes
GH->>Repo: Checkout repository
GH->>GH: Setup Node.js 20.x and install dependencies
GH->>DB: Run "npx prisma migrate deploy"
GH->>Repo: Run "npx prisma generate"
GH-->>Dev: Complete workflow execution
sequenceDiagram
participant User as End User
participant UI as PolicyOverview Component
participant Server as publishPolicy Action
participant DB as Database
User->>UI: Click "Publish" button
UI->>Server: Execute publishPolicy({ id })
Server->>DB: Update policy status and timestamp
DB-->>Server: Return updated policy
Server-->>UI: Return success response
UI->>User: Display success toast notification
Poem
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/Components/PolicyOverview.tsx (1)
12-16: Consider enhancing error handling and type safety.While the null checks are good, consider these improvements:
- Add error state handling for failed policy fetches
- Use type guards instead of type assertion for safer type conversion
- Add loading state handling
Here's a suggested implementation:
- if (!policy) return null; - const content = policy.content as JSONContent; - if (!content) return null; + if (!policy) { + return <div>Loading policy...</div>; + } + + function isJSONContent(content: unknown): content is JSONContent { + return content !== null && typeof content === 'object'; + } + + if (!policy.content || !isJSONContent(policy.content)) { + return <div>Invalid policy content</div>; + } + + const content = policy.content;apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/Actions/publish-policy.ts (1)
7-15: Add validation for organizationId in the schema.The schema only validates the
idparameter, but the action also requires a validorganizationId. Consider adding validation fororganizationIdto catch invalid requests earlier.const schema = z.object({ id: z.string(), + organizationId: z.string(), });apps/app/src/components/policies/policy-overview.tsx (1)
32-39: Enhance button state handling.The button should be disabled during execution and could benefit from a loading spinner for better visual feedback.
<Button variant="secondary" className="w-fit" + disabled={isExecuting} onClick={() => execute({ id: policyId })} > - {isExecuting ? "Publishing..." : "Publish"} + {isExecuting ? ( + <> + <span className="animate-spin mr-2">⟳</span> + Publishing... + </> + ) : ( + "Publish" + )} </Button>.github/workflows/database-migrations.yml (2)
3-10: Add pull_request trigger for validation.Consider adding a pull_request trigger to validate migrations during PR reviews. This helps catch issues before they reach main.
on: push: branches: - main paths: - "packages/db/prisma/schema.prisma" - "packages/db/prisma/migrations/**" workflow_dispatch: # Allows manual triggering + pull_request: + paths: + - "packages/db/prisma/schema.prisma" + - "packages/db/prisma/migrations/**"
12-13: Use .nvmrc or package.json engines for Node.js version.Instead of hardcoding the Node.js version, consider reading it from .nvmrc or package.json engines field for better maintainability.
-env: - NODE_VERSION: "20.x" +env: + NODE_VERSION: ${{ steps.nvm.outputs.NVMRC }} jobs: migrate: name: Run Database Migrations runs-on: ubuntu-latest steps: + - name: Read .nvmrc + id: nvm + run: echo "NVMRC=$(cat .nvmrc)" >> $GITHUB_OUTPUT
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/database-migrations.yml(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/Actions/publish-policy.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/Components/PolicyOverview.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/layout.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/page.tsx(0 hunks)apps/app/src/components/policies/policy-overview.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/page.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/layout.tsx
🔇 Additional comments (4)
apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/Components/PolicyOverview.tsx (3)
1-8: LGTM! Imports are well-organized.All necessary dependencies are properly imported, and the client directive is correctly placed.
9-10: LGTM! Component structure follows best practices.The component is well-structured with proper TypeScript typing for the props.
18-29:⚠️ Potential issueImplement the missing publishing functionality.
The AI summary mentions publishing functionality using
useActionand toast notifications, but these features are not implemented in the code. The Publish button is currently non-functional.Add the missing functionality:
+ import { useAction } from "next-safe-action/hooks"; + import { publishPolicy } from "@/app/[locale]/(app)/(dashboard)/policies/[id]/Actions/publish-policy"; + import { toast } from "sonner"; export function PolicyOverview({ policyId }: { policyId: string }) { const { data: policy } = usePolicy({ policyId }); + const { execute, isExecuting } = useAction(publishPolicy, { + onSuccess: () => { + toast.success("Policy published successfully"); + }, + }); // ... existing code ... <Button variant="secondary" className="w-fit" + onClick={() => execute({ id: policyId })} + disabled={isExecuting} > - Publish + {isExecuting ? "Publishing..." : "Publish"} </Button>Likely an incorrect or invalid review comment.
apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/Actions/publish-policy.ts (1)
17-25: LGTM! Good tracking setup.The action metadata is well-structured with appropriate tracking information.
| try { | ||
| const policy = await db.organizationPolicy.update({ | ||
| where: { | ||
| id, | ||
| organizationId: user.organizationId!, | ||
| }, | ||
| data: { | ||
| status: "published", | ||
| updatedAt: new Date(), | ||
| }, | ||
| }); | ||
|
|
||
| return { | ||
| success: true, | ||
| data: policy, | ||
| }; | ||
| } catch (error) { | ||
| return { | ||
| success: false, | ||
| error: "Failed to publish policy", | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and add policy existence check.
The current implementation has two areas for improvement:
- No validation that the policy exists before attempting to update
- Generic error message that doesn't help with debugging
try {
+ const existingPolicy = await db.organizationPolicy.findUnique({
+ where: {
+ id,
+ organizationId: user.organizationId!,
+ },
+ });
+
+ if (!existingPolicy) {
+ return {
+ success: false,
+ error: "Policy not found",
+ };
+ }
+
const policy = await db.organizationPolicy.update({
where: {
id,
organizationId: user.organizationId!,
},
data: {
status: "published",
updatedAt: new Date(),
},
});
return {
success: true,
data: policy,
};
} catch (error) {
+ console.error('Failed to publish policy:', error);
return {
success: false,
- error: "Failed to publish policy",
+ error: `Failed to publish policy: ${error instanceof Error ? error.message : 'Unknown error'}`,
};
}📝 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 policy = await db.organizationPolicy.update({ | |
| where: { | |
| id, | |
| organizationId: user.organizationId!, | |
| }, | |
| data: { | |
| status: "published", | |
| updatedAt: new Date(), | |
| }, | |
| }); | |
| return { | |
| success: true, | |
| data: policy, | |
| }; | |
| } catch (error) { | |
| return { | |
| success: false, | |
| error: "Failed to publish policy", | |
| }; | |
| } | |
| try { | |
| const existingPolicy = await db.organizationPolicy.findUnique({ | |
| where: { | |
| id, | |
| organizationId: user.organizationId!, | |
| }, | |
| }); | |
| if (!existingPolicy) { | |
| return { | |
| success: false, | |
| error: "Policy not found", | |
| }; | |
| } | |
| const policy = await db.organizationPolicy.update({ | |
| where: { | |
| id, | |
| organizationId: user.organizationId!, | |
| }, | |
| data: { | |
| status: "published", | |
| updatedAt: new Date(), | |
| }, | |
| }); | |
| return { | |
| success: true, | |
| data: policy, | |
| }; | |
| } catch (error) { | |
| console.error('Failed to publish policy:', error); | |
| return { | |
| success: false, | |
| error: `Failed to publish policy: ${error instanceof Error ? error.message : 'Unknown error'}`, | |
| }; | |
| } |
| if (!user.organizationId) { | ||
| return { | ||
| success: false, | ||
| error: "Not authorized - no organization found", | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add permission check for policy publishing.
The current authorization only checks if the user has an organization. Consider adding a permission check to verify if the user has the right to publish policies.
if (!user.organizationId) {
return {
success: false,
error: "Not authorized - no organization found",
};
}
+
+ if (!user.permissions?.includes('PUBLISH_POLICY')) {
+ return {
+ success: false,
+ error: "Not authorized - insufficient permissions",
+ };
+ }📝 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.
| if (!user.organizationId) { | |
| return { | |
| success: false, | |
| error: "Not authorized - no organization found", | |
| }; | |
| } | |
| if (!user.organizationId) { | |
| return { | |
| success: false, | |
| error: "Not authorized - no organization found", | |
| }; | |
| } | |
| if (!user.permissions?.includes('PUBLISH_POLICY')) { | |
| return { | |
| success: false, | |
| error: "Not authorized - insufficient permissions", | |
| }; | |
| } |
| const { execute, isExecuting } = useAction( | ||
| () => publishPolicy({ id: policyId }), | ||
| { | ||
| onSuccess: () => { | ||
| toast.success("Policy published successfully"); | ||
| }, | ||
| } | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and loading feedback.
The current implementation only shows success feedback. Consider adding error handling and loading state feedback for better user experience.
const { execute, isExecuting } = useAction(
() => publishPolicy({ id: policyId }),
{
onSuccess: () => {
toast.success("Policy published successfully");
},
+ onError: (error) => {
+ toast.error(error.message || "Failed to publish policy");
+ },
+ onExecute: () => {
+ toast.loading("Publishing policy...");
+ },
}
);📝 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.
| const { execute, isExecuting } = useAction( | |
| () => publishPolicy({ id: policyId }), | |
| { | |
| onSuccess: () => { | |
| toast.success("Policy published successfully"); | |
| }, | |
| } | |
| ); | |
| const { execute, isExecuting } = useAction( | |
| () => publishPolicy({ id: policyId }), | |
| { | |
| onSuccess: () => { | |
| toast.success("Policy published successfully"); | |
| }, | |
| onError: (error) => { | |
| toast.error(error.message || "Failed to publish policy"); | |
| }, | |
| onExecute: () => { | |
| toast.loading("Publishing policy..."); | |
| }, | |
| } | |
| ); |
| - name: Apply database migrations | ||
| env: | ||
| DATABASE_URL: ${{ secrets.DATABASE_URL_PROD }} | ||
| run: | | ||
| cd packages/db | ||
| npx prisma migrate deploy | ||
|
|
||
| - name: Generate Prisma Client | ||
| run: | | ||
| cd packages/db | ||
| npx prisma generate |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add safety measures for database migrations.
Consider adding these safety measures:
- Verify migrations can be applied using
prisma migrate resetin a test environment - Create a database backup before applying migrations
- Add a rollback plan in case of failures
- name: Apply database migrations
env:
DATABASE_URL: ${{ secrets.DATABASE_URL_PROD }}
run: |
cd packages/db
+ # Create backup
+ npx prisma db pull --schema backup_$(date +%Y%m%d_%H%M%S).prisma
+
+ # Verify migrations
+ npx prisma migrate reset --force --skip-seed --preview-feature
+
+ # Apply migrations
npx prisma migrate deploy
+
+ # Verify database state
+ npx prisma db pull --schema after_migration.prisma
+ if ! diff backup_*.prisma after_migration.prisma > migration_changes.diff; then
+ echo "Migration changes:"
+ cat migration_changes.diff
+ fi📝 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.
| - name: Apply database migrations | |
| env: | |
| DATABASE_URL: ${{ secrets.DATABASE_URL_PROD }} | |
| run: | | |
| cd packages/db | |
| npx prisma migrate deploy | |
| - name: Generate Prisma Client | |
| run: | | |
| cd packages/db | |
| npx prisma generate | |
| - name: Apply database migrations | |
| env: | |
| DATABASE_URL: ${{ secrets.DATABASE_URL_PROD }} | |
| run: | | |
| cd packages/db | |
| # Create backup | |
| npx prisma db pull --schema backup_$(date +%Y%m%d_%H%M%S).prisma | |
| # Verify migrations | |
| npx prisma migrate reset --force --skip-seed --preview-feature | |
| # Apply migrations | |
| npx prisma migrate deploy | |
| # Verify database state | |
| npx prisma db pull --schema after_migration.prisma | |
| if ! diff backup_*.prisma after_migration.prisma > migration_changes.diff; then | |
| echo "Migration changes:" | |
| cat migration_changes.diff | |
| fi | |
| - name: Generate Prisma Client | |
| run: | | |
| cd packages/db | |
| npx prisma generate |
1 participant
Summary by CodeRabbit
Chores
New Features
Style