fix: sort by for listing repos

api/repo.go

@@ -361,17 +361,18 @@ func GetRepos(c *gin.Context) {
 	// ensure per_page isn't above or below allowed values
 	perPage = util.MaxInt(1, util.MinInt(100, perPage))
 
+	// capture the sort_by query parameter if present
+	sortBy := util.QueryParameter(c, "sort_by", "name")
+
 	// capture the query parameters if present:
 	//
 	// * active
-	// * sort_by
 	filters := map[string]interface{}{
-		"active":  util.QueryParameter(c, "active", "true"),
-		"sort_by": util.QueryParameter(c, "sort_by", "name"),
+		"active": util.QueryParameter(c, "active", "true"),
 	}
 
 	// send API call to capture the list of repos for the user
-	r, t, err := database.FromContext(c).ListReposForUser(u, filters, page, perPage)
+	r, t, err := database.FromContext(c).ListReposForUser(u, sortBy, filters, page, perPage)
 	if err != nil {
 		retErr := fmt.Errorf("unable to get repos for user %s: %w", u.GetName(), err)
 
@@ -492,13 +493,14 @@ func GetOrgRepos(c *gin.Context) {
 	// ensure per_page isn't above or below allowed values
 	perPage = util.MaxInt(1, util.MinInt(100, perPage))
 
+	// capture the sort_by query parameter if present
+	sortBy := util.QueryParameter(c, "sort_by", "name")
+
 	// capture the query parameters if present:
 	//
 	// * active
-	// * sort_by
 	filters := map[string]interface{}{
-		"active":  util.QueryParameter(c, "active", "true"),
-		"sort_by": util.QueryParameter(c, "sort_by", "name"),
+		"active": util.QueryParameter(c, "active", "true"),
 	}
 
 	// See if the user is an org admin to bypass individual permission checks
@@ -512,7 +514,7 @@ func GetOrgRepos(c *gin.Context) {
 	}
 
 	// send API call to capture the list of repos for the org
-	r, t, err := database.FromContext(c).ListReposForOrg(o, filters, page, perPage)
+	r, t, err := database.FromContext(c).ListReposForOrg(o, sortBy, filters, page, perPage)
 	if err != nil {
 		retErr := fmt.Errorf("unable to get repos for org %s: %w", o, err)
 

api/scm.go

@@ -70,12 +70,7 @@ func SyncRepos(c *gin.Context) {
 		logger.Errorf("unable to get user %s access level for org %s", u.GetName(), o)
 	}
 
-	// capture the query parameters if present:
-	//
-	// * sort_by
-	filters := map[string]interface{}{
-		"sort_by": util.QueryParameter(c, "sort_by", "name"),
-	}
+	filters := map[string]interface{}{}
 	// Only show public repos to non-admins
 	if perm != "admin" {
 		filters["visibility"] = constants.VisibilityPublic
@@ -95,7 +90,7 @@ func SyncRepos(c *gin.Context) {
 	page := 0
 	// capture all repos belonging to a certain org in database
 	for orgRepos := int64(0); orgRepos < t; orgRepos += 100 {
-		r, _, err := database.FromContext(c).ListReposForOrg(o, filters, page, 100)
+		r, _, err := database.FromContext(c).ListReposForOrg(o, "name", filters, page, 100)
 		if err != nil {
 			retErr := fmt.Errorf("unable to get repo count for org %s: %w", o, err)
 

api/user.go

@@ -436,7 +436,7 @@ func GetUserSourceRepos(c *gin.Context) {
 
 		for page > 0 {
 			// send API call to capture the list of repos for the org
-			dbReposPart, _, err := database.FromContext(c).ListReposForOrg(org, filters, page, 100)
+			dbReposPart, _, err := database.FromContext(c).ListReposForOrg(org, "name", filters, page, 100)
 			if err != nil {
 				retErr := fmt.Errorf("unable to get repos for org %s: %w", org, err)
 

database/repo/list_org.go

@@ -14,7 +14,7 @@ import (
 // ListReposForOrg gets a list of repos by org name from the database.
 //
 // nolint: lll // ignore long line length due to variable names
-func (e *engine) ListReposForOrg(org string, filters map[string]interface{}, page, perPage int) ([]*library.Repo, int64, error) {
+func (e *engine) ListReposForOrg(org, sortBy string, filters map[string]interface{}, page, perPage int) ([]*library.Repo, int64, error) {
 	e.logger.WithFields(logrus.Fields{
 		"org": org,
 	}).Tracef("listing repos for org %s from the database", org)
@@ -38,7 +38,7 @@ func (e *engine) ListReposForOrg(org string, filters map[string]interface{}, pag
 	// calculate offset for pagination through results
 	offset := perPage * (page - 1)
 
-	switch filters["sort_by"] {
+	switch sortBy {
 	case "latest":
 		query := e.client.
 			Table(constants.TableBuild).

database/repo/list_org_test.go

@@ -7,13 +7,28 @@ package repo
 import (
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/DATA-DOG/go-sqlmock"
+	"github.com/go-vela/types/constants"
+	"github.com/go-vela/types/database"
 	"github.com/go-vela/types/library"
 )
 
 func TestRepo_Engine_ListReposForOrg(t *testing.T) {
 	// setup types
+	_buildOne := new(library.Build)
+	_buildOne.SetID(1)
+	_buildOne.SetRepoID(1)
+	_buildOne.SetNumber(1)
+	_buildOne.SetCreated(time.Now().UTC().Unix())
+
+	_buildTwo := new(library.Build)
+	_buildTwo.SetID(2)
+	_buildTwo.SetRepoID(2)
+	_buildTwo.SetNumber(1)
+	_buildTwo.SetCreated(time.Now().UTC().Unix())
+
 	_repoOne := testRepo()
 	_repoOne.SetID(1)
 	_repoOne.SetUserID(1)
@@ -37,21 +52,36 @@ func TestRepo_Engine_ListReposForOrg(t *testing.T) {
 	_postgres, _mock := testPostgres(t)
 	defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }()
 
-	// create expected result in mock
+	// create expected name count query result in mock
 	_rows := sqlmock.NewRows([]string{"count"}).AddRow(2)
 
-	// ensure the mock expects the query
+	// ensure the mock expects the name count query
 	_mock.ExpectQuery(`SELECT count(*) FROM "repos" WHERE org = $1`).WithArgs("foo").WillReturnRows(_rows)
 
-	// create expected result in mock
+	// create expected name query result in mock
 	_rows = sqlmock.NewRows(
 		[]string{"id", "user_id", "hash", "org", "name", "full_name", "link", "clone", "branch", "timeout", "counter", "visibility", "private", "trusted", "active", "allow_pull", "allow_push", "allow_deploy", "allow_tag", "allow_comment", "pipeline_type", "previous_name"}).
 		AddRow(1, 1, "baz", "foo", "bar", "foo/bar", "", "", "", 0, 0, "public", false, false, false, false, false, false, false, false, "yaml", nil).
 		AddRow(2, 1, "bar", "foo", "baz", "foo/baz", "", "", "", 0, 0, "public", false, false, false, false, false, false, false, false, "yaml", nil)
 
-	// ensure the mock expects the query
+	// ensure the mock expects the name query
 	_mock.ExpectQuery(`SELECT * FROM "repos" WHERE org = $1 ORDER BY name LIMIT 10`).WithArgs("foo").WillReturnRows(_rows)
 
+	// create expected latest count query result in mock
+	_rows = sqlmock.NewRows([]string{"count"}).AddRow(2)
+
+	// ensure the mock expects the latest count query
+	_mock.ExpectQuery(`SELECT count(*) FROM "repos" WHERE org = $1`).WithArgs("foo").WillReturnRows(_rows)
+
+	// create expected latest query result in mock
+	_rows = sqlmock.NewRows(
+		[]string{"id", "user_id", "hash", "org", "name", "full_name", "link", "clone", "branch", "timeout", "counter", "visibility", "private", "trusted", "active", "allow_pull", "allow_push", "allow_deploy", "allow_tag", "allow_comment", "pipeline_type", "previous_name"}).
+		AddRow(1, 1, "baz", "foo", "bar", "foo/bar", "", "", "", 0, 0, "public", false, false, false, false, false, false, false, false, "yaml", nil).
+		AddRow(2, 1, "bar", "foo", "baz", "foo/baz", "", "", "", 0, 0, "public", false, false, false, false, false, false, false, false, "yaml", nil)
+
+	// ensure the mock expects the latest query
+	_mock.ExpectQuery(`SELECT repos.* FROM "repos" LEFT JOIN (SELECT repos.id, MAX(builds.created) AS latest_build FROM "builds" INNER JOIN repos repos ON builds.repo_id = repos.id WHERE repos.org = $1 GROUP BY "repos"."id") t on repos.id = t.id ORDER BY latest_build DESC NULLS LAST LIMIT 10`).WithArgs("foo").WillReturnRows(_rows)
+
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
@@ -65,23 +95,55 @@ func TestRepo_Engine_ListReposForOrg(t *testing.T) {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}
 
+	err = _sqlite.client.AutoMigrate(&database.Build{})
+	if err != nil {
+		t.Errorf("unable to create build table for sqlite: %v", err)
+	}
+
+	err = _sqlite.client.Table(constants.TableBuild).Create(database.BuildFromLibrary(_buildOne).Crop()).Error
+	if err != nil {
+		t.Errorf("unable to create test build for sqlite: %v", err)
+	}
+
+	err = _sqlite.client.Table(constants.TableBuild).Create(database.BuildFromLibrary(_buildTwo).Crop()).Error
+	if err != nil {
+		t.Errorf("unable to create test build for sqlite: %v", err)
+	}
+
 	// setup tests
 	tests := []struct {
 		failure  bool
 		name     string
+		sort     string
 		database *engine
 		want     []*library.Repo
 	}{
 		{
 			failure:  false,
-			name:     "postgres",
+			name:     "postgres with name",
+			database: _postgres,
+			sort:     "name",
+			want:     []*library.Repo{_repoOne, _repoTwo},
+		},
+		{
+			failure:  false,
+			name:     "postgres with latest",
 			database: _postgres,
+			sort:     "latest",
+			want:     []*library.Repo{_repoOne, _repoTwo},
+		},
+		{
+			failure:  false,
+			name:     "sqlite with name",
+			database: _sqlite,
+			sort:     "name",
 			want:     []*library.Repo{_repoOne, _repoTwo},
 		},
 		{
 			failure:  false,
-			name:     "sqlite3",
+			name:     "sqlite with latest",
 			database: _sqlite,
+			sort:     "latest",
 			want:     []*library.Repo{_repoOne, _repoTwo},
 		},
 	}
@@ -91,7 +153,7 @@ func TestRepo_Engine_ListReposForOrg(t *testing.T) {
 	// run tests
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			got, _, err := test.database.ListReposForOrg("foo", filters, 1, 10)
+			got, _, err := test.database.ListReposForOrg("foo", test.sort, filters, 1, 10)
 
 			if test.failure {
 				if err == nil {

database/repo/list_user.go

@@ -14,7 +14,7 @@ import (
 // ListReposForUser gets a list of repos by user ID from the database.
 //
 // nolint: lll // ignore long line length due to variable names
-func (e *engine) ListReposForUser(u *library.User, filters map[string]interface{}, page, perPage int) ([]*library.Repo, int64, error) {
+func (e *engine) ListReposForUser(u *library.User, sortBy string, filters map[string]interface{}, page, perPage int) ([]*library.Repo, int64, error) {
 	e.logger.WithFields(logrus.Fields{
 		"user": u.GetName(),
 	}).Tracef("listing repos for user %s from the database", u.GetName())
@@ -38,7 +38,7 @@ func (e *engine) ListReposForUser(u *library.User, filters map[string]interface{
 	// calculate offset for pagination through results
 	offset := perPage * (page - 1)
 
-	switch filters["sort_by"] {
+	switch sortBy {
 	case "latest":
 		query := e.client.
 			Table(constants.TableBuild).

database/repo/list_user_test.go

@@ -7,13 +7,28 @@ package repo
 import (
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/DATA-DOG/go-sqlmock"
+	"github.com/go-vela/types/constants"
+	"github.com/go-vela/types/database"
 	"github.com/go-vela/types/library"
 )
 
 func TestRepo_Engine_ListReposForUser(t *testing.T) {
 	// setup types
+	_buildOne := new(library.Build)
+	_buildOne.SetID(1)
+	_buildOne.SetRepoID(1)
+	_buildOne.SetNumber(1)
+	_buildOne.SetCreated(time.Now().UTC().Unix())
+
+	_buildTwo := new(library.Build)
+	_buildTwo.SetID(2)
+	_buildTwo.SetRepoID(2)
+	_buildTwo.SetNumber(1)
+	_buildTwo.SetCreated(time.Now().UTC().Unix())
+
 	_repoOne := testRepo()
 	_repoOne.SetID(1)
 	_repoOne.SetUserID(1)
@@ -42,21 +57,36 @@ func TestRepo_Engine_ListReposForUser(t *testing.T) {
 	_postgres, _mock := testPostgres(t)
 	defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }()
 
-	// create expected result in mock
+	// create expected name count query result in mock
 	_rows := sqlmock.NewRows([]string{"count"}).AddRow(2)
 
-	// ensure the mock expects the query
+	// ensure the mock expects the name count query
 	_mock.ExpectQuery(`SELECT count(*) FROM "repos" WHERE user_id = $1`).WithArgs(1).WillReturnRows(_rows)
 
-	// create expected result in mock
+	// create expected name query result in mock
 	_rows = sqlmock.NewRows(
 		[]string{"id", "user_id", "hash", "org", "name", "full_name", "link", "clone", "branch", "timeout", "counter", "visibility", "private", "trusted", "active", "allow_pull", "allow_push", "allow_deploy", "allow_tag", "allow_comment", "pipeline_type", "previous_name"}).
 		AddRow(1, 1, "baz", "foo", "bar", "foo/bar", "", "", "", 0, 0, "public", false, false, false, false, false, false, false, false, "yaml", nil).
 		AddRow(2, 1, "baz", "bar", "foo", "bar/foo", "", "", "", 0, 0, "public", false, false, false, false, false, false, false, false, "yaml", nil)
 
-	// ensure the mock expects the query
+	// ensure the mock expects the name query
 	_mock.ExpectQuery(`SELECT * FROM "repos" WHERE user_id = $1 ORDER BY name LIMIT 10`).WithArgs(1).WillReturnRows(_rows)
 
+	// create expected latest count query result in mock
+	_rows = sqlmock.NewRows([]string{"count"}).AddRow(2)
+
+	// ensure the mock expects the latest count query
+	_mock.ExpectQuery(`SELECT count(*) FROM "repos" WHERE user_id = $1`).WithArgs(1).WillReturnRows(_rows)
+
+	// create expected latest query result in mock
+	_rows = sqlmock.NewRows(
+		[]string{"id", "user_id", "hash", "org", "name", "full_name", "link", "clone", "branch", "timeout", "counter", "visibility", "private", "trusted", "active", "allow_pull", "allow_push", "allow_deploy", "allow_tag", "allow_comment", "pipeline_type", "previous_name"}).
+		AddRow(1, 1, "baz", "foo", "bar", "foo/bar", "", "", "", 0, 0, "public", false, false, false, false, false, false, false, false, "yaml", nil).
+		AddRow(2, 1, "baz", "bar", "foo", "bar/foo", "", "", "", 0, 0, "public", false, false, false, false, false, false, false, false, "yaml", nil)
+
+	// ensure the mock expects the latest query
+	_mock.ExpectQuery(`SELECT repos.* FROM "repos" LEFT JOIN (SELECT repos.id, MAX(builds.created) AS latest_build FROM "builds" INNER JOIN repos repos ON builds.repo_id = repos.id WHERE repos.user_id = $1 GROUP BY "repos"."id") t on repos.id = t.id ORDER BY latest_build DESC NULLS LAST LIMIT 10`).WithArgs(1).WillReturnRows(_rows)
+
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
@@ -70,23 +100,55 @@ func TestRepo_Engine_ListReposForUser(t *testing.T) {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}
 
+	err = _sqlite.client.AutoMigrate(&database.Build{})
+	if err != nil {
+		t.Errorf("unable to create build table for sqlite: %v", err)
+	}
+
+	err = _sqlite.client.Table(constants.TableBuild).Create(database.BuildFromLibrary(_buildOne).Crop()).Error
+	if err != nil {
+		t.Errorf("unable to create test build for sqlite: %v", err)
+	}
+
+	err = _sqlite.client.Table(constants.TableBuild).Create(database.BuildFromLibrary(_buildTwo).Crop()).Error
+	if err != nil {
+		t.Errorf("unable to create test build for sqlite: %v", err)
+	}
+
 	// setup tests
 	tests := []struct {
 		failure  bool
 		name     string
+		sort     string
 		database *engine
 		want     []*library.Repo
 	}{
 		{
 			failure:  false,
-			name:     "postgres",
+			name:     "postgres with name",
+			database: _postgres,
+			sort:     "name",
+			want:     []*library.Repo{_repoOne, _repoTwo},
+		},
+		{
+			failure:  false,
+			name:     "postgres with latest",
 			database: _postgres,
+			sort:     "latest",
+			want:     []*library.Repo{_repoOne, _repoTwo},
+		},
+		{
+			failure:  false,
+			name:     "sqlite with name",
+			database: _sqlite,
+			sort:     "name",
 			want:     []*library.Repo{_repoOne, _repoTwo},
 		},
 		{
 			failure:  false,
-			name:     "sqlite3",
+			name:     "sqlite with latest",
 			database: _sqlite,
+			sort:     "latest",
 			want:     []*library.Repo{_repoOne, _repoTwo},
 		},
 	}
@@ -96,7 +158,7 @@ func TestRepo_Engine_ListReposForUser(t *testing.T) {
 	// run tests
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			got, _, err := test.database.ListReposForUser(_user, filters, 1, 10)
+			got, _, err := test.database.ListReposForUser(_user, test.sort, filters, 1, 10)
 
 			if test.failure {
 				if err == nil {