fix: remove unnecessary environmentID from service delete#193
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughRemoved the temporary service expose feature and its types; simplified service deletion by removing the environmentID parameter across the CLI, API interface, and client implementation. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant CLI
end
rect rgba(200,255,200,0.5)
participant ParamFiller
end
rect rgba(255,200,200,0.5)
participant APIClient
end
rect rgba(255,255,200,0.5)
participant Server
end
CLI->>ParamFiller: resolve service ID (interactive/non-interactive)
ParamFiller-->>CLI: returns serviceID
CLI->>APIClient: DeleteService(ctx, serviceID)
APIClient->>Server: GraphQL mutation deleteService(_id: $id)
Server-->>APIClient: deletion result
APIClient-->>CLI: result / error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/cmd/service/delete/delete.go (1)
47-56: Consider usingServiceByNameinstead ofServiceByNameWithEnvironmentto avoid unnecessary environment selection.The
environmentIDvariable is populated byServiceByNameWithEnvironmentbut never used after the call, sinceDeleteServiceonly requires the service ID. Prompting the user for an environment they don't need to select degrades the interactive experience.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmd/service/delete/delete.go` around lines 47 - 56, Replace the call to f.ParamFiller.ServiceByNameWithEnvironment (which fills environmentID) with the simpler f.ParamFiller.ServiceByName variant so the user isn't prompted for an environment that isn't used; remove the unused environmentID variable and call ServiceByName with the same ProjectCtx, ServiceID (&opts.id), ServiceName (&opts.name) and CreateNew=false (use the ServiceByName options struct used by that helper) and return any error as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/cmd/service/delete/delete.go`:
- Around line 47-56: Replace the call to
f.ParamFiller.ServiceByNameWithEnvironment (which fills environmentID) with the
simpler f.ParamFiller.ServiceByName variant so the user isn't prompted for an
environment that isn't used; remove the unused environmentID variable and call
ServiceByName with the same ProjectCtx, ServiceID (&opts.id), ServiceName
(&opts.name) and CreateNew=false (use the ServiceByName options struct used by
that helper) and return any error as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 526a51ef-e36d-4834-a725-16c8dc477e79
📒 Files selected for processing (3)
internal/cmd/service/delete/delete.gopkg/api/interface.gopkg/api/service.go
The underlying `exposeTempTcpPort` mutation does not exist in the backend. The CLI command has been removed in zeabur/cli#193. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2668787 to
eb399f8
Compare
…pose 1. deleteService: The backend mutation only accepts `_id`, but the CLI was passing an extra `environmentID` causing 422 errors. Removed from API client, interface, and delete command. 2. service expose: The `exposeTempTcpPort` mutation does not exist in the backend. Removed the entire command, API method, and model. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
eb399f8 to
cc7e726
Compare
- service-delete: remove `--env-id` flag (removed from CLI, backend `deleteService` never required environmentID) - service-delete: add warning that `--name` requires project context - service-expose: remove skill entirely (`exposeTempTcpPort` mutation does not exist in backend, command removed from CLI) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- service-delete: remove `--env-id` flag (removed from CLI, backend `deleteService` never required environmentID) - service-delete: add warning that `--name` requires project context - service-expose: remove skill entirely (`exposeTempTcpPort` mutation does not exist in backend, command removed from CLI) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fix: sync skills with CLI cleanup (zeabur/cli#193)
Summary
environmentIDparameter fromDeleteServiceAPI client method--env-idflag and environment resolution from theservice deletecommandRoot Cause
The backend's
deleteServiceGraphQL mutation only accepts_id:But the CLI was sending an extra
environmentIDthat doesn't exist in the schema:This caused 422 Unprocessable Entity errors when deleting services.
Test plan
npx zeabur@latest service delete -i=false --id <service-id> -yshould succeed without needing--env-idgo build ./...passes🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Removed Features