diff --git a/pkg/registry/apis/dashboard/legacysearcher/search_client.go b/pkg/registry/apis/dashboard/legacysearcher/search_client.go index fd5e2283b27..be67dcddb9c 100644 --- a/pkg/registry/apis/dashboard/legacysearcher/search_client.go +++ b/pkg/registry/apis/dashboard/legacysearcher/search_client.go @@ -34,11 +34,33 @@ func NewDashboardSearchClient(dashboardStore dashboards.Store, sorter sort.Servi } var sortByMapping = map[string]string{ - unisearch.DASHBOARD_VIEWS_LAST_30_DAYS: "viewed-recently-", - unisearch.DASHBOARD_VIEWS_TOTAL: "viewed-", - unisearch.DASHBOARD_ERRORS_LAST_30_DAYS: "errors-recently-", - unisearch.DASHBOARD_ERRORS_TOTAL: "errors-", - "title": "alpha-", + unisearch.DASHBOARD_VIEWS_LAST_30_DAYS: "viewed-recently", + unisearch.DASHBOARD_VIEWS_TOTAL: "viewed", + unisearch.DASHBOARD_ERRORS_LAST_30_DAYS: "errors-recently", + unisearch.DASHBOARD_ERRORS_TOTAL: "errors", + "title": "alpha", +} + +func ParseSortName(sortName string) (string, bool, error) { + if sortName == "" { + return "", false, nil + } + + isDesc := strings.HasSuffix(sortName, "-desc") + isAsc := strings.HasSuffix(sortName, "-asc") + // default to desc if no suffix is provided + if !isDesc && !isAsc { + isDesc = true + } + + prefix := strings.TrimSuffix(strings.TrimSuffix(sortName, "-desc"), "-asc") + for key, mappedPrefix := range sortByMapping { + if prefix == mappedPrefix { + return key, isDesc, nil + } + } + + return "", false, fmt.Errorf("no matching sort field found for: %s", sortName) } // nolint:gocyclo @@ -99,9 +121,9 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resource.Resour sorterName := sortByMapping[sortByField] if sort.Desc { - sorterName += "desc" + sorterName += "-desc" } else { - sorterName += "asc" + sorterName += "-asc" } if sorter, ok := c.sorter.GetSortOption(sorterName); ok { @@ -207,22 +229,27 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resource.Resour } } searchFields := resource.StandardSearchFields() + columns := []*resource.ResourceTableColumnDefinition{ + searchFields.Field(resource.SEARCH_FIELD_TITLE), + searchFields.Field(resource.SEARCH_FIELD_FOLDER), + searchFields.Field(resource.SEARCH_FIELD_TAGS), + { + Name: unisearch.DASHBOARD_LEGACY_ID, + Type: resource.ResourceTableColumnDefinition_INT64, + Description: "Deprecated legacy id of the dashboard", + }, + } + + if sortByField != "" { + columns = append(columns, &resource.ResourceTableColumnDefinition{ + Name: sortByField, + Type: resource.ResourceTableColumnDefinition_INT64, + }) + } + list := &resource.ResourceSearchResponse{ Results: &resource.ResourceTable{ - Columns: []*resource.ResourceTableColumnDefinition{ - searchFields.Field(resource.SEARCH_FIELD_TITLE), - searchFields.Field(resource.SEARCH_FIELD_FOLDER), - searchFields.Field(resource.SEARCH_FIELD_TAGS), - { - Name: unisearch.DASHBOARD_LEGACY_ID, - Type: resource.ResourceTableColumnDefinition_INT64, - Description: "Deprecated legacy id of the dashboard", - }, - { - Name: sortByField, - Type: resource.ResourceTableColumnDefinition_INT64, - }, - }, + Columns: columns, }, } @@ -249,11 +276,22 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resource.Resour } for _, dashboard := range dashes { + cells := [][]byte{ + []byte(dashboard.Title), + []byte(dashboard.FolderUID), + []byte("[]"), // no tags retrieved for provisioned dashboards + []byte(strconv.FormatInt(dashboard.ID, 10)), + } + + if sortByField != "" { + cells = append(cells, []byte("0")) + } + list.Results.Rows = append(list.Results.Rows, &resource.ResourceTableRow{ Key: getResourceKey(&dashboards.DashboardSearchProjection{ UID: dashboard.UID, }, req.Options.Key.Namespace), - Cells: [][]byte{[]byte(dashboard.Title), []byte(dashboard.FolderUID), []byte(strconv.FormatInt(dashboard.ID, 10)), {}, {}}, + Cells: cells, }) } @@ -275,9 +313,20 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resource.Resour return nil, err } + cells := [][]byte{ + []byte(dashboard.Title), + []byte(dashboard.FolderUID), + tags, + []byte(strconv.FormatInt(dashboard.ID, 10)), + } + + if sortByField != "" { + cells = append(cells, []byte(strconv.FormatInt(dashboard.SortMeta, 10))) + } + list.Results.Rows = append(list.Results.Rows, &resource.ResourceTableRow{ Key: getResourceKey(dashboard, req.Options.Key.Namespace), - Cells: [][]byte{[]byte(dashboard.Title), []byte(dashboard.FolderUID), tags, []byte(strconv.FormatInt(dashboard.ID, 10)), []byte(strconv.FormatInt(dashboard.SortMeta, 10))}, + Cells: cells, }) } diff --git a/pkg/registry/apis/dashboard/legacysearcher/search_client_test.go b/pkg/registry/apis/dashboard/legacysearcher/search_client_test.go index a4eecae9058..700fad7a2ce 100644 --- a/pkg/registry/apis/dashboard/legacysearcher/search_client_test.go +++ b/pkg/registry/apis/dashboard/legacysearcher/search_client_test.go @@ -77,10 +77,6 @@ func TestDashboardSearchClient_Search(t *testing.T) { Type: resource.ResourceTableColumnDefinition_INT64, Description: "Deprecated legacy id of the dashboard", }, - { - Name: "", // sort by should be empty if title is what we sorted by - Type: resource.ResourceTableColumnDefinition_INT64, - }, }, Rows: []*resource.ResourceTableRow{ { @@ -94,7 +90,6 @@ func TestDashboardSearchClient_Search(t *testing.T) { []byte("folder1"), tags, []byte("1"), - []byte(strconv.FormatInt(0, 10)), }, }, { @@ -108,7 +103,6 @@ func TestDashboardSearchClient_Search(t *testing.T) { []byte("folder2"), emptyTags, []byte("2"), - []byte(strconv.FormatInt(0, 10)), }, }, }, @@ -501,4 +495,131 @@ func TestDashboardSearchClient_Search(t *testing.T) { } require.Equal(t, resp.TotalHits, int64(1)) }) + + t.Run("Should set empty sort field when sorting by title", func(t *testing.T) { + mockStore.On("FindDashboards", mock.Anything, &dashboards.FindPersistedDashboardsQuery{ + SignedInUser: user, + Sort: sort.SortAlphaAsc, + Type: "dash-db", + }).Return([]dashboards.DashboardSearchProjection{ + {ID: 1, UID: "uid", Title: "Test Dashboard", FolderUID: "folder1"}, + }, nil).Once() + + req := &resource.ResourceSearchRequest{ + Options: &resource.ListOptions{ + Key: dashboardKey, + }, + SortBy: []*resource.ResourceSearchRequest_Sort{ + { + Field: resource.SEARCH_FIELD_TITLE, + }, + }, + } + resp, err := client.Search(ctx, req) + require.NoError(t, err) + require.NotNil(t, resp) + require.Len(t, resp.Results.Columns, 4) + mockStore.AssertExpectations(t) + }) + + t.Run("Should set correct sort field when sorting by views", func(t *testing.T) { + mockStore.On("FindDashboards", mock.Anything, mock.Anything).Return([]dashboards.DashboardSearchProjection{ + {ID: 1, UID: "uid", Title: "Test Dashboard", FolderUID: "folder1", SortMeta: 100}, + }, nil).Once() + + req := &resource.ResourceSearchRequest{ + Options: &resource.ListOptions{ + Key: dashboardKey, + }, + SortBy: []*resource.ResourceSearchRequest_Sort{ + { + Field: resource.SEARCH_FIELD_PREFIX + unisearch.DASHBOARD_VIEWS_TOTAL, + }, + }, + } + resp, err := client.Search(ctx, req) + require.NoError(t, err) + require.NotNil(t, resp) + + require.Len(t, resp.Results.Columns, 5) + i := len(resp.Results.Columns) - 1 + require.Equal(t, "views_total", resp.Results.Columns[i].Name) + require.Equal(t, []byte(strconv.FormatInt(100, 10)), resp.Results.Rows[0].Cells[i]) // views should be set to 100 + mockStore.AssertExpectations(t) + }) +} + +func TestParseSortName(t *testing.T) { + tests := []struct { + name string + sortName string + wantField string + wantDesc bool + wantErr bool + }{ + { + name: "empty sort name", + sortName: "", + wantField: "", + wantDesc: false, + wantErr: false, + }, + { + name: "viewed-recently with desc suffix", + sortName: "viewed-recently-desc", + wantField: unisearch.DASHBOARD_VIEWS_LAST_30_DAYS, + wantDesc: true, + wantErr: false, + }, + { + name: "defaults to desc", + sortName: "viewed", + wantField: unisearch.DASHBOARD_VIEWS_TOTAL, + wantDesc: true, + wantErr: false, + }, + { + name: "errors-recentlyy with asc suffix", + sortName: "errors-recently-asc", + wantField: unisearch.DASHBOARD_ERRORS_LAST_30_DAYS, + wantDesc: false, + wantErr: false, + }, + { + name: "errors - defaults to desc too", + sortName: "errors", + wantField: unisearch.DASHBOARD_ERRORS_TOTAL, + wantDesc: true, + wantErr: false, + }, + { + name: "alpha sort with asc suffix", + sortName: "alpha-asc", + wantField: "title", + wantDesc: false, + wantErr: false, + }, + { + name: "invalid sort name", + sortName: "invalid-sort-desc", + wantField: "", + wantDesc: false, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + field, isDesc, err := ParseSortName(tt.sortName) + + if tt.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + require.Equal(t, tt.wantField, field) + require.Equal(t, tt.wantDesc, isDesc) + }) + } } diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index 1b895e8492a..53164cda375 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -11,6 +11,7 @@ import ( "time" "github.com/google/uuid" + "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacysearcher" "github.com/grafana/grafana/pkg/util/retryer" "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel" @@ -1299,6 +1300,14 @@ func (dr *DashboardServiceImpl) FindDashboards(ctx context.Context, query *dashb Tags: hit.Tags, } + if hit.Field != nil && query.Sort.Name != "" { + fieldName, _, err := legacysearcher.ParseSortName(query.Sort.Name) + if err != nil { + return nil, err + } + result.SortMeta = hit.Field.GetNestedInt64(fieldName) + } + if hit.Resource == folderv0alpha1.RESOURCE { result.IsFolder = true } @@ -1844,12 +1853,12 @@ func (dr *DashboardServiceImpl) searchDashboardsThroughK8sRaw(ctx context.Contex request.Federated = []*resource.ResourceKey{federate} } - // technically, there exists the ability to register multiple ways of sorting using the legacy database - // see RegisterSortOption in pkg/services/search/sorting.go - // however, it doesn't look like we are taking advantage of that. And since by default the legacy - // sql will sort by title ascending, we only really need to handle the "alpha-desc" case - if query.Sort.Name == "alpha-desc" { - request.SortBy = append(request.SortBy, &resource.ResourceSearchRequest_Sort{Field: resource.SEARCH_FIELD_TITLE, Desc: true}) + if query.Sort.Name != "" { + sortName, isDesc, err := legacysearcher.ParseSortName(query.Sort.Name) + if err != nil { + return dashboardv0alpha1.SearchResults{}, err + } + request.SortBy = append(request.SortBy, &resource.ResourceSearchRequest_Sort{Field: sortName, Desc: isDesc}) } res, err := dr.k8sclient.Search(ctx, query.OrgId, request) diff --git a/pkg/services/dashboards/service/dashboard_service_test.go b/pkg/services/dashboards/service/dashboard_service_test.go index 5e495089d1d..2223eabb9d7 100644 --- a/pkg/services/dashboards/service/dashboard_service_test.go +++ b/pkg/services/dashboards/service/dashboard_service_test.go @@ -32,6 +32,7 @@ import ( "github.com/grafana/grafana/pkg/services/publicdashboards" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/search/model" + "github.com/grafana/grafana/pkg/services/search/sort" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/unified/resource" @@ -2165,9 +2166,15 @@ func TestSearchDashboardsThroughK8sRaw(t *testing.T) { service := &DashboardServiceImpl{k8sclient: k8sCliMock} query := &dashboards.FindPersistedDashboardsQuery{ OrgId: 1, + Sort: sort.SortAlphaAsc, } k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default") - k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.Anything).Return(&resource.ResourceSearchResponse{ + k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool { + return len(req.SortBy) == 1 && + // should be converted to "title" due to ParseSortName + req.SortBy[0].Field == "title" && + !req.SortBy[0].Desc + })).Return(&resource.ResourceSearchResponse{ Results: &resource.ResourceTable{ Columns: []*resource.ResourceTableColumnDefinition{ {