feat: Add Projects management feature with CRUD API, tests, and UI#473
feat: Add Projects management feature with CRUD API, tests, and UI#473devin-ai-integration[bot] wants to merge 1 commit intomainfrom
Conversation
- Add projects table to database with name, description, client_id, start_date, status fields - Add Joi validation schemas for project create/update - Add full CRUD API endpoints at /api/projects - Add comprehensive backend tests (31 tests covering all endpoints and error cases) - Add Project TypeScript types and API client methods - Add ProjectsPage React component with table view, create/edit/delete dialogs - Add Projects navigation item to sidebar layout - Status supports active/completed/on-hold with color-coded chips - Projects can be assigned to existing clients via dropdown
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| db.run( | ||
| 'INSERT INTO projects (name, description, client_id, start_date, status, user_email) VALUES (?, ?, ?, ?, ?, ?)', | ||
| [name, description || null, clientId || null, startDate || null, status || 'active', req.userEmail], |
There was a problem hiding this comment.
🔴 Missing client ownership validation allows cross-user data leakage
The POST / and PUT /:id project routes accept a clientId but never verify that it belongs to the authenticated user. The existing workEntries.js:90-102 route has this check (SELECT id FROM clients WHERE id = ? AND user_email = ?) before inserting, but the new projects routes skip it entirely. A malicious user can create or update a project with any client_id, and the LEFT JOIN clients c ON p.client_id = c.id in the GET queries (projects.js:16-18) would then return another user's client_name, leaking their data.
Prompt for agents
The POST / and PUT /:id handlers in backend/src/routes/projects.js accept a clientId from the request body but never validate that the referenced client belongs to the authenticated user. Compare with the pattern in backend/src/routes/workEntries.js:90-102, which does SELECT id FROM clients WHERE id = ? AND user_email = ? before inserting. The fix should add a similar ownership check in both the create handler (before the INSERT, around line 75) and the update handler (when value.clientId is provided, around line 154). If clientId is provided and non-null, query the clients table to verify the client belongs to req.userEmail. Return a 400 error like 'Client not found or does not belong to user' if the check fails. The clientId is optional/nullable for projects, so only perform the check when a non-null clientId is provided.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| const projectPayload = { | ||
| name: formData.name, | ||
| description: formData.description || undefined, |
There was a problem hiding this comment.
🟡 Description field cannot be cleared when editing a project due to || undefined coercion
In handleSubmit, description uses formData.description || undefined (line 165), which converts an empty string to undefined. Since undefined is stripped during JSON serialization, the backend never receives the field and won't update it. In contrast, clientId and startDate in the same payload use null (lines 166-167), which IS serialized and correctly clears those fields. This inconsistency means a user can clear a project's client or start date, but cannot clear its description once set.
| description: formData.description || undefined, | |
| description: formData.description || null, |
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Adds a full "Projects" management feature to the timesheet app, following existing codebase patterns (clients, work entries).
Backend:
projectstable in SQLite withname,description,client_id(FK → clients),start_date,status(active/completed/on-hold via CHECK constraint), anduser_emailscopingprojectSchema,updateProjectSchema) with status enum validation/api/projects: list all, get by ID, create, update, delete single, delete allclient_namein responsesFrontend:
Project,CreateProjectRequest,UpdateProjectRequestTypeScript interfacesProjectsPagecomponent with table view, create/edit dialog (with client dropdown + status select + date picker), delete with confirmationReview & Testing Checklist for Human
PRAGMA foreign_keys = ONis required). Theclient_idFK on the projects table and theON DELETE SET NULLbehavior may be silently unenforced. Checkdatabase/init.jsfor whether this pragma is set.client_idexists — Unlike work entries (which check client existence before insert), the projects route trusts the FK constraint. If FK enforcement is off, users can assign non-existent client IDs to projects. Manually test creating a project with an invalidclientId.client_idis set to null (not orphaned or errored).Notes
clientsCRUD pattern for consistency.Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/e7740419175444d893dd063cabfd2702