fix(deploy): resolve server-group names passed to -s/--server (DHQ-586)#11
Conversation
dhq deploy -s "<group name>" was documented as supported but only worked when the group's UUID was passed; a display name fell through to the server-only resolver and triggered the picker or "Multiple servers found" error in non-interactive mode. Add a resolveGroupName helper mirroring resolveServerName's exact / normalized / contains tiers, and try it as a fallback in runDeploy before showing the server picker. On a unique group match, the group's identifier is used as parent_identifier and resolvedServer stays nil — downstream code already tolerates this (GetServer 404s for groups). Refs DHQ-586. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WalkthroughThis PR adds server-group name resolution to the deploy command. It introduces a ChangesServer-group resolution feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/commands/deploy_test.go (1)
303-356: ⚡ Quick winAdd one deploy-flow test for server-precedence then group fallback wiring.
These tests cover
resolveGroupNamewell, but they don’t verify the actualdeploycommand path (server-name precedence, then group fallback settingparent_identifierbehavior). A singlehttptest.NewServer-backed command test here would lock in DHQ-586 behavior and prevent regressions.As per coding guidelines, "Use Go unit tests with
httptest.NewServerfor recorded response shapes andintegration_test.gofor validating types against real API JSON (golden tests)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/commands/deploy_test.go` around lines 303 - 356, Add a new unit test in deploy_test.go that exercises the deploy command end-to-end using httptest.NewServer to assert server-name precedence over group fallback and that when a group is used the request payload includes parent_identifier; specifically, invoke the CLI deploy code path (the same helper/tests that call the deploy command) with a mock API server that records incoming requests, assert that when both --server and --group are provided the server identifier is used, and in the fallback case (no server) the deployed server's POST/PUT contains parent_identifier set to the resolved group's Identifier (use resolveGroupName to derive expected values and inspect the request body from the httptest server).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/commands/deploy_test.go`:
- Around line 303-356: Add a new unit test in deploy_test.go that exercises the
deploy command end-to-end using httptest.NewServer to assert server-name
precedence over group fallback and that when a group is used the request payload
includes parent_identifier; specifically, invoke the CLI deploy code path (the
same helper/tests that call the deploy command) with a mock API server that
records incoming requests, assert that when both --server and --group are
provided the server identifier is used, and in the fallback case (no server) the
deployed server's POST/PUT contains parent_identifier set to the resolved
group's Identifier (use resolveGroupName to derive expected values and inspect
the request body from the httptest server).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0652ee4-c2d3-4b60-9b0c-141db582f788
📒 Files selected for processing (2)
internal/commands/deploy.gointernal/commands/deploy_test.go
Summary
dhq deploy -s \"<group name>\"was documented as supported (deployhq.com/support/cli/cli-deploying) but only worked when a UUID was passed; a display name fell through to the server-only resolver and either triggered the interactive picker or failed with "Multiple servers found" in non-interactive mode.resolveGroupNamehelper mirroringresolveServerName's exact / normalized / contains tiers, and tries it as a fallback before falling back to the server picker.parent_identifierandresolvedServerstays nil — downstream code already tolerates this (GetServer 404s for groups; see existing comments at deploy.go:40 and :325).Test plan
go test ./...— 6 new unit tests forresolveGroupName(exact, case-insensitive, normalized, contains, ambiguous, no-match) all pass; existing suite still greengolangci-lint run— cleandhq deploy -p <project> -s \"<group name>\" --dry-run --json, confirmparent_identifierin the request matches the group identifier (not a server)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests