try simplify logic

Author: a1012112796

models/user_pinnedrepo.go

@@ -56,10 +56,10 @@ func (u *User) IsPinnedRepoExist(repoID int64) (isExist bool, err error) {
 	return x.Exist(&UserPinnedRepo{UID: u.ID, RepoID: repoID})
 }
 
-// GetPinnedRepos get pinned repos
-func (u *User) GetPinnedRepos(actor *User, excludeRepoIDs []int64, loadAttributes bool) (repos RepositoryList, err error) {
+// GetPinnedRepoIDs get repos id
+func (u *User) GetPinnedRepoIDs(actor *User) (results []int64, err error) {
 	var cond = builder.NewCond()
-	repos = make(RepositoryList, 0, 10)
+	results = make([]int64, 0, 10)
 
 	if actor == nil {
 		if u.IsOrganization() && u.Visibility != structs.VisibleTypePublic {
@@ -77,19 +77,6 @@ func (u *User) GetPinnedRepos(actor *User, excludeRepoIDs []int64, loadAttribute
 
 	cond = cond.And(builder.In("id", idBuilder))
 
-	if len(excludeRepoIDs) > 0 {
-		cond = cond.And(builder.NotIn("id", excludeRepoIDs))
-	}
-
-	if err = x.Where(cond).Find(&repos); err != nil {
-		return nil, err
-	}
-
-	if loadAttributes {
-		if err = repos.LoadAttributes(); err != nil {
-			return nil, err
-		}
-	}
-
+	err = x.Table("repository").Cols("id").Where(cond).Find(&results)
 	return
 }

models/user_pinnedrepo_test.go

@@ -54,15 +54,11 @@ func TestGetPinnedRepos(t *testing.T) {
 
 	user2 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
 
-	repos, err := user2.GetPinnedRepos(user2, nil, true)
+	repoIDs, err := user2.GetPinnedRepoIDs(user2)
 	assert.NoError(t, err)
-	assert.Equal(t, 3, len(repos))
+	assert.Equal(t, []int64{1, 4, 16}, repoIDs)
 
-	repos, err = user2.GetPinnedRepos(user2, []int64{1}, true)
+	repoIDs, err = user2.GetPinnedRepoIDs(nil)
 	assert.NoError(t, err)
-	assert.Equal(t, 2, len(repos))
-
-	repos, err = user2.GetPinnedRepos(nil, []int64{1}, true)
-	assert.NoError(t, err)
-	assert.Equal(t, 1, len(repos))
+	assert.Equal(t, []int64{1, 4}, repoIDs)
 }

routers/org/home.go

@@ -120,17 +120,46 @@ func Home(ctx *context.Context) {
 	}
 
 	pinnedRepos := make([]*models.Repository, 0, 10)
-	has := false
-	for _, repo := range repos {
-		if has, err = org.IsPinnedRepoExist(repo.ID); err != nil {
-			ctx.ServerError("IsPinnedRepoExist", err)
-			return
+	pinnedRepoIDs, err := ctx.Org.Organization.GetPinnedRepoIDs(ctx.User)
+	if err != nil {
+		ctx.ServerError("GetPinnedRepos", err)
+		return
+	}
+
+	pinnedRepos2 := make(models.RepositoryList, 0, 5)
+	for _, pinnedRepoID := range pinnedRepoIDs {
+		has := false
+		for _, repo := range repos {
+			if repo.ID == pinnedRepoID {
+				has = true
+				repo.IsPinned = true
+				pinnedRepos = append(pinnedRepos, repo)
+				break
+			}
 		}
 
-		if has {
-			pinnedRepos = append(pinnedRepos, repo)
-			repo.IsPinned = true
+		if !has {
+			repo, err := models.GetRepositoryByID(pinnedRepoID)
+			if err != nil {
+				if !models.IsErrRepoNotExist(err) {
+					ctx.ServerError("GetRepositoryByID", err)
+				}
+				return
+			}
+
+			if repo != nil {
+				repo.IsPinned = true
+				pinnedRepos2 = append(pinnedRepos2, repo)
+			}
+		}
+	}
+
+	if len(pinnedRepos2) > 0 {
+		if err = pinnedRepos2.LoadAttributes(); err != nil {
+			ctx.ServerError("pinnedRepos2.LoadAttributes()", err)
+			return
 		}
+		pinnedRepos = append(pinnedRepos, pinnedRepos2...)
 	}
 
 	ctx.Data["PinnedRepos"] = pinnedRepos

routers/user/profile.go

@@ -247,35 +247,44 @@ func Profile(ctx *context.Context) {
 		}
 
 		pinnedRepos := make([]*models.Repository, 0, 10)
-		has := false
-		for _, repo := range repos {
-			if has, err = ctxUser.IsPinnedRepoExist(repo.ID); err != nil {
-				ctx.ServerError("IsPinnedRepoExist", err)
-				return
-			}
-
-			if has {
-				pinnedRepos = append(pinnedRepos, repo)
-				repo.IsPinned = true
-			}
-		}
-
-		exitsPinnedRepoIDs := make([]int64, 0, len(pinnedRepos))
-		for _, repo := range pinnedRepos {
-			exitsPinnedRepoIDs = append(exitsPinnedRepoIDs, repo.ID)
+		pinnedRepoIDs, err := ctxUser.GetPinnedRepoIDs(ctx.User)
+		if err != nil {
+			ctx.ServerError("GetPinnedRepos", err)
+			return
 		}
 
-		var pinnedRepos2 []*models.Repository
+		pinnedRepos2 := make(models.RepositoryList, 0, 5)
+		for _, pinnedRepoID := range pinnedRepoIDs {
+			has := false
+			for _, repo := range repos {
+				if repo.ID == pinnedRepoID {
+					has = true
+					repo.IsPinned = true
+					pinnedRepos = append(pinnedRepos, repo)
+					break
+				}
+			}
 
-		pinnedRepos2, err = ctxUser.GetPinnedRepos(ctx.User, exitsPinnedRepoIDs, true)
-		if err != nil {
-			ctx.ServerError("GetPinnedRepos(true)", err)
-			return
+			if !has {
+				repo, err := models.GetRepositoryByID(pinnedRepoID)
+				if err != nil {
+					if !models.IsErrRepoNotExist(err) {
+						ctx.ServerError("GetRepositoryByID", err)
+					}
+					return
+				}
+
+				if repo != nil {
+					repo.IsPinned = true
+					pinnedRepos2 = append(pinnedRepos2, repo)
+				}
+			}
 		}
 
 		if len(pinnedRepos2) > 0 {
-			for _, repo := range pinnedRepos2 {
-				repo.IsPinned = true
+			if err = pinnedRepos2.LoadAttributes(); err != nil {
+				ctx.ServerError("pinnedRepos2.LoadAttributes()", err)
+				return
 			}
 			pinnedRepos = append(pinnedRepos, pinnedRepos2...)
 		}