Refine Project Update Input Validation

[jubilarhacker]

Overview: This PR addresses validation for the project update logic, enhancing data integrity and standardizing error responses.

Changes

• Implemented trimming for input fields within src/actions/projects/updateProject/logic.ts to remove extraneous whitespace.
• Added comprehensive length checks for project name and description to enforce predefined data constraints.
• Standardized error messages for invalid input, ensuring consistent feedback to the client.
• Focused changes on src/actions/projects/updateProject/logic.ts.

Summary by CodeRabbit

• Bug Fixes

  • Added validation for project titles to reject empty titles and titles exceeding 200 characters, returning descriptive error messages to users.

• Tests

  • Enhanced test coverage for project title validation scenarios with structured assertions.

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

[coderabbitai]

Walkthrough

This change adds title validation to the updateProject logic by trimming whitespace and enforcing a maximum length of 200 characters, returning BAD_REQUEST errors for empty or oversized titles. Tests are updated to validate these constraints using result-based assertions instead of exception-throwing patterns.

Changes

Cohort / File(s)	Summary
Title Validation in updateProject src/actions/projects/updateProject/logic.ts, src/actions/projects/updateProject/logic.test.ts	Implementation adds title trimming and enforces max 200-character constraint, rejecting empty or oversized titles with BAD_REQUEST errors. Tests replace error-throwing expectations with two new BAD_REQUEST validation tests for empty and overlength titles, using result-based assertions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Validation logic is straightforward (trim + length checks)
• Consistent error handling pattern applied across both updateMany and update paths
• Pay attention to: boundary conditions (200-char limit), empty title edge case, and test assertions for both failure scenarios

Possibly related PRs

• improved next.js server action authorization and refactoring #15: Modifies the same updateProject implementation function in logic.ts
• Feature: Normalize project name input and sanitize whitespace #117: Adds related title normalization and whitespace validation logic
• Feature: Add Optimistic Concurrency Control to Project Updates #122: Also modifies updateProject logic.ts with title trimming and result-based error handling

Suggested reviewers

• fehranbit

Poem

🐰 A title trimmed, a length defined,
Two hundred chars, the perfect find.
No empty strings shall pass our way,
Validation strong—hop hooray! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'Refine Project Update Input Validation' directly aligns with the main changeset, which adds validation for project titles (trimming whitespace and enforcing length constraints) in the updateProject logic.

✨ 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 fix/project-update-validation

---

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/actions/projects/updateProject/logic.test.ts (1)

107-107: Pre-existing bug: Mismatched project ID in assertion.

The test input uses id: 'project-123' (line 97), but the assertion on line 107 expects id: 'project-100'. This appears to be a pre-existing bug in the test that should be corrected.

🔎 Proposed fix

     expect(prisma.project.updateMany).toHaveBeenCalledWith({
       where: {
-        id: 'project-100',
+        id: 'project-123',
         updatedAt: new Date('2025-01-01T09:00:00.000Z'),
       },
       data: { title: 'New Title' },
     });

🧹 Nitpick comments (2)

src/actions/projects/updateProject/logic.test.ts (1)

116-127: Good test coverage, but consider adding a trimming verification test.

The test correctly verifies that whitespace-only titles are rejected. However, there's no test that verifies the trimming behavior itself (e.g., ' Valid Title ' should be trimmed to 'Valid Title' and saved correctly).

🔎 Suggested additional test case

it('should trim whitespace from title before saving', async () => {
  (prisma.project.update as jest.Mock).mockResolvedValue({ 
    ...mockProject, 
    title: 'Trimmed Title' 
  });

  const input = { id: 'project-123', title: '  Trimmed Title  ' };
  const result = await updateProject(input);

  expect(result.success).toBe(true);
  expect(prisma.project.update).toHaveBeenCalledWith({
    where: { id: 'project-123' },
    data: { title: 'Trimmed Title' }, // Verify trimmed value is passed
  });
});

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

16-22: Verify schema constraints and address PR description inconsistency.

The validation logic looks correct, but there are a few concerns:

1. Schema verification needed: Line 16 calls title.trim() without a null/undefined check. If title can be null or undefined, this will throw a runtime error.
2. PR description inconsistency: The PR description mentions validating both "project name and description," but only title validation is present. Is description validation missing, or is the PR description outdated?
3. Magic number: The 200-character limit is hardcoded. Consider extracting it to a named constant for maintainability.

Run the following script to verify the schema definition:

#!/bin/bash
# Description: Check UpdateProjectInput schema to verify title is required/non-nullable

# Search for UpdateProjectInput schema definition
ast-grep --pattern 'export const UpdateProjectInputSchema = $$$'

# Also search for the type definition
rg -n "UpdateProjectInput" --type=ts -C5

🔎 Proposed refactor to extract magic number

+const MAX_TITLE_LENGTH = 200;
+
 export async function updateProject(input: UpdateProjectInput): Promise<Result<Project>> {
   const { id, title, lastUpdatedAt } = input;
 
   try {
     let updatedProject: Project;
 
     const trimmedTitle = title.trim();
     if (trimmedTitle.length === 0) {
       return error('Title is required', ErrorCodes.BAD_REQUEST);
     }
-    if (trimmedTitle.length > 200) {
+    if (trimmedTitle.length > MAX_TITLE_LENGTH) {
       return error('Title is too long', ErrorCodes.BAD_REQUEST);
     }

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf2c5bc and 92e26ea.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (2)

• src/actions/projects/updateProject/logic.test.ts (1 hunks)
• src/actions/projects/updateProject/logic.ts (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

src/actions/projects/updateProject/logic.test.ts (2)

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

• updateProject (10-69)

src/lib/prisma.ts (1)

• prisma (14-18)

src/actions/projects/updateProject/logic.ts (2)

src/lib/result.ts (1)

• error (38-44)

src/lib/prisma.ts (1)

• prisma (14-18)

🔇 Additional comments (2)

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

31-31: LGTM: Consistent use of trimmed title.

The trimmed title is correctly used in both the optimistic locking path (updateMany) and the standard update path, ensuring consistent behavior across both branches.

Also applies to: 54-54

src/actions/projects/updateProject/logic.test.ts (1)

129-141: LGTM: Title length validation test is thorough.

The test correctly verifies that titles exceeding 200 characters are rejected with appropriate error handling and no database calls.