feat(projects): Add permission checks to delete project

[jasonpetro]

This PR introduces robust permission checks to the deleteProject action, ensuring only authorized users can remove projects.

Changes

• Integrated permissions utility within src/actions/projects/deleteProject/action.ts to validate user access before project deletion.
• Returns a Result.err with a clear unauthorized error if the requesting user lacks the necessary permissions.
• This change directly enhances security and data integrity for project management.

Summary by CodeRabbit

• Bug Fixes
  • Clearer messaging and status when a project deletion fails due to lack of authorization or when the project doesn't exist.
• Tests
  • Added unit tests covering unauthorized attempts, successful deletions, and deletion failures to ensure correct behavior and feedback.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Replaces the not-found/permission error path in the project deletion action with a user-facing message and changes the error code from ErrorCodes.NOT_FOUND to ErrorCodes.UNAUTHORIZED; adds unit tests covering unauthorized, success, and delete-failure scenarios.

Changes

Cohort / File(s)	Summary
Action: error message & code src/actions/projects/deleteProject/action.ts	Replaced previous not-found/permission error with message "You are not authorized to delete this project or it does not exist." and changed error code from ErrorCodes.NOT_FOUND to ErrorCodes.UNAUTHORIZED.
Tests: unit coverage added src/actions/projects/deleteProject/action.test.ts	New tests added (mocks for Prisma, session, and deleteProject logic) covering unauthorized path, successful deletion, and deletion logic failure.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Pay attention to test mocks (Prisma/session/deleteProject) and assertions for the exact error code/message.
• Verify no behavioral changes beyond error messaging and that existing callers expect the new UNAUTHORIZED code.

Possibly related PRs

• feat: Implement project deletion functionality #40 — Related implementation of the deleteProject action that this change updates (error handling/message).

Suggested reviewers

• fehranbit
• Stereco-btc

Poem

🐇 I hopped through code with careful paws,

A message tuned without a pause.
Not-found now reads unauthorized and clear,
Tests watch the doors so bugs don't appear.
🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat(projects): Add permission checks to delete project' accurately summarizes the main change: adding permission validation to the deleteProject action before deletion.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/delete-project-permissions

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/actions/projects/deleteProject/action.ts (1)

26-29: Validate impact of switching from NOT_FOUND to UNAUTHORIZED and consider de‑duplicating the message

The new message and use of ErrorCodes.UNAUTHORIZED in the !existingProject path look consistent with the intent to avoid leaking whether the project exists. The only two follow‑ups I'd suggest:

1. Confirm no callers or tests rely on this branch returning ErrorCodes.NOT_FOUND (e.g., to distinguish stale links vs. permission issues). If such logic exists, it will now see an UNAUTHORIZED instead.
2. Very minor DRY improvement: avoid repeating the same literal string for both the toast and the error:

-      if (!existingProject) {
-        toast.error('You are not authorized to delete this project or it does not exist.');
-        return error('You are not authorized to delete this project or it does not exist.', ErrorCodes.UNAUTHORIZED);
-      }
+      if (!existingProject) {
+        const message = 'You are not authorized to delete this project or it does not exist.';
+        toast.error(message);
+        return error(message, ErrorCodes.UNAUTHORIZED);
+      }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f6aad90 and 1ede5bb.

📒 Files selected for processing (1)

• src/actions/projects/deleteProject/action.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/actions/projects/deleteProject/action.ts (2)

src/components/ui/sonner.tsx (1)

• toast (26-26)

src/lib/result.ts (1)

• error (38-44)

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (5)

src/actions/projects/deleteProject/action.test.ts (5)

1-26: Mocks and wiring align with the action’s dependencies

The module mocks for prisma, getSession, and deleteProject match how deleteProjectAction uses them, so the tests are nicely isolated. One minor nit: since deleteProject is mocked and you never assert on prisma.project.delete, that mock field is currently unused — you can drop it now or keep it if you plan to assert against it later.

---

32-38: Session mocking is consistent; consider asserting it’s actually used

The shared beforeEach correctly ensures every invocation of deleteProjectAction sees the same mockUserId and a valid save function. As an optional tightening, you could add expect(getSession).toHaveBeenCalled() in one test to guarantee the action still flows through the auth/session layer and doesn’t regress to using a hard‑coded user.

---

40-60: Unauthorized-path test gives good coverage; could be slightly less brittle

This test cleanly verifies:

• the UNAUTHORIZED result (message + ErrorCodes.UNAUTHORIZED), and
• that the Prisma findFirst filter includes both id and userId, while deleteProject is not invoked.

To make it a bit more resilient to future changes (e.g., adding extra selected fields), you could switch the Prisma assertion to expect.objectContaining:

expect(prisma.project.findFirst).toHaveBeenCalledWith(
  expect.objectContaining({
    where: { id: mockProjectId, userId: mockUserId },
  }),
);

This still guarantees the permission filter while tolerating additional options in the query.

---

62-82: Success-path test validates both authorization and delegation to business logic

The success test correctly:

• stubs findFirst to return a project,
• stubs deleteProject to succeed,
• asserts result.success === true and that result.data is the value returned from deleteProject, and
• verifies the Prisma filter again includes both id and userId, and that deleteProject is called with the parsed input.

As with the previous test, consider using expect.objectContaining for the Prisma call to reduce brittleness if the query evolves while still locking in the permission check.

---

84-105: Delete-failure test nicely pins the BAD_REQUEST behavior

This test cleanly covers the branch where deleteProject returns a failed Result, ensuring:

• the action surfaces the same error message,
• uses ErrorCodes.BAD_REQUEST, and
• still enforces the (id, userId) filter and delegates to deleteProject with the correct payload.

If you want even fuller coverage in the future, you could add a separate test where Prisma (or deleteProject) throws to exercise the catch path and errorFromException, but that’s optional for this PR.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ede5bb and 85e3313.

📒 Files selected for processing (2)

• src/actions/projects/deleteProject/action.test.ts (1 hunks)
• src/actions/projects/deleteProject/action.ts (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/actions/projects/deleteProject/action.ts

🧰 Additional context used

🧬 Code graph analysis (1)

src/actions/projects/deleteProject/action.test.ts (4)

src/lib/session.ts (1)

• getSession (33-36)

src/lib/prisma.ts (1)

• prisma (14-18)

src/actions/projects/deleteProject/action.ts (1)

• deleteProjectAction (10-45)

src/actions/projects/deleteProject/logic.ts (1)

• deleteProject (7-13)

[fehranbit]

the added permission checks for project deletion are a solid move for security. approved to merge! 🚀