[GH-30056] Refactor SELECT statements in compliance_store.go (#30124)

* Change queries to use builder

* extract to tableSelectQuery

* Apply suggestions from code review

* update tests to check errors, lengths

* linting

* leverage s.toReserveCase

---------

Co-authored-by: Jesse Hallam <jesse@thehallams.ca>
Co-authored-by: Jesse Hallam <jesse.hallam@gmail.com>
Co-authored-by: Mattermost Build <build@mattermost.com>

server/channels/store/sqlstore/compliance_store.go

@@ -18,10 +18,33 @@ import (
 
 type SqlComplianceStore struct {
 	*SqlStore
+
+	tableSelectQuery sq.SelectBuilder
 }
 
 func newSqlComplianceStore(sqlStore *SqlStore) store.ComplianceStore {
-	return &SqlComplianceStore{sqlStore}
+	s := SqlComplianceStore{
+		SqlStore: sqlStore,
+	}
+
+	s.tableSelectQuery = s.
+		getQueryBuilder().
+		Select(
+			"Id",
+			"CreateAt",
+			"UserId",
+			"Status",
+			"Count",
+			s.toReserveCase("desc"),
+			"Type",
+			"StartAt",
+			"EndAt",
+			"Keywords",
+			"Emails",
+		).
+		From("Compliances")
+
+	return &s
 }
 
 func (s SqlComplianceStore) Save(compliance *model.Compliance) (*model.Compliance, error) {
@@ -83,17 +106,24 @@ func (s SqlComplianceStore) Update(compliance *model.Compliance) (*model.Complia
 }
 
 func (s SqlComplianceStore) GetAll(offset, limit int) (model.Compliances, error) {
-	query := "SELECT * FROM Compliances ORDER BY CreateAt DESC LIMIT ? OFFSET ?"
-	compliances := model.Compliances{}
-	if err := s.GetReplica().Select(&compliances, query, limit, offset); err != nil {
+	query := s.tableSelectQuery.
+		OrderBy("CreateAt DESC").
+		Limit(uint64(limit)).
+		Offset(uint64(offset))
+
+	var compliances model.Compliances
+	if err := s.GetReplica().SelectBuilder(&compliances, query); err != nil {
 		return nil, errors.Wrap(err, "failed to find all Compliances")
 	}
+
 	return compliances, nil
 }
 
 func (s SqlComplianceStore) Get(id string) (*model.Compliance, error) {
+	query := s.tableSelectQuery.Where(sq.Eq{"Id": id})
+
 	var compliance model.Compliance
-	if err := s.GetReplica().Get(&compliance, `SELECT * FROM Compliances WHERE Id = ?`, id); err != nil {
+	if err := s.GetReplica().GetBuilder(&compliance, query); err != nil {
 		if err == sql.ErrNoRows {
 			return nil, store.NewErrNotFound("Compliances", id)
 		}

server/channels/store/storetest/compliance_store.go

@@ -17,19 +17,19 @@ import (
 )
 
 func cleanupStoreState(t *testing.T, rctx request.CTX, ss store.Store) {
-	//remove existing users
+	// remove existing users
 	allUsers, err := ss.User().GetAll()
 	require.NoError(t, err, "error cleaning all test users", err)
 	for _, u := range allUsers {
 		err = ss.User().PermanentDelete(rctx, u.Id)
 		require.NoError(t, err, "failed cleaning up test user %s", u.Username)
 
-		//remove all posts by this user
+		// remove all posts by this user
 		nErr := ss.Post().PermanentDeleteByUser(rctx, u.Id)
 		require.NoError(t, nErr, "failed cleaning all posts of test user %s", u.Username)
 	}
 
-	//remove existing channels
+	// remove existing channels
 	allChannels, nErr := ss.Channel().GetAllChannels(0, 100000, store.ChannelSearchOpts{IncludeDeleted: true})
 	require.NoError(t, nErr, "error cleaning all test channels", nErr)
 	for _, channel := range allChannels {
@@ -37,7 +37,7 @@ func cleanupStoreState(t *testing.T, rctx request.CTX, ss store.Store) {
 		require.NoError(t, nErr, "failed cleaning up test channel %s", channel.Id)
 	}
 
-	//remove existing teams
+	// remove existing teams
 	allTeams, nErr := ss.Team().GetAll()
 	require.NoError(t, nErr, "error cleaning all test teams", nErr)
 	for _, team := range allTeams {
@@ -72,8 +72,10 @@ func testComplianceStore(t *testing.T, rctx request.CTX, ss store.Store) {
 	require.NoError(t, err)
 	time.Sleep(100 * time.Millisecond)
 
-	compliances, _ := ss.Compliance().GetAll(0, 1000)
+	compliances, err := ss.Compliance().GetAll(0, 1000)
+	require.NoError(t, err)
 
+	require.Len(t, compliances, 2)
 	require.Equal(t, model.ComplianceStatusRunning, compliances[0].Status)
 	require.Equal(t, compliance2.Id, compliances[0].Id)
 
@@ -81,20 +83,26 @@ func testComplianceStore(t *testing.T, rctx request.CTX, ss store.Store) {
 	_, err = ss.Compliance().Update(compliance2)
 	require.NoError(t, err)
 
-	compliances, _ = ss.Compliance().GetAll(0, 1000)
+	compliances, err = ss.Compliance().GetAll(0, 1000)
+	require.NoError(t, err)
 
+	require.Len(t, compliances, 2)
 	require.Equal(t, model.ComplianceStatusFailed, compliances[0].Status)
 	require.Equal(t, compliance2.Id, compliances[0].Id)
 
-	compliances, _ = ss.Compliance().GetAll(0, 1)
+	compliances, err = ss.Compliance().GetAll(0, 1)
+	require.NoError(t, err)
 
 	require.Len(t, compliances, 1)
 
-	compliances, _ = ss.Compliance().GetAll(1, 1)
+	compliances, err = ss.Compliance().GetAll(1, 1)
+	require.NoError(t, err)
 
 	require.Len(t, compliances, 1)
 
-	rc2, _ := ss.Compliance().Get(compliance2.Id)
+	rc2, err := ss.Compliance().Get(compliance2.Id)
+	require.NoError(t, err)
+
 	require.Equal(t, compliance2.Status, rc2.Status)
 }
 
@@ -837,7 +845,7 @@ func testEditExportMessage(t *testing.T, rctx request.CTX, ss store.Store) {
 	post1, err = ss.Post().Save(rctx, post1)
 	require.NoError(t, err)
 
-	//user 1 edits the previous post
+	// user 1 edits the previous post
 	post1e := post1.Clone()
 	post1e.Message = "edit " + post1.Message
 
@@ -948,7 +956,7 @@ func testEditAfterExportMessage(t *testing.T, rctx request.CTX, ss store.Store)
 	assert.Equal(t, user1.Username, *v.Username)
 
 	postEditTime := post1.UpdateAt + 1
-	//user 1 edits the previous post
+	// user 1 edits the previous post
 	post1e := post1.Clone()
 	post1e.EditAt = postEditTime
 	post1e.Message = "edit " + post1.Message
@@ -1040,7 +1048,7 @@ func testDeleteExportMessage(t *testing.T, rctx request.CTX, ss store.Store) {
 	post1, err = ss.Post().Save(rctx, post1)
 	require.NoError(t, err)
 
-	//user 1 deletes the previous post
+	// user 1 deletes the previous post
 	postDeleteTime := post1.UpdateAt + 1
 	err = ss.Post().Delete(rctx, post1.Id, postDeleteTime, user1.Id)
 	require.NoError(t, err)
@@ -1143,7 +1151,7 @@ func testDeleteAfterExportMessage(t *testing.T, rctx request.CTX, ss store.Store
 	assert.Equal(t, user1.Email, *v.UserEmail)
 	assert.Equal(t, user1.Username, *v.Username)
 
-	//user 1 deletes the previous post
+	// user 1 deletes the previous post
 	postDeleteTime := post1.UpdateAt + 1
 	err = ss.Post().Delete(rctx, post1.Id, postDeleteTime, user1.Id)
 	require.NoError(t, err)