Fix incorrect oldest sort in project list (go-gitea#25806) (go-gitea#…

…25835) Backport go-gitea#25806 by @yp05327 sort type `oldest` should be `Asc`. Added a test for this. I see we have `SearchOrderBy` in db model, but we are using many different ways to define the sort type. ~Maybe we can improve this later.~ ↑ Improved in this PR Co-authored-by: yp05327 <[email protected]>
hazycora · Jul 12, 2023 · 052e65e · 052e65e
1 parent c1a10be
commit 052e65e
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 15 deletions.
diff --git a/models/fixtures/project.yml b/models/fixtures/project.yml
@@ -7,6 +7,8 @@
   creator_id: 2
   board_type: 1
   type: 2
+  created_unix: 1688973030
+  updated_unix: 1688973030
 
 -
   id: 2
@@ -17,6 +19,8 @@
   creator_id: 3
   board_type: 1
   type: 2
+  created_unix: 1688973010
+  updated_unix: 1688973010
 
 -
   id: 3
@@ -27,6 +31,8 @@
   creator_id: 5
   board_type: 1
   type: 2
+  created_unix: 1688973020
+  updated_unix: 1688973020
 
 -
   id: 4
@@ -37,3 +43,5 @@
   creator_id: 2
   board_type: 1
   type: 2
+  created_unix: 1688973000
+  updated_unix: 1688973000
diff --git a/models/project/project.go b/models/project/project.go
@@ -196,7 +196,7 @@ type SearchOptions struct {
 	RepoID   int64
 	Page     int
 	IsClosed util.OptionalBool
-	SortType string
+	OrderBy  db.SearchOrderBy
 	Type     Type
 }
 
@@ -226,26 +226,28 @@ func CountProjects(ctx context.Context, opts SearchOptions) (int64, error) {
 	return db.GetEngine(ctx).Where(opts.toConds()).Count(new(Project))
 }
 
+func GetSearchOrderByBySortType(sortType string) db.SearchOrderBy {
+	switch sortType {
+	case "oldest":
+		return db.SearchOrderByOldest
+	case "recentupdate":
+		return db.SearchOrderByRecentUpdated
+	case "leastupdate":
+		return db.SearchOrderByLeastUpdated
+	default:
+		return db.SearchOrderByNewest
+	}
+}
+
 // FindProjects returns a list of all projects that have been created in the repository
 func FindProjects(ctx context.Context, opts SearchOptions) ([]*Project, int64, error) {
-	e := db.GetEngine(ctx).Where(opts.toConds())
+	e := db.GetEngine(ctx).Where(opts.toConds()).OrderBy(opts.OrderBy.String())
 	projects := make([]*Project, 0, setting.UI.IssuePagingNum)
 
 	if opts.Page > 0 {
 		e = e.Limit(setting.UI.IssuePagingNum, (opts.Page-1)*setting.UI.IssuePagingNum)
 	}
 
-	switch opts.SortType {
-	case "oldest":
-		e.Desc("created_unix")
-	case "recentupdate":
-		e.Desc("updated_unix")
-	case "leastupdate":
-		e.Asc("updated_unix")
-	default:
-		e.Asc("created_unix")
-	}
-
 	count, err := e.FindAndCount(&projects)
 	return projects, count, err
 }

diff --git a/models/project/project_test.go b/models/project/project_test.go
@@ -82,3 +82,42 @@ func TestProject(t *testing.T) {
 
 	assert.True(t, projectFromDB.IsClosed)
 }
+
+func TestProjectsSort(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	tests := []struct {
+		sortType string
+		wants    []int64
+	}{
+		{
+			sortType: "default",
+			wants:    []int64{1, 3, 2, 4},
+		},
+		{
+			sortType: "oldest",
+			wants:    []int64{4, 2, 3, 1},
+		},
+		{
+			sortType: "recentupdate",
+			wants:    []int64{1, 3, 2, 4},
+		},
+		{
+			sortType: "leastupdate",
+			wants:    []int64{4, 2, 3, 1},
+		},
+	}
+
+	for _, tt := range tests {
+		projects, count, err := FindProjects(db.DefaultContext, SearchOptions{
+			OrderBy: GetSearchOrderByBySortType(tt.sortType),
+		})
+		assert.NoError(t, err)
+		assert.EqualValues(t, int64(4), count)
+		if assert.Len(t, projects, 4) {
+			for i := range projects {
+				assert.EqualValues(t, tt.wants[i], projects[i].ID)
+			}
+		}
+	}
+}
diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go
@@ -61,7 +61,7 @@ func Projects(ctx *context.Context) {
 		OwnerID:  ctx.ContextUser.ID,
 		Page:     page,
 		IsClosed: util.OptionalBoolOf(isShowClosed),
-		SortType: sortType,
+		OrderBy:  project_model.GetSearchOrderByBySortType(sortType),
 		Type:     projectType,
 	})
 	if err != nil {

diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go
@@ -74,7 +74,7 @@ func Projects(ctx *context.Context) {
 		RepoID:   repo.ID,
 		Page:     page,
 		IsClosed: util.OptionalBoolOf(isShowClosed),
-		SortType: sortType,
+		OrderBy:  project_model.GetSearchOrderByBySortType(sortType),
 		Type:     project_model.TypeRepository,
 	})
 	if err != nil {