Some improvements

Author: lunny

models/issues/issue.go

@@ -17,6 +17,7 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/container"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/modules/util"
@@ -95,8 +96,7 @@ type Issue struct {
 	// TODO: RemoveIssueRef: see "repo/issue/branch_selector_field.tmpl"
 	Ref string
 
-	PinOrder       int  `xorm:"-"`
-	pinOrderLoaded bool `xorm:"-"`
+	PinOrder int `xorm:"-"` // 0 means not loaded, -1 means loaded but not pinned
 
 	DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"`
 
@@ -291,16 +291,18 @@ func (issue *Issue) LoadMilestone(ctx context.Context) (err error) {
 }
 
 func (issue *Issue) LoadPinOrder(ctx context.Context) error {
-	if issue.pinOrderLoaded || issue.PinOrder > 0 {
+	if issue.PinOrder != 0 {
 		return nil
 	}
 	issuePin, err := GetIssuePin(ctx, issue)
 	if err != nil && !db.IsErrNotExist(err) {
 		return err
 	}
-	issue.pinOrderLoaded = true
+
 	if issuePin != nil {
 		issue.PinOrder = issuePin.PinOrder
+	} else {
+		issue.PinOrder = -1
 	}
 	return nil
 }
@@ -362,7 +364,10 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) {
 
 // IsPinned returns if a Issue is pinned
 func (issue *Issue) IsPinned() bool {
-	return issue.PinOrder != 0
+	if issue.PinOrder == 0 && (!setting.IsProd || setting.IsInTesting) {
+		log.Fatal("issue's pinorder has not been loaded")
+	}
+	return issue.PinOrder > 0
 }
 
 func (issue *Issue) ResetAttributesLoaded() {

models/issues/issue_list.go

@@ -512,7 +512,7 @@ func (issues IssueList) LoadPinOrder(ctx context.Context) error {
 	}
 
 	issueIDs := container.FilterSlice(issues, func(issue *Issue) (int64, bool) {
-		return issue.ID, issue.PinOrder == 0 && !issue.pinOrderLoaded
+		return issue.ID, issue.PinOrder == 0
 	})
 	if len(issueIDs) == 0 {
 		return nil
@@ -523,7 +523,7 @@ func (issues IssueList) LoadPinOrder(ctx context.Context) error {
 	}
 
 	for _, issue := range issues {
-		if issue.PinOrder > 0 || issue.pinOrderLoaded {
+		if issue.PinOrder != 0 {
 			continue
 		}
 		for _, pin := range issuePins {
@@ -532,6 +532,9 @@ func (issues IssueList) LoadPinOrder(ctx context.Context) error {
 				break
 			}
 		}
+		if issue.PinOrder == 0 {
+			issue.PinOrder = -1
+		}
 	}
 	return nil
 }

models/issues/issue_pin.go

@@ -208,6 +208,9 @@ func GetPinnedIssues(ctx context.Context, repoID int64, isPull bool) (IssueList,
 				break
 			}
 		}
+		if (!setting.IsProd || setting.IsInTesting) && issue.PinOrder == 0 {
+			panic("It should not happen that a pinned Issue has no PinOrder")
+		}
 	}
 	sort.Slice(issues, func(i, j int) bool {
 		return issues[i].PinOrder < issues[j].PinOrder

routers/web/repo/issue_pin.go

@@ -43,8 +43,12 @@ func IssuePinOrUnpin(ctx *context.Context) {
 	}
 
 	if err != nil {
-		ctx.Status(http.StatusInternalServerError)
-		log.Error(err.Error())
+		if issues_model.IsErrIssueMaxPinReached(err) {
+			ctx.JSONError(ctx.Tr("repo.issues.max_pinned"))
+		} else {
+			ctx.Status(http.StatusInternalServerError)
+			log.Error(err.Error())
+		}
 		return
 	}