feat: Add Projects management feature with CRUD operations#463
feat: Add Projects management feature with CRUD operations#463devin-ai-integration[bot] wants to merge 2 commits intomainfrom
Conversation
- Add projects table to database with name, description, client assignment, start date, and status (active/completed/on-hold) - Add Joi validation schemas for project create/update - Add backend API endpoints: GET/POST/PUT/DELETE /api/projects - Add comprehensive backend tests (31 tests) - Add frontend ProjectsPage with table view and create/edit/delete dialogs - Add Project types and API client methods - Add Projects navigation item to sidebar layout
🤖 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-tenant data reference
When creating or updating a project, the clientId is inserted/updated directly without verifying that the client belongs to the authenticated user. This is unlike the work entries route (backend/src/routes/workEntries.js:90-102), which explicitly checks SELECT id FROM clients WHERE id = ? AND user_email = ? before allowing the operation.
A user can associate their project with another user's client by providing that client's ID. The GET queries then expose the other user's client name via the LEFT JOIN clients c ON p.client_id = c.id (e.g., backend/src/routes/projects.js:16-18), leaking cross-tenant data.
Affected locations
POST route (line 77) inserts clientId without ownership check:
[name, description || null, clientId || null, startDate || null, status || 'active', req.userEmail]PUT route (lines 154-157) updates client_id without ownership check:
if (value.clientId !== undefined) {
updates.push('client_id = ?');
values.push(value.clientId || null);
}Prompt for agents
In backend/src/routes/projects.js, both the POST (line 75-104) and PUT (line 154-157) routes accept a clientId but never verify that the client belongs to the authenticated user. The established pattern from backend/src/routes/workEntries.js (lines 90-102 for create, lines 173-193 for update) is to query SELECT id FROM clients WHERE id = ? AND user_email = ? before proceeding.
For the POST route: before the INSERT INTO projects query, add a conditional check when clientId is provided. Query the clients table to verify ownership (using clientId and req.userEmail). If the client is not found, return 400 with an appropriate error message. Only proceed with the insert if validation passes (or if clientId is null).
For the PUT route: similarly, when value.clientId is defined and not null, verify ownership of the client before building and executing the UPDATE query. This follows the same pattern as workEntries.js lines 173-193.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Adds a full "Projects" management feature to the timesheet application, covering backend API, frontend UI, and backend tests. Each project has a name, description, optional client assignment, start date, and status (
active/completed/on-hold).Backend:
projectstable in SQLite schema with FK toclients(ON DELETE SET NULL) andusers(ON DELETE CASCADE), plus a CHECK constraint on status valuesprojectSchema,updateProjectSchema)/api/projects(GET all, GET by id, POST, PUT, DELETE) — follows the same pattern as the existing/api/clientsroutesclients.test.jsstructureFrontend:
Project,CreateProjectRequest,UpdateProjectRequestTypeScript interfacesProjectsPagecomponent with table view, colored status chips, create/edit dialog (with client dropdown + date picker + status selector)/projectsrouteUpdates since last revision
|| undefinedwhich gets stripped during JSON serialization, so the backend never received the field. Changed to|| '', which the backend correctly normalizes tonullin the DB.Review & Testing Checklist for Human
client_idactually exists before inserting — it relies on the SQLite FK constraint, which would surface as a generic 500 error rather than a descriptive 400. The existingworkEntriesroutes DO validate client existence; this PR does not. Decide if this gap is acceptable.client_idset to NULL. Verify this is the desired behavior vs. blocking deletion or cascading.active,completed,on-hold) match product requirements. The CHECK constraint is enforced at the DB level in addition to Joi validation.Notes
startDateJoi schema allows bothnulland empty string'', coercing both tonullin the DB. This is intentional but reviewers should be aware of the normalization behavior.Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/8c62860204b74e93b1dc92edd92438fe