Skip to content

feat(environments): add new filtering and pagination flags#951

Merged
mbevc1 merged 3 commits into
mainfrom
20260610_list_envs_flags
Jun 11, 2026
Merged

feat(environments): add new filtering and pagination flags#951
mbevc1 merged 3 commits into
mainfrom
20260610_list_envs_flags

Conversation

@mbevc1

@mbevc1 mbevc1 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #934

  • Pagination params are only sent to the API when the user explicitly sets --page or --page-limit, preserving the existing full-list behaviour when no flags are passed.
  • The table printer handles both the legacy array response and the paginated response shape, appending a pagination footer for the latter.
  • Filters map to the corresponding query params on GET /api/v2/environments/{org}: name, type, space_id and tag are repeatable with OR semantics. As with pagination, nothing is sent when the flags are not used (feat: kosli list environments: add pagination and filtering flags #934).
  • Adds a TagEnv test helper mirroring TagFlow.
  • Map to the sort and sort_direction query params; sent only when set, so the API defaults (name, asc) apply otherwise. Valid values are enforced by the API, not the CLI.
  • The unknown-space-id case is blocked on the server returning a 5xx instead of an empty list, reported upstream in kosli-dev/server#5858. Re-enable the commented-out case once that is fixed.
  • Align flag ENV type description to include cloud-run

Fixes #934

* Pagination params are only sent to the API when the user explicitly
sets --page or --page-limit, preserving the existing full-list
behaviour when no flags are passed.

* The table printer handles both the legacy array response and the
paginated response shape, appending a pagination footer for the latter.

* Filters map to the corresponding query params on
`GET /api/v2/environments/{org}`: name, type, space_id and tag are repeatable with OR semantics. As with
pagination, nothing is sent when the flags are not used (#934).

* Adds a TagEnv test helper mirroring TagFlow.

* Map to the sort and sort_direction query params; sent only when set,
so the API defaults (name, asc) apply otherwise. Valid values are
enforced by the API, not the CLI.

* The `unknown-space-id` case is blocked on the server returning a `5xx`
instead of an empty list, reported upstream in kosli-dev/server#5858.
Re-enable the commented-out case once that is fixed.
@mbevc1 mbevc1 added enhancement New feature or request go Pull requests that update go code labels Jun 10, 2026
@claude

claude Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mbevc1's task in 2m 21s —— View job


Claude PR Review

  • Gather context & read changed files
  • Static analysis (go build, go vet, go mod tidy)
  • Review code quality, edge cases, bugs, tests
  • Post review summary

Static analysis: go build ./... ✓ · go vet ./cmd/kosli/... ✓ · go mod tidy → no diff ✓


This is a clean, well-scoped PR. The conditional-params design (nothing sent unless the flag is set) is the right call for preserving existing behaviour, and it's applied consistently across pagination, filters, and sort. Good test coverage with deterministic, uniquely-prefixed fixtures.

What's done well 👍

  • Single source of truth for env typesvalidEnvTypesList in root.go resolves the drift between newEnvTypeFlag and envTypeFilterFlag flagged in the earlier review. The comment correctly notes the server is the real authority.
  • Defensive nil-checking on the tags map in printEnvListAsTable — the old code did an unchecked env["tags"].(map[string]interface{}) type assertion that would panic if tags were absent; the new , ok form fixes a real latent bug.
  • Pagination-shape handling — the array-vs-wrapped-object fallback is a pragmatic way to support both response shapes, and the page-aware "No environments were found at page number N" message is a nice touch.
  • Good error-path tests--page 0, --page-limit 0, negative page, and the invalid---sort API passthrough are all covered.

Suggestions (non-blocking)

  1. --space-id has no test coverage (inline comment on listEnvironments_test.go). The only case is commented out pending kosli-dev/server#5858, but that blocker is specific to the unknown space-id behaviour. A positive case using a real space ID would cover the space_id query-param wiring independently of the upstream fix.

  2. Response-shape detection relies on unmarshal failure (listEnvironments.go ~L170). Trying []map[...] first and falling back to the wrapped object on error works, but it's implicit. Since withPagination is already known at request time, you could branch on that directly (or check the leading non-whitespace byte for [ vs {) to make the intent explicit and avoid masking a genuinely malformed array payload. Minor — current approach is functionally correct.

  3. Example block uses literal tabs for continuation indentation; depending on terminal width these can render slightly ragged under --help. Cosmetic only.

No correctness or security concerns found. Nice work.
· 20260610_list_envs_flags

Comment thread cmd/kosli/root.go Outdated
Comment thread cmd/kosli/root.go Outdated
Comment thread cmd/kosli/listEnvironments_test.go
@mbevc1 mbevc1 enabled auto-merge (squash) June 10, 2026 11:37
@mbevc1 mbevc1 merged commit 214bc0a into main Jun 11, 2026
20 checks passed
@mbevc1 mbevc1 deleted the 20260610_list_envs_flags branch June 11, 2026 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request go Pull requests that update go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: kosli list environments: add pagination and filtering flags

2 participants