Skip to content

Mariano/stuff #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Marfuen merged 4 commits into from
Feb 12, 2025
Merged

Mariano/stuff #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b85a43a
allow publish
Marfuen Feb 12, 2025
50087ed
add db migrations to prod merge
Marfuen Feb 12, 2025
251f130
add migration check on PR
Marfuen Feb 12, 2025
4a6de50
remove migrations check since it's overkill
Marfuen Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions .github/workflows/database-migrations.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
name: Database Migrations

on:
push:
branches:
- main
paths:
- "packages/db/prisma/schema.prisma"
- "packages/db/prisma/migrations/**"
workflow_dispatch: # Allows manual triggering

env:
NODE_VERSION: "20.x"

jobs:
migrate:
name: Run Database Migrations
runs-on: ubuntu-latest

steps:
- name: Checkout repository
uses: actions/checkout@v4

- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: ${{ env.NODE_VERSION }}
cache: "npm"

- name: Install dependencies
run: npm ci

- 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
Comment on lines +33 to +43

@coderabbitai coderabbitai Bot Feb 12, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add safety measures for database migrations.

Consider adding these safety measures:

  1. Verify migrations can be applied using prisma migrate reset in a test environment
  2. Create a database backup before applying migrations
  3. 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.

Suggested change
- 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

59 changes: 59 additions & 0 deletions apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/Actions/publish-policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
"use server";

import { authActionClient } from "@/actions/safe-action";
import { db, type OrganizationPolicy } from "@bubba/db";
import { z } from "zod";

const schema = z.object({
id: z.string(),
});

export type PublishPolicyResponse = {
success: boolean;
data?: OrganizationPolicy;
error?: string;
};

export const publishPolicy = authActionClient
.schema(schema)
.metadata({
name: "publish-policy",
track: {
event: "publish-policy",
channel: "server",
},
})
.action(async ({ ctx, parsedInput }) => {
const { user } = ctx;
const { id } = parsedInput;

if (!user.organizationId) {
return {
success: false,
error: "Not authorized - no organization found",
};
}
Comment on lines +30 to +35

@coderabbitai coderabbitai Bot Feb 12, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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",
};
}


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",
};
}
Comment on lines +37 to +58

@coderabbitai coderabbitai Bot Feb 12, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve error handling and add policy existence check.

The current implementation has two areas for improvement:

  1. No validation that the policy exists before attempting to update
  2. 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.

Suggested change
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'}`,
};
}

});
29 changes: 29 additions & 0 deletions apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/Components/PolicyOverview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
"use client";

import type { JSONContent } from "@tiptap/react";
import PolicyEditor from "@/components/editor/advanced-editor";
import { usePolicy } from "@/app/[locale]/(app)/(dashboard)/policies/hooks/usePolicy";
import { Button } from "@bubba/ui/button";
import { Separator } from "@bubba/ui/separator";

export function PolicyOverview({ policyId }: { policyId: string }) {
const { data: policy } = usePolicy({ policyId });

if (!policy) return null;

const content = policy.content as JSONContent;

if (!content) return null;

return (
<div className="h-[calc(100vh-8rem)] flex flex-col py-4 gap-4">
<div className="flex justify-end">
<Button variant="secondary" className="w-fit">
Publish
</Button>
</div>
<Separator className="opacity-50" />
<PolicyEditor policyId={policyId} content={content} />
</div>
);
}
2 changes: 1 addition & 1 deletion apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export default async function Layout({
}

return (
<div className="max-w-[1200px] space-y-4">
<div className="max-w-[1200px] m-auto space-y-4">
<main className="h-[calc(100vh-4rem-4rem)]">{children}</main>
</div>
);
Expand Down
2 changes: 0 additions & 2 deletions apps/app/src/app/[locale]/(app)/(dashboard)/policies/[id]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import { auth } from "@/auth";
import { PolicyOverview } from "@/components/policies/policy-overview";
import { redirect } from "next/navigation";

interface PageProps {
params: Promise<{ id: string }>;
Expand Down
25 changes: 24 additions & 1 deletion apps/app/src/components/policies/policy-overview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,22 @@
import type { JSONContent } from "@tiptap/react";
import PolicyEditor from "../editor/advanced-editor";
import { usePolicy } from "@/app/[locale]/(app)/(dashboard)/policies/hooks/usePolicy";
import { Button } from "@bubba/ui/button";
import { Separator } from "@bubba/ui/separator";
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({ id: policyId }),
{
onSuccess: () => {
toast.success("Policy published successfully");
},
}
);
Comment on lines +14 to +21

@coderabbitai coderabbitai Bot Feb 12, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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...");
},
}
);


if (!policy) return null;

Expand All @@ -14,7 +27,17 @@ export function PolicyOverview({ policyId }: { policyId: string }) {
if (!content) return null;

return (
<div className="h-[calc(100vh-8rem)] flex flex-col">
<div className="h-[calc(100vh-8rem)] flex flex-col py-4 gap-4">
<div className="flex justify-end">
<Button
variant="secondary"
className="w-fit"
onClick={() => execute({ id: policyId })}
>
{isExecuting ? "Publishing..." : "Publish"}
</Button>
</div>
<Separator className="opacity-50" />
<PolicyEditor policyId={policyId} content={content} />
</div>
);
Expand Down