Skip to content

RHINENG-25944: add search to the tags list endpoint #2204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dominikvagner wants to merge 1 commit into
base: master
Choose a base branch
from
+57 −6
Open

RHINENG-25944: add search to the tags list endpoint #2204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/v3/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5656,6 +5656,14 @@
"type": "integer"
}
},
{
"name": "search",
"in": "query",
"description": "Find matching text",
"schema": {
"type": "string"
}
},
{
"name": "offset",
"in": "query",
Expand Down
17 changes: 12 additions & 5 deletions manager/controllers/systemtags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package controllers

import (
"app/base/database"
"app/base/utils"
"errors"
"net/http"

"app/base/database"
"app/base/utils"

"app/manager/middlewares"

"github.com/gin-gonic/gin"
Expand Down Expand Up @@ -48,16 +49,22 @@ var SystemTagsOpts = ListOpts{
},
StableSort: "tag",
DefaultSort: "tag",
SearchFields: []string{
"sq.tag->>'namespace'",
"sq.tag->>'key'",
"sq.tag->>'value'",
Comment on lines +52 to +55
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Avoid hard-coding the SQL alias in SearchFields to reduce coupling and future breakage risk

These SearchFields are tightly coupled to the sq alias (for example, "sq.tag->>'namespace'"). If the table/CTE alias changes, search will silently break. Either make these expressions independent of the outer alias (if supported), or centralize them in a shared constant/helper used by both the query builder and SearchFields so alias updates are done in one place.

Suggested implementation:

	SearchFields: []string{
		"tag->>'namespace'",
		"tag->>'key'",
		"tag->>'value'",
	},

This change assumes that the query builder / underlying SQL does not require an explicit table/CTE alias prefix for these expressions. If the current implementation does require the alias (e.g. because multiple tables expose a tag column), you should instead:

  1. Introduce a shared constant or helper (e.g. const systemTagAlias = "sq" or a function that formats search fields for a given alias) in this file or a shared package.
  2. Use that constant/helper both where the alias is defined in the SQL/CTE and here in SearchFields, so alias changes are centralized.

Comment on lines +52 to +55
Copy link
Copy Markdown
Contributor

@rverdile rverdile May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question for my understanding, how did you determine these 3 fields should be searched?

},
}

// @Summary Show me systems tags applicable to this application
// @Description Show me systems tags applicable to this application
// @ID listSystemTags
// @Security RhIdentity
// @Produce json
// @Param sort query string false "Sort field" Enums(tag, count)
// @Param limit query int fals "Limit for paging"
// @Param offset query int false "Offset for paging"
// @Param sort query string false "Sort field" Enums(tag, count)
// @Param limit query int false "Limit for paging"
// @Param search query string false "Find matching text"
// @Param offset query int false "Offset for paging"
// @Success 200 {object} SystemTagsResponse
// @Failure 400 {object} utils.ErrorResponse
// @Failure 500 {object} utils.ErrorResponse
Expand Down
38 changes: 37 additions & 1 deletion manager/controllers/systemtags_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package controllers

import (
"app/base/core"
"net/http"
"testing"

"app/base/core"

"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -76,3 +77,38 @@ func TestSystemTagsListBadRequestOnIdtSort(t *testing.T) {
w := CreateRequestRouterWithAccount("GET", "/", "", "?sort=id", nil, "", SystemTagListHandler, 1)
assert.Equal(t, 400, w.Code)
}

func TestSystemTagsListSearch(t *testing.T) {
core.SetupTest(t)

w := CreateRequestRouterWithAccount("GET", "/", "", "?search=k3", nil, "", SystemTagListHandler, 1)

var output SystemTagsResponse
CheckResponse(t, w, http.StatusOK, &output)

assert.Equal(t, 2, len(output.Data))
assert.Equal(t, 2, output.Meta.TotalItems)
Comment on lines +81 to +90
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Avoid relying on result ordering and tighten assertions to better express expectations

This test is tightly coupled to the current result order and exact per-index counts/values, so it may become brittle if query or default sorting changes while behavior remains correct. Either (a) assert that both expected tag combinations are present regardless of order (e.g., by sorting or mapping the response), or (b) treat sort order as part of the contract and assert on the sort field explicitly. Also consider asserting output.Meta.TotalItems relative to len(output.Data) to ensure pagination metadata matches the returned items.

Suggested implementation:

import (
	"fmt"
	"net/http"
	"testing"

	"app/base/core"

	"github.com/stretchr/testify/assert"
)
func TestSystemTagsListSearch(t *testing.T) {
	core.SetupTest(t)

	w := CreateRequestRouterWithAccount("GET", "/", "", "?search=k3", nil, "", SystemTagListHandler, 1)

	var output SystemTagsResponse
	CheckResponse(t, w, http.StatusOK, &output)

	// Expected tag combinations and counts, independent of result order.
	expected := map[string]int{
		"ns1:k3:val3": 1,
		"ns1:k3:val4": 3,
	}

	assert.Equal(t, len(expected), len(output.Data))
	assert.Equal(t, len(expected), output.Meta.TotalItems)
	assert.Equal(t, "k3", output.Meta.Search)

	seen := make(map[string]bool)
	for _, item := range output.Data {
		key := fmt.Sprintf("%s:%s:%s", item.Tag.Namespace, item.Tag.Key, item.Tag.Value)

		expectedCount, ok := expected[key]
		assert.True(t, ok, "unexpected tag combination in response: %s", key)
		assert.Equal(t, expectedCount, item.Count, "unexpected count for tag %s", key)

		seen[key] = true
	}

	// Ensure all expected tag combinations were returned.
	for key := range expected {
		assert.True(t, seen[key], "expected tag combination not found in response: %s", key)
	}
}

assert.Equal(t, "k3", output.Meta.Search)

assert.Equal(t, 1, output.Data[0].Count)
assert.Equal(t, "k3", output.Data[0].Tag.Key)
assert.Equal(t, "ns1", output.Data[0].Tag.Namespace)
assert.Equal(t, "val3", output.Data[0].Tag.Value)

assert.Equal(t, 3, output.Data[1].Count)
assert.Equal(t, "k3", output.Data[1].Tag.Key)
assert.Equal(t, "ns1", output.Data[1].Tag.Namespace)
assert.Equal(t, "val4", output.Data[1].Tag.Value)
}

func TestSystemTagsListSearchUnknown(t *testing.T) {
core.SetupTest(t)

w := CreateRequestRouterWithAccount("GET", "/", "", "?search=unknown", nil, "", SystemTagListHandler, 1)

var output SystemTagsResponse
CheckResponse(t, w, http.StatusOK, &output)

assert.Equal(t, 0, len(output.Data))
assert.Equal(t, "unknown", output.Meta.Search)
}
Loading