feat: add GET /api/v1/check endpoint#244
Conversation
WalkthroughA new API endpoint, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Router
participant Store
Client->>API_Router: GET /api/v1/check
API_Router->>Store: getAllChecks()
Store-->>API_Router: [Array of Check objects]
API_Router-->>Client: 200 OK + JSON array of checks
alt Error occurs
API_Router-->>Client: 500 Internal Server Error + error message
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-check-list
branch
from
1eabdd0 to
7bb1e80
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: 0
🔭 Outside diff range comments (1)
src/store/index.js (1)
219-253: Duplicate accessor clutters the public API
getAllComplianceChecks(line 220) and the newly-addedgetAllChecks(line 252) expose the same data. Two names for the same thing will confuse consumers and make refactors harder.- getAllComplianceChecks: () => getAll('compliance_checks'), + // Deprecated alias – remove once all callers migrate to getAllChecks + // getAllComplianceChecks: () => getAll('compliance_checks'), … getAllChecks: () => getAll('compliance_checks')Either keep a single canonical accessor or add a deprecation note/alias for backward compatibility.
🧹 Nitpick comments (4)
src/httpServer/routers/apiV1.js (1)
124-132: Endpoint lacks pagination & could DoS the service
/checkreturns the fullcompliance_checkstable in one go. On large deployments this may lead to huge payloads and long DB scans.Quick win:
-const checks = await getAllChecks() -res.json(checks) +const { limit = 100, offset = 0 } = req.query +const checks = await knex('compliance_checks') + .limit(limit) + .offset(offset) + .select('*') +res.json(checks)At minimum add
limit/offsetquery-string handling and reflect it in the OpenAPI spec.__tests__/httpServer/apiV1.test.js (1)
393-404: Test compares serialised blobs – use explicit shape assertionsStringifying both sides hides subtle divergences (e.g., type coercion, missing fields). Prefer matching selected fields or deep equality with proper Date handling:
-// @TODO: find a more elegant way to solve the issue with the date format -expect(response.body).toStrictEqual(JSON.parse(JSON.stringify(storedChecks))) +expect(response.body).toHaveLength(storedChecks.length) +storedChecks.forEach((check, idx) => { + expect(response.body[idx]).toMatchObject({ + id: check.id, + code_name: check.code_name, + title: check.title + }) +})Makes failures more readable and resilient to cosmetic differences.
src/httpServer/swagger/api-v1.yml (2)
214-235: Trailing whitespace breaksyamllint& CILine 362 (and a few others) contains a stray space, flagged by YAMLlint. Remove it to keep the spec lint-clean.
+362
214-235: Plural noun recommended for collection endpointREST conventions favour plural resources (
/checks) when returning a collection. Consider renaming the path and operation to keep the API consistent (/projects,/workflows, etc.).Update router, tests, and spec accordingly if you decide to adopt this.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
__tests__/httpServer/apiV1.test.js(3 hunks)src/httpServer/routers/apiV1.js(2 hunks)src/httpServer/swagger/api-v1.yml(2 hunks)src/store/index.js(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
src/httpServer/swagger/api-v1.yml
[error] 362-362: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
src/httpServer/routers/apiV1.js (1)
30-30: Destructuring now depends on the duplicate store method
getAllChecksis imported, butgetAllComplianceChecksis still exported by the store. If the duplication is resolved (see store comment) make sure this destructuring is updated accordingly; otherwise the router will break at runtime.
1 participant
Related #216
Summary by CodeRabbit
New Features
/api/v1/checkto retrieve a list of all checks.Tests