From 8ec402b80a7b1f80825cdba09cde08fc79cc62ae Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Mon, 17 Aug 2026 07:58:01 +0200 Subject: [PATCH 1/2] docs: prepare CHANGELOG for v0.10.0 release --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 998a66c..04ed64b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +## [0.10.0] - 2026-08-17 + ### Changed - An invalid `cache_ttl` (unparseable, zero or negative) now fails the command instead of silently defaulting; the error names the config file, the expected duration syntax and `--refresh` From 27296b76ec757acaad8e94f59b67721e5183a38d Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Sun, 23 Aug 2026 19:06:10 +0200 Subject: [PATCH 2/2] fix(request): send filter syntax the UAR API accepts `grant request list --state`, `--result` and `--priority` all returned HTTP 400 BAD_REQUEST. Two defects in the filter builder: - a lone condition was wrapped a second time, emitting `((requestState eq PENDING))`; the API only accepts `(requestState eq PENDING)` - the priority operand was quoted, emitting `(priority eq 'High')`; the API only accepts `(priority eq High)` Both confirmed against the live API. Two or more conditions still get the single outer wrap, `((a) and (b))`, which the API accepts. `TestRequestList_ParamsFromFlags` had pinned the broken double-wrapped string, so the suite passed for the wrong reason; that expectation is corrected and `TestRequestList_FilterSyntaxMatchesLiveAPI` now pins the exact string for every flag combination. --- CHANGELOG.md | 4 ++ cmd/request_args_test.go | 81 +++++++++++++++++++++++++++++++++++++++- cmd/request_list.go | 21 +++++++++-- docs/mutation-ledger.md | 8 ++-- 4 files changed, 105 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 04ed64b..cbd5243 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Fixed + +- `grant request list --state`, `--result` and `--priority` now work; every filtered invocation previously failed with HTTP 400 + ## [0.10.0] - 2026-08-17 ### Changed diff --git a/cmd/request_args_test.go b/cmd/request_args_test.go index 555723f..8f502c4 100644 --- a/cmd/request_args_test.go +++ b/cmd/request_args_test.go @@ -245,7 +245,7 @@ func TestRequestList_ParamsFromFlags(t *testing.T) { name: "state becomes a filter", args: []string{"request", "list", "--state", "pending"}, wantSort: "createdAt desc", - wantFilt: "((requestState eq PENDING))", + wantFilt: "(requestState eq PENDING)", }, { name: "search becomes free text", @@ -294,6 +294,85 @@ func TestRequestList_ParamsFromFlags(t *testing.T) { } } +// TestRequestList_FilterSyntaxMatchesLiveAPI pins the exact OData filter +// strings the UAR API accepts. Measured against the live API: +// +// (requestState eq PENDING) -> 200 +// ((requestState eq PENDING)) -> 400 +// (requestState eq 'PENDING') -> 400 +// (priority eq High) -> 200 +// (priority eq 'High') -> 400 +// (requestResult eq APPROVED) -> 200 +// ((requestState eq FINISHED) and (priority eq High)) -> 200 +// +// So: operand values are never quoted, and a lone condition is sent bare -- +// the outer wrap is only correct when two or more conditions are combined. +func TestRequestList_FilterSyntaxMatchesLiveAPI(t *testing.T) { + tests := []struct { + name string + args []string + wantFilt string + }{ + { + name: "no filter flags", + args: []string{"request", "list"}, + wantFilt: "", + }, + { + name: "single state is not double wrapped", + args: []string{"request", "list", "--state", "PENDING"}, + wantFilt: "(requestState eq PENDING)", + }, + { + name: "single result is not double wrapped", + args: []string{"request", "list", "--result", "APPROVED"}, + wantFilt: "(requestResult eq APPROVED)", + }, + { + name: "single priority is unquoted and not double wrapped", + args: []string{"request", "list", "--priority", "High"}, + wantFilt: "(priority eq High)", + }, + { + name: "state and priority combine under one wrap", + args: []string{"request", "list", "--state", "FINISHED", "--priority", "High"}, + wantFilt: "((requestState eq FINISHED) and (priority eq High))", + }, + { + name: "state and result combine under one wrap", + args: []string{"request", "list", "--state", "FINISHED", "--result", "REJECTED"}, + wantFilt: "((requestState eq FINISHED) and (requestResult eq REJECTED))", + }, + { + name: "all three combine under one wrap", + args: []string{"request", "list", "--state", "FINISHED", "--result", "APPROVED", "--priority", "Low"}, + wantFilt: "((requestState eq FINISHED) and (requestResult eq APPROVED) and (priority eq Low))", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc := &mockAccessRequestService{} + + cmd := NewRequestCommandWithDeps(svc) + root := newTestRootCommand() + root.AddCommand(cmd) + + output, err := executeCommand(root, tt.args...) + if err != nil { + t.Fatalf("unexpected error: %v\noutput: %s", err, output) + } + + if len(svc.listCalls) != 1 { + t.Fatalf("expected exactly 1 ListRequests call, got %d", len(svc.listCalls)) + } + if got := svc.lastListParams().Filter; got != tt.wantFilt { + t.Errorf("Filter = %q, want %q", got, tt.wantFilt) + } + }) + } +} + // TestRequestList_RejectsInvalidRole kills REQ-05: dropping the --role // validation at cmd/request_list.go:82 would forward an arbitrary role string // to the API instead of failing locally. diff --git a/cmd/request_list.go b/cmd/request_list.go index 475c3d1..538150f 100644 --- a/cmd/request_list.go +++ b/cmd/request_list.go @@ -44,6 +44,21 @@ var ( validSorts = map[string]bool{"createdAt": true, "updatedAt": true, "calculatedRequestStartTime": true} ) +// combineFilters joins already-parenthesised conditions into a UAR OData filter. +// A lone condition is emitted bare: the API rejects the redundant outer wrap +// (`((requestState eq PENDING))` -> 400) that a single condition would get. +// Two or more are wrapped once, giving `((a) and (b))`. +func combineFilters(filters []string) string { + switch len(filters) { + case 0: + return "" + case 1: + return filters[0] + default: + return "(" + strings.Join(filters, " and ") + ")" + } +} + func runRequestList(cmd *cobra.Command, svc accessRequestService) error { ctx := cmd.Context() @@ -68,11 +83,9 @@ func runRequestList(cmd *cobra.Command, svc accessRequestService) error { if !validPriorities[v] { return fmt.Errorf("--priority must be one of High, Medium, Low (got %q)", v) } - filters = append(filters, fmt.Sprintf("(priority eq '%s')", v)) - } - if len(filters) > 0 { - params.Filter = "(" + strings.Join(filters, " and ") + ")" + filters = append(filters, fmt.Sprintf("(priority eq %s)", v)) } + params.Filter = combineFilters(filters) if v, _ := cmd.Flags().GetString("search"); v != "" { params.FreeText = v diff --git a/docs/mutation-ledger.md b/docs/mutation-ledger.md index a958d0c..bec4b72 100644 --- a/docs/mutation-ledger.md +++ b/docs/mutation-ledger.md @@ -55,10 +55,10 @@ premise does not hold). | REQ-01 | cmd/request finalize | `cmd/request_finalize.go:100` | `svc.FinalizeRequest(ctx, requestID, decision, reason)` → `svc.FinalizeRequest(ctx, requestID, "APPROVED", reason)` | CONFIRMED | test | `TestRequestReject_SendsRejectedDecision` + `TestRequestApprove_SendsApprovedDecision` | PR4 | done | | REQ-02 | cmd/request cancel | `cmd/request_cancel.go:60` | `svc.CancelRequest(ctx, requestID, reason)` → `svc.CancelRequest(ctx, "WRONG-ID", reason)` | CONFIRMED | test | `TestRequestCancel_PassesRequestID` | PR4 | done | | REQ-03 | cmd/request get | `cmd/request_get.go:53` | `svc.GetRequest(ctx, requestID)` → `svc.GetRequest(ctx, "WRONG-ID")` | CONFIRMED | test | `TestRequestGet_PassesRequestID` | PR4 | done | -| REQ-04 | cmd/request list | `cmd/request_list.go:93-98` | Swap the asc/desc branches: `order := "asc"` → `order := "desc"` and `order = "desc"` → `order = "asc"` | CONFIRMED | test | `TestRequestList_ParamsFromFlags` | PR4 | done | -| REQ-05 | cmd/request list | `cmd/request_list.go:82` | `if role != "CREATOR" && role != "APPROVER" {` → `if false {` | CONFIRMED | test | `TestRequestList_RejectsInvalidRole` | PR4 | done | -| REQ-06 | cmd/request list | `cmd/request_list.go:78` | `params.FreeText = v` → `params.FreeText = ""` | CONFIRMED | test | `TestRequestList_ParamsFromFlags` | PR4 | done | -| REQ-07 | cmd/request list | `cmd/request_list.go:58` | Delete `filters = append(filters, fmt.Sprintf("(requestState eq %s)", upper))` | CONFIRMED | test | `TestRequestList_ParamsFromFlags` | PR4 | done | +| REQ-04 | cmd/request list | `cmd/request_list.go:108-113` | Swap the asc/desc branches: `order := "asc"` → `order := "desc"` and `order = "desc"` → `order = "asc"` | CONFIRMED | test | `TestRequestList_ParamsFromFlags` | PR4 | done | +| REQ-05 | cmd/request list | `cmd/request_list.go:95` | `if role != "CREATOR" && role != "APPROVER" {` → `if false {` | CONFIRMED | test | `TestRequestList_RejectsInvalidRole` | PR4 | done | +| REQ-06 | cmd/request list | `cmd/request_list.go:91` | `params.FreeText = v` → `params.FreeText = ""` | CONFIRMED | test | `TestRequestList_ParamsFromFlags` | PR4 | done | +| REQ-07 | cmd/request list | `cmd/request_list.go:73` | Delete `filters = append(filters, fmt.Sprintf("(requestState eq %s)", upper))` | CONFIRMED | test | `TestRequestList_ParamsFromFlags` | PR4 | done | | REQ-08 | cmd/request submit | `cmd/request_submit.go:306` | `TargetCategory: "CLOUD_CONSOLE"` → `TargetCategory: "WRONG"` | CONFIRMED | test | `TestRunRequestSubmit_SubmitPayload` | PR4 | done | | REQ-09 | cmd/request submit | `cmd/request_submit.go:507` | `"workspaceId": ws.WorkspaceID,` → `"workspaceId": "",` | CONFIRMED | test | `TestRunRequestSubmit_SubmitPayload` | PR4 | done | | REQ-10 | cmd/request submit | `cmd/request_submit.go:511-512` | Swap the two values: `"timeFrom": f.timeTo,` / `"timeTo": f.timeFrom,` | CONFIRMED | test | `TestRunRequestSubmit_SubmitPayload` | PR4 | done |