feat: add POST /api/v1/project/{projectId}/gh-org endpoint#243
feat: add POST /api/v1/project/{projectId}/gh-org endpoint#243UlisesGascon merged 3 commits intomainfrom
POST /api/v1/project/{projectId}/gh-org endpoint#243Conversation
WalkthroughA new API endpoint, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Router
participant Store/DB
Client->>API_Router: POST /api/v1/project/{projectId}/gh-org { githubOrgUrl }
API_Router->>API_Router: Validate projectId & githubOrgUrl
API_Router->>Store/DB: getProjectById(projectId)
alt Project not found
API_Router-->>Client: 404 Project not found
else Project found
API_Router->>Store/DB: getAllGithubOrganizationsByProjectsId(projectId)
alt Org already exists
API_Router-->>Client: 409 Conflict
else Org does not exist
API_Router->>Store/DB: addGithubOrganization(projectId, githubOrgUrl)
alt Success
API_Router-->>Client: 201 Created (GithubOrganization)
else DB Error
API_Router-->>Client: 500 Internal Server Error
end
end
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
__tests__/httpServer/apiV1.test.js (1)
268-276: Test is tightly coupled to OpenAPI validator wordingThe assertion expects the exact message
must match pattern "https://github.com/[^/]+".
Any change in the schema or the validator library will break the test while the behaviour (400) remains correct. Consider asserting only the status code and maybe the presence of an error array, e.g.expect(response.status).toBe(400) expect(response.body).toHaveProperty('errors')This keeps the test valuable yet resilient.
src/httpServer/swagger/api-v1.yml (1)
214-214: Trailing whitespace detectedYAML-lint flagged trailing spaces on this line. Trim to keep the spec tidy.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 214-214: trailing spaces
(trailing-spaces)
📜 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(2 hunks)src/httpServer/swagger/api-v1.yml(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
__tests__/httpServer/apiV1.test.js (1)
src/store/index.js (2)
addProject(51-61)getAllGithubOrganizationsByProjectsId(135-143)
🪛 YAMLlint (1.37.1)
src/httpServer/swagger/api-v1.yml
[error] 214-214: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Playwright Tests
🔇 Additional comments (1)
src/httpServer/swagger/api-v1.yml (1)
181-186:Locationheader description mismatches implementationThe spec promises a
Locationheader but the handler currently omits it (see router comment). After adding the header, ensure the example value reflects the final URL (/api/v1/project/{projectId}/gh-org/{orgId}) to keep docs & code in sync.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
src/httpServer/swagger/api-v1.yml (7)
157-163: ConstrainprojectIdto a positive integer
To prevent invalid IDs (0 or negative), add aminimum: 1constraint under the parameter schema.
165-177: Enhance request body schema
Consider addingformat: uriand adescriptionforgithubOrgUrlto improve validation and generated docs.
178-213: Add examples to responses
Includeexampleorexamplesfor the 201 success payload (and error cases) to aid clients and documentation.
214-214: Remove trailing spaces
YAMLlint flags trailing whitespace on this line. Please delete any extra spaces.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 214-214: trailing spaces
(trailing-spaces)
269-272: Manage schema complexity
TheGithubOrganizationobject is very large. Splitting it into reusable sub-schemas or leveraging shared definitions will improve maintainability.
273-279: Add property descriptions
Fields likeidandloginlackdescription:entries. Document each property to enrich API docs.
279-479: Review extensibility of optional fields
The long list of nullable booleans and integers may be hard to maintain. Consider grouping related flags or usingadditionalProperties: truefor less-critical fields.
📜 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(2 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
🪛 YAMLlint (1.37.1)
src/httpServer/swagger/api-v1.yml
[error] 214-214: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Playwright Tests
ee0b516 to
43f357c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/httpServer/swagger/api-v1.yml (1)
214-214: Remove trailing whitespaceLine 214 has trailing spaces, which triggers a YAML linter error. Please remove the extra spaces.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 214-214: trailing spaces
(trailing-spaces)
📜 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(2 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
🪛 YAMLlint (1.37.1)
src/httpServer/swagger/api-v1.yml
[error] 214-214: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Playwright Tests
🔇 Additional comments (3)
src/httpServer/swagger/api-v1.yml (3)
157-164: Project ID parameter is defined correctlyThe
projectIdpath parameter is now typed asintegerwith an appropriate example, aligning with backend expectations.
165-177: Request body schema validation is strict and well-anchoredThe
githubOrgUrlproperty is correctly marked as required, withadditionalProperties: falseand an anchored regex pattern to prevent invalid URLs.
179-213: Responses section follows the established patternAll response codes (
201,400,404,409,500) and theLocationheader on201are defined consistently with existing endpoints.
43f357c to
e1bc818
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/httpServer/swagger/api-v1.yml (2)
173-175:⚠️ Potential issueAnchor the
githubOrgUrlpattern
The regex still allows trailing characters. Re-apply the earlier fix by anchoring the end of the pattern.- pattern: '^https://github.com/[^/]+' + pattern: '^https://github.com/[^/]+$'
270-479: 🛠️ Refactor suggestion
GithubOrganizationschema still lacks arequiredlist
Without it, the validator will happily emit partly-filled objects. At minimum mark the core fields as required:properties: id: type: integer example: 1 ... project_id: type: integer example: 1 + required: + - id + - login + - html_url + - created_at + - updated_at
🧹 Nitpick comments (1)
src/httpServer/swagger/api-v1.yml (1)
208-214: Trailing-space lint error
yamllintflags line 214 – remove the stray spaces to keep the spec clean.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 214-214: trailing spaces
(trailing-spaces)
📜 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(2 hunks)src/httpServer/swagger/api-v1.yml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/httpServer/routers/apiV1.js
🧰 Additional context used
🧬 Code Graph Analysis (1)
__tests__/httpServer/apiV1.test.js (1)
src/store/index.js (2)
addProject(51-61)getAllGithubOrganizationsByProjectsId(135-143)
🪛 YAMLlint (1.37.1)
src/httpServer/swagger/api-v1.yml
[error] 214-214: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Playwright Tests
🔇 Additional comments (3)
__tests__/httpServer/apiV1.test.js (3)
37-39:addProject/getAllGithubOrganizationsByProjectsIdmock extraction looks good
Pulling the extra store helpers into the test context improves readability and keeps each spec self-contained.
230-259: Nice happy-path coverage
The test exercises: status, body, header and DB side-effects – exactly what we want for an endpoint contract test.
316-324: Error-message assertion will break once the OpenAPI regex is tightened
Previous reviews asked to anchor the regex with$(see swagger file).
After that change the automatic validation message will include the$, so this assertion will fail.- expect(response.body.errors[0]).toHaveProperty('message', 'must match pattern "^https://github.com/[^/]+"') + expect(response.body.errors[0]).toHaveProperty( + 'message', + 'must match pattern "^https://github.com/[^/]+$"' + )Please adjust when you update the spec.
e1bc818 to
183d5bc
Compare
Related #216
Summary by CodeRabbit
New Features
Tests