From 652c374c4c992a6bdfc7e5917bd7307b3bd0f6f5 Mon Sep 17 00:00:00 2001
From: Michael Mandrus <41969079+mmandrus@users.noreply.github.com>
Date: Fri, 11 Apr 2025 15:29:07 -0400
Subject: [PATCH] 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
---
 .../cloudmigrationimpl/xorm_store.go          |  9 ++-
 .../cloudmigrationimpl/xorm_store_test.go     | 71 +++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/pkg/services/cloudmigration/cloudmigrationimpl/xorm_store.go b/pkg/services/cloudmigration/cloudmigrationimpl/xorm_store.go
index a29c77f7a37..d2cf210d919 100644
--- a/pkg/services/cloudmigration/cloudmigrationimpl/xorm_store.go
+++ b/pkg/services/cloudmigration/cloudmigrationimpl/xorm_store.go
@@ -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,
 		})
 	})
diff --git a/pkg/services/cloudmigration/cloudmigrationimpl/xorm_store_test.go b/pkg/services/cloudmigration/cloudmigrationimpl/xorm_store_test.go
index 530c489d19b..b64dd11dc85 100644
--- a/pkg/services/cloudmigration/cloudmigrationimpl/xorm_store_test.go
+++ b/pkg/services/cloudmigration/cloudmigrationimpl/xorm_store_test.go
@@ -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()