feat: add POST /api/v1/project endpoint
#239
Conversation
WalkthroughA new POST Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Router
participant Store
participant DB
Client->>API_Router: POST /api/v1/project { name }
API_Router->>Store: getProjectByName(name)
Store->>DB: SELECT * FROM projects WHERE name = ?
DB-->>Store: project or null
Store-->>API_Router: project or null
alt project exists
API_Router-->>Client: 409 Conflict
else invalid name
API_Router-->>Client: 400 Bad Request
else valid and unique
API_Router->>Store: addProject({ name, ... })
Store->>DB: INSERT INTO projects ...
DB-->>Store: new project
Store-->>API_Router: new project
API_Router-->>Client: 201 Created + Location header + project JSON
end
alt unexpected error
API_Router-->>Client: 500 Internal Server Error
end
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
UlisesGascon
force-pushed
the
ulises/v1-create-project
branch
from
74f0f2e to
e17a443
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/store/index.js (1)
46-60: 🛠️ Refactor suggestionRace-condition risk when checking duplicates – add a DB uniqueness constraint
getProjectByName+ the pre-insert duplicate check inaddProjectrun outside a transaction.
Two concurrent requests with the same slug can still slip through and hit theINSERT, causing a duplicate row (or whichever wins last).Hard-lock it at the persistence layer:
-- create table projects ( ... - name text not null, +-- add a UNIQUE constraint so the DB enforces slug uniqueness + name text not null UNIQUE,Then in code, rely on catching the database error:
- const projectExists = await knex('projects').where({ name }).first() - if (projectExists) { - throw new Error(`Project with name (${name}) already exists`) - } - return knex('projects').insert(project).returning('*').then(r => r[0]) + try { + return await knex('projects').insert(project).returning('*').then(r => r[0]) + } catch (err) { + if (err.code === '23505') { // unique_violation in Postgres + throw new Error(`Project with name (${name}) already exists`) + } + throw err + }This removes the timing window and the extra SELECT.
🧹 Nitpick comments (4)
src/store/index.js (1)
250-252: Minor: exposegetProjectByNamevia same “getter” styleOther getters are anonymous wrappers:
getProjectById: (id) => getOne('projects', id).
For consistency you could mirror that style:- getProjectByName: getProjectByName(knex) + getProjectByName: (name) => getProjectByName(knex)(name)Not blocking – just stylistic.
src/httpServer/routers/apiV1.js (1)
21-26: Double validation:kebabCasemay mask bad input
_.kebabCase('Invalid Name!')becomesinvalid-name, which passesisSlug, so the later 400 branch is never hit.
OpenAPI validation already rejects bad patterns before we reach the handler. Consider:
- Remove the in-handler slug check (keep OpenAPI as single source of truth), or
- Validate the raw
namebefore transforming.Keeps behaviour predictable.
__tests__/httpServer/apiV1.test.js (1)
83-95: Test too brittle – asserts entire OpenAPI error payloadThe exact shape of the validation error object is owned by
swagger-endpoint-validator; a minor lib upgrade will break this test without a code change.Prefer partial matching:
- expect(response.body).toStrictEqual({ errors: [...] }) + expect(response.body).toHaveProperty('errors') + expect(response.body.errors[0]).toMatchObject({ + errorCode: 'pattern.openapi.validation', + path: '/body/name' + })Keeps intent while future-proofing.
src/httpServer/swagger/api-v1.yml (1)
290-290: YAML lint: add trailing newlineThe file lacks a terminating newline, flagged by yamllint.
@@ - - updated_at \ No newline at end of file + - updated_at +🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 290-290: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
__tests__/httpServer/apiV1.test.js(3 hunks)package.json(1 hunks)src/httpServer/routers/apiV1.js(1 hunks)src/httpServer/swagger/api-v1.yml(2 hunks)src/store/index.js(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
src/httpServer/swagger/api-v1.yml
[error] 290-290: no new line character at the end of file
(new-line-at-end-of-file)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/httpServer/swagger/api-v1.yml (1)
88-93: Align error response examples with schema
Provide consistentexampleblocks for the400,409, and500responses that match theErrorResponsestructure (anerrorsarray ofErrorObject).
🧹 Nitpick comments (6)
src/httpServer/swagger/api-v1.yml (6)
58-63: Enhance request schema with descriptions
Consider adding adescriptionfor thenameproperty to clarify that it must be a slug (alphanumeric, dash, underscore) and optionally include aminLength: 1constraint to forbid empty strings.
65-75: Include response examples for 201
It’s helpful to provide a concreteexampleunderapplication/jsonfor the201response so clients can see the exactProjectpayload shape.
76-93: Reorder HTTP response codes
For readability and consistency, sort the response codes in ascending order (400,409,500) instead of listing400last.
152-271: Consider refactoring policy flags into a nested object
TheProjectschema has a long flat list of nullable boolean policy fields. To improve maintainability and reduce verbosity, nest these under a singlepoliciesobject or usepatternPropertiesinstead of enumerating each flag.
298-304: Set a maximum for theerrorsarray
To adhere to array-size best practices (e.g., Checkov CKV_OPENAPI_21), consider adding amaxItemsconstraint (such asmaxItems: 10) to theerrorsarray inErrorResponse.🧰 Tools
🪛 Checkov (3.2.334)
[MEDIUM] 299-304: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
317-317: Add newline at end-of-file
YAML files should end with a single newline to satisfy linters like YAMLlint.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 317-317: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
__tests__/httpServer/apiV1.test.js(3 hunks)src/httpServer/routers/apiV1.js(1 hunks)src/httpServer/swagger/api-v1.yml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/httpServer/apiV1.test.js
- src/httpServer/routers/apiV1.js
🧰 Additional context used
🪛 Checkov (3.2.334)
src/httpServer/swagger/api-v1.yml
[MEDIUM] 299-304: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🪛 YAMLlint (1.37.1)
src/httpServer/swagger/api-v1.yml
[error] 317-317: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Playwright Tests
Related #216
Summary by CodeRabbit