CloudMigrations: Make table sort case insensitive (#103898)

* case insensitive sort

* fix for all db types

* add comment

* add unit test

* add a TODO to fix later
This commit is contained in:
Michael Mandrus
2025-04-11 15:29:07 -04:00
committed by GitHub
parent 5efb620f1b
commit 652c374c4c
2 changed files with 79 additions and 1 deletions
@@ -12,6 +12,7 @@ import (
"github.com/grafana/grafana/pkg/services/secrets"
secretskv "github.com/grafana/grafana/pkg/services/secrets/kvstore"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/util"
)
@@ -434,7 +435,13 @@ func (ss *sqlStore) getSnapshotResources(ctx context.Context, snapshotUid string
if errorsOnly {
sess.Where("status = ?", cloudmigration.ItemStatusError)
}
return sess.OrderBy(fmt.Sprintf("%s %s", col, dir)).Find(&resources, &cloudmigration.CloudMigrationResource{
// TODO: It would be better if the query builder supported a case-insensitive flag for the .OrderBy() method
orderByClause := fmt.Sprintf("lower(%s) %s", col, dir)
if ss.db.GetDBType() == migrator.Postgres || // Postgres does not support lower() in ORDER BY -- sorts by case-insensitive by default
params.SortColumn == cloudmigration.SortColumnID { // Don't apply a string sort to a numeric column
orderByClause = fmt.Sprintf("%s %s", col, dir)
}
return sess.OrderBy(orderByClause).Find(&resources, &cloudmigration.CloudMigrationResource{
SnapshotUID: snapshotUid,
})
})
@@ -359,6 +359,77 @@ func Test_SnapshotResources(t *testing.T) {
})
}
func Test_SnapshotResourceCaseInsensitiveSorting(t *testing.T) {
t.Parallel()
_, s := setUpTest(t)
ctx := context.Background()
// Create test data with mixed case names
resources := []cloudmigration.CloudMigrationResource{
{UID: "1", SnapshotUID: "abc123", Name: "B", Type: cloudmigration.DashboardDataType, Status: cloudmigration.ItemStatusOK},
{UID: "2", SnapshotUID: "abc123", Name: "aa", Type: cloudmigration.AlertRuleType, Status: cloudmigration.ItemStatusOK},
{UID: "3", SnapshotUID: "abc123", Name: "ba", Type: cloudmigration.DashboardDataType, Status: cloudmigration.ItemStatusOK},
{UID: "4", SnapshotUID: "abc123", Name: "A", Type: cloudmigration.AlertRuleType, Status: cloudmigration.ItemStatusOK},
}
err := s.db.WithDbSession(ctx, func(sess *db.Session) error {
_, err := sess.Insert(resources)
return err
})
require.NoError(t, err)
// Test ascending sort
results, err := s.getSnapshotResources(ctx, "abc123", cloudmigration.SnapshotResultQueryParams{
ResultPage: 1,
ResultLimit: 100,
SortColumn: cloudmigration.SortColumnName,
SortOrder: cloudmigration.SortOrderAsc,
})
require.NoError(t, err)
assert.True(t, testNameComesBefore(t, results, "A", "aa"))
assert.True(t, testNameComesBefore(t, results, "B", "ba"))
assert.True(t, testNameComesBefore(t, results, "A", "B"))
assert.True(t, testNameComesBefore(t, results, "aa", "B"))
assert.True(t, testNameComesBefore(t, results, "aa", "ba"))
assert.True(t, testNameComesBefore(t, results, "A", "ba"))
assert.True(t, testNameComesBefore(t, results, "A", "B"))
// Test descending sort
results, err = s.getSnapshotResources(ctx, "abc123", cloudmigration.SnapshotResultQueryParams{
ResultPage: 1,
ResultLimit: 100,
SortColumn: cloudmigration.SortColumnName,
SortOrder: cloudmigration.SortOrderDesc,
})
require.NoError(t, err)
assert.True(t, testNameComesBefore(t, results, "ba", "B"))
assert.True(t, testNameComesBefore(t, results, "aa", "A"))
assert.True(t, testNameComesBefore(t, results, "ba", "B"))
assert.True(t, testNameComesBefore(t, results, "aa", "A"))
assert.True(t, testNameComesBefore(t, results, "ba", "B"))
assert.True(t, testNameComesBefore(t, results, "aa", "A"))
}
func testNameComesBefore(t *testing.T, input []cloudmigration.CloudMigrationResource, first string, second string) bool {
t.Helper()
foundFirst, foundSecond := false, false
for _, r := range input {
if r.Name == second {
foundSecond = true
continue
}
if r.Name == first {
if foundSecond {
return false
}
foundFirst = true
}
}
return foundFirst && foundSecond
}
func TestGetSnapshotList(t *testing.T) {
t.Parallel()