chore: get env by name endpoint#1061
Conversation
📝 WalkthroughWalkthroughA new GET endpoint is introduced to retrieve an environment by name within a workspace (GET Changes
Sequence DiagramsequenceDiagram
participant Client
participant RouteHandler as Route Handler
participant Database as Environment<br/>Lookup
participant SystemHelper as getEnvironmentWithSystems<br/>Helper
participant SystemDB as System<br/>Lookup
Client->>RouteHandler: GET /v1/workspaces/{id}/environments/name/{name}
RouteHandler->>Database: Find environment by name
alt Environment found
Database-->>RouteHandler: Environment data
RouteHandler->>SystemHelper: getEnvironmentWithSystems(env)
SystemHelper->>SystemDB: Fetch associated systems
SystemDB-->>SystemHelper: Systems list
SystemHelper-->>RouteHandler: EnvironmentWithSystems
RouteHandler-->>Client: 200 EnvironmentWithSystems
else Environment not found
Database-->>RouteHandler: null
RouteHandler-->>Client: 404 ErrorResponse
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/routes/v1/workspaces/environments.ts (1)
73-80:⚠️ Potential issue | 🟡 Minor
system.slugis incorrectly populated fromsystem.name.In the mapped
systemsarray,slugis set tor.system.nameinstead ofr.system.slug. This pre-existing bug (previously insidegetEnvironment) is now shared by both thegetEnvironmentand the newgetEnvironmentByNameresponses, so API consumers of both endpoints will receive the environment's system name where the slug is expected. Since this PR extracts and reuses this logic, it's a good opportunity to correct it.🐛 Proposed fix
const systems = systemRows.map((r) => ({ id: r.system.id, workspaceId: r.system.workspaceId, name: r.system.name, - slug: r.system.name, + slug: r.system.slug, description: r.system.description, metadata: r.system.metadata, }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/routes/v1/workspaces/environments.ts` around lines 73 - 80, The systems mapping sets slug incorrectly to r.system.name; update the mapping that builds the systems array (where systemRows.map(...) creates const systems) to assign slug: r.system.slug instead of slug: r.system.name so both getEnvironment and getEnvironmentByName responses return the correct system.slug value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/api/src/routes/v1/workspaces/environments.ts`:
- Around line 73-80: The systems mapping sets slug incorrectly to r.system.name;
update the mapping that builds the systems array (where systemRows.map(...)
creates const systems) to assign slug: r.system.slug instead of slug:
r.system.name so both getEnvironment and getEnvironmentByName responses return
the correct system.slug value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b4213bb0-3013-46d7-a240-d2ce4db848ac
📒 Files selected for processing (6)
apps/api/openapi/openapi.jsonapps/api/openapi/paths/environments.jsonnetapps/api/src/routes/v1/workspaces/environments.tsapps/api/src/types/openapi.tse2e/api/schema.tse2e/tests/api/environments.spec.ts
There was a problem hiding this comment.
Pull request overview
Adds a new API endpoint to fetch a workspace environment by its name, and updates the OpenAPI artifacts and e2e coverage accordingly.
Changes:
- Implement
GET /v1/workspaces/{workspaceId}/environments/name/{name}in the environments router. - Extend OpenAPI path definitions and generated OpenAPI TypeScript types to include the new operation.
- Add Playwright e2e tests covering successful lookup by name and 404 for missing names.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| e2e/tests/api/environments.spec.ts | Adds e2e tests for “get environment by name” and missing-name behavior. |
| e2e/api/schema.ts | Updates generated e2e OpenAPI TS schema to include the new endpoint/operation. |
| apps/api/src/types/openapi.ts | Updates generated API-side OpenAPI TS types with getEnvironmentByName. |
| apps/api/src/routes/v1/workspaces/environments.ts | Implements the handler and wires the new /name/:name route; refactors shared “env + systems” response building. |
| apps/api/openapi/paths/environments.jsonnet | Defines the new OpenAPI path entry for lookup-by-name. |
| apps/api/openapi/openapi.json | Adds the new path to the compiled OpenAPI JSON. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const getEnvironmentByName: AsyncTypedHandler< | ||
| "/v1/workspaces/{workspaceId}/environments/name/{name}", | ||
| "get" | ||
| > = async (req, res) => { | ||
| const { workspaceId, name } = req.params; | ||
|
|
||
| const env = await db.query.environment.findFirst({ | ||
| where: and( | ||
| eq(schema.environment.name, name), | ||
| eq(schema.environment.workspaceId, workspaceId), | ||
| ), | ||
| }); | ||
|
|
||
| if (env == null) throw new ApiError("Environment not found", 404); | ||
|
|
||
| res.status(200).json(await getEnvironmentWithSystems(env)); |
There was a problem hiding this comment.
GET /environments/name/:name uses the raw environment name as a path segment, but environment names are not validated/normalized (create/upsert accept any string). This means some valid names (e.g. containing / or other reserved characters) cannot be reliably fetched via this endpoint. Consider either (a) enforcing a URL-safe name/slug format at create/upsert (and documenting it in OpenAPI with a pattern), or (b) changing this lookup to use a query parameter instead of a path parameter.
| await api.DELETE( | ||
| "/v1/workspaces/{workspaceId}/environments/{environmentId}", | ||
| { params: { path: { workspaceId: workspace.id, environmentId } } }, | ||
| ); |
There was a problem hiding this comment.
The new test deletes the created environment but doesn't assert the delete request succeeded. Adding a status expectation (e.g. 202) here would make the test more robust and reduce the chance of leaking test data when deletes fail.
| await api.DELETE( | |
| "/v1/workspaces/{workspaceId}/environments/{environmentId}", | |
| { params: { path: { workspaceId: workspace.id, environmentId } } }, | |
| ); | |
| const deleteRes = await api.DELETE( | |
| "/v1/workspaces/{workspaceId}/environments/{environmentId}", | |
| { params: { path: { workspaceId: workspace.id, environmentId } } }, | |
| ); | |
| expect(deleteRes.response.status).toBe(202); |
Summary by CodeRabbit
Release Notes
New Features
Tests