Skip to content

Commit 1b11529

Browse files
JakobDevsilverwinddelvh
authored
Followup to pinned Issues (#24945)
This addressees some things from #24406 that came up after the PR was merged. Mostly from @delvh. --------- Co-authored-by: silverwind <[email protected]> Co-authored-by: delvh <[email protected]>
1 parent faae819 commit 1b11529

File tree

8 files changed

+39
-22
lines changed

8 files changed

+39
-22
lines changed

‎custom/conf/app.example.ini

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1044,7 +1044,7 @@ LEVEL = Info
10441044
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
10451045
;; List of reasons why a Pull Request or Issue can be locked
10461046
;LOCK_REASONS = Too heated,Off-topic,Resolved,Spam
1047-
;; Maximum number of pinned Issues
1047+
;; Maximum number of pinned Issues per repo
10481048
;; Set to 0 to disable pinning Issues
10491049
;MAX_PINNED = 3
10501050

‎docs/content/doc/administration/config-cheat-sheet.en-us.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ In addition there is _`StaticRootPath`_ which can be set as a built-in at build
141141
### Repository - Issue (`repository.issue`)
142142

143143
- `LOCK_REASONS`: **Too heated,Off-topic,Resolved,Spam**: A list of reasons why a Pull Request or Issue can be locked
144-
- `MAX_PINNED`: **3**: Maximum number of pinned Issues. Set to 0 to disable pinning Issues.
144+
- `MAX_PINNED`: **3**: Maximum number of pinned Issues per Repo. Set to 0 to disable pinning Issues.
145145

146146
### Repository - Upload (`repository.upload`)
147147

‎models/issues/issue.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,8 @@ func (issue *Issue) HasOriginalAuthor() bool {
687687
return issue.OriginalAuthor != "" && issue.OriginalAuthorID != 0
688688
}
689689

690+
var ErrIssueMaxPinReached = util.NewInvalidArgumentErrorf("the max number of pinned issues has been readched")
691+
690692
// IsPinned returns if a Issue is pinned
691693
func (issue *Issue) IsPinned() bool {
692694
return issue.PinOrder != 0
@@ -707,7 +709,7 @@ func (issue *Issue) Pin(ctx context.Context, user *user_model.User) error {
707709

708710
// Check if the maximum allowed Pins reached
709711
if maxPin >= setting.Repository.Issue.MaxPinned {
710-
return fmt.Errorf("You have reached the max number of pinned Issues")
712+
return ErrIssueMaxPinReached
711713
}
712714

713715
_, err = db.GetEngine(ctx).Table("issue").
@@ -856,10 +858,15 @@ func GetPinnedIssues(ctx context.Context, repoID int64, isPull bool) ([]*Issue,
856858
// IsNewPinnedAllowed returns if a new Issue or Pull request can be pinned
857859
func IsNewPinAllowed(ctx context.Context, repoID int64, isPull bool) (bool, error) {
858860
var maxPin int
859-
_, err := db.GetEngine(ctx).SQL("SELECT MAX(pin_order) FROM issue WHERE repo_id = ? AND is_pull = ?", repoID, isPull).Get(&maxPin)
861+
_, err := db.GetEngine(ctx).SQL("SELECT COUNT(pin_order) FROM issue WHERE repo_id = ? AND is_pull = ? AND pin_order > 0", repoID, isPull).Get(&maxPin)
860862
if err != nil {
861863
return false, err
862864
}
863865

864866
return maxPin < setting.Repository.Issue.MaxPinned, nil
865867
}
868+
869+
// IsErrIssueMaxPinReached returns if the error is, that the User can't pin more Issues
870+
func IsErrIssueMaxPinReached(err error) bool {
871+
return err == ErrIssueMaxPinReached
872+
}

‎routers/api/v1/repo/issue_pin.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ func PinIssue(ctx *context.APIContext) {
4545
if err != nil {
4646
if issues_model.IsErrIssueNotExist(err) {
4747
ctx.NotFound()
48+
} else if issues_model.IsErrIssueMaxPinReached(err) {
49+
ctx.Error(http.StatusBadRequest, "MaxPinReached", err)
4850
} else {
4951
ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err)
5052
}
@@ -55,11 +57,13 @@ func PinIssue(ctx *context.APIContext) {
5557
err = issue.LoadRepo(ctx)
5658
if err != nil {
5759
ctx.Error(http.StatusInternalServerError, "LoadRepo", err)
60+
return
5861
}
5962

6063
err = issue.Pin(ctx, ctx.Doer)
6164
if err != nil {
6265
ctx.Error(http.StatusInternalServerError, "PinIssue", err)
66+
return
6367
}
6468

6569
ctx.Status(http.StatusNoContent)
@@ -108,11 +112,13 @@ func UnpinIssue(ctx *context.APIContext) {
108112
err = issue.LoadRepo(ctx)
109113
if err != nil {
110114
ctx.Error(http.StatusInternalServerError, "LoadRepo", err)
115+
return
111116
}
112117

113118
err = issue.Unpin(ctx, ctx.Doer)
114119
if err != nil {
115120
ctx.Error(http.StatusInternalServerError, "UnpinIssue", err)
121+
return
116122
}
117123

118124
ctx.Status(http.StatusNoContent)
@@ -166,6 +172,7 @@ func MoveIssuePin(ctx *context.APIContext) {
166172
err = issue.MovePin(ctx, int(ctx.ParamsInt64(":position")))
167173
if err != nil {
168174
ctx.Error(http.StatusInternalServerError, "MovePin", err)
175+
return
169176
}
170177

171178
ctx.Status(http.StatusNoContent)
@@ -193,12 +200,12 @@ func ListPinnedIssues(ctx *context.APIContext) {
193200
// "200":
194201
// "$ref": "#/responses/IssueList"
195202
issues, err := issues_model.GetPinnedIssues(ctx, ctx.Repo.Repository.ID, false)
196-
197-
if err == nil {
198-
ctx.JSON(http.StatusOK, convert.ToAPIIssueList(ctx, issues))
199-
} else {
203+
if err != nil {
200204
ctx.Error(http.StatusInternalServerError, "LoadPinnedIssues", err)
205+
return
201206
}
207+
208+
ctx.JSON(http.StatusOK, convert.ToAPIIssueList(ctx, issues))
202209
}
203210

204211
// ListPinnedPullRequests returns a list of all pinned PRs
@@ -225,6 +232,7 @@ func ListPinnedPullRequests(ctx *context.APIContext) {
225232
issues, err := issues_model.GetPinnedIssues(ctx, ctx.Repo.Repository.ID, true)
226233
if err != nil {
227234
ctx.Error(http.StatusInternalServerError, "LoadPinnedPullRequests", err)
235+
return
228236
}
229237

230238
apiPrs := make([]*api.PullRequest, len(issues))

‎routers/web/repo/issue_pin.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
issues_model "code.gitea.io/gitea/models/issues"
1010
"code.gitea.io/gitea/modules/context"
1111
"code.gitea.io/gitea/modules/json"
12+
"code.gitea.io/gitea/modules/log"
1213
)
1314

1415
// IssuePinOrUnpin pin or unpin a Issue
@@ -19,12 +20,14 @@ func IssuePinOrUnpin(ctx *context.Context) {
1920
err := issue.LoadRepo(ctx)
2021
if err != nil {
2122
ctx.Status(http.StatusInternalServerError)
23+
log.Error(err.Error())
2224
return
2325
}
2426

2527
err = issue.PinOrUnpin(ctx, ctx.Doer)
2628
if err != nil {
2729
ctx.Status(http.StatusInternalServerError)
30+
log.Error(err.Error())
2831
return
2932
}
3033

@@ -33,22 +36,26 @@ func IssuePinOrUnpin(ctx *context.Context) {
3336

3437
// IssueUnpin unpins a Issue
3538
func IssueUnpin(ctx *context.Context) {
36-
issue, err := issues_model.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id"))
39+
issue, err := issues_model.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
3740
if err != nil {
38-
ctx.Status(http.StatusNoContent)
41+
ctx.Status(http.StatusInternalServerError)
42+
log.Error(err.Error())
3943
return
4044
}
4145

4246
// If we don't do this, it will crash when trying to add the pin event to the comment history
4347
err = issue.LoadRepo(ctx)
4448
if err != nil {
4549
ctx.Status(http.StatusInternalServerError)
50+
log.Error(err.Error())
4651
return
4752
}
4853

4954
err = issue.Unpin(ctx, ctx.Doer)
5055
if err != nil {
5156
ctx.Status(http.StatusInternalServerError)
57+
log.Error(err.Error())
58+
return
5259
}
5360

5461
ctx.Status(http.StatusNoContent)
@@ -69,18 +76,21 @@ func IssuePinMove(ctx *context.Context) {
6976
form := &movePinIssueForm{}
7077
if err := json.NewDecoder(ctx.Req.Body).Decode(&form); err != nil {
7178
ctx.Status(http.StatusInternalServerError)
79+
log.Error(err.Error())
7280
return
7381
}
7482

7583
issue, err := issues_model.GetIssueByID(ctx, form.ID)
7684
if err != nil {
7785
ctx.Status(http.StatusInternalServerError)
86+
log.Error(err.Error())
7887
return
7988
}
8089

8190
err = issue.MovePin(ctx, form.Position)
8291
if err != nil {
8392
ctx.Status(http.StatusInternalServerError)
93+
log.Error(err.Error())
8494
return
8595
}
8696

‎routers/web/web.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,8 +1025,8 @@ func registerRoutes(m *web.Route) {
10251025
m.Post("/resolve_conversation", reqRepoIssuesOrPullsReader, repo.UpdateResolveConversation)
10261026
m.Post("/attachments", repo.UploadIssueAttachment)
10271027
m.Post("/attachments/remove", repo.DeleteAttachment)
1028-
m.Delete("/unpin/{id}", reqRepoAdmin, repo.IssueUnpin)
1029-
m.Post("/pin_move", reqRepoAdmin, repo.IssuePinMove)
1028+
m.Delete("/unpin/{index}", reqRepoAdmin, repo.IssueUnpin)
1029+
m.Post("/move_pin", reqRepoAdmin, repo.IssuePinMove)
10301030
}, context.RepoMustNotBeArchived())
10311031
m.Group("/comments/{id}", func() {
10321032
m.Post("", repo.UpdateCommentContent)

‎templates/repo/issue/list.tmpl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
{{if .PinnedIssues}}
77
<div id="issue-pins" {{if .IsRepoAdmin}}data-is-repo-admin{{end}}>
88
{{range .PinnedIssues}}
9-
<div class="pinned-issue-card gt-word-break" data-move-url="{{$.Link}}/pin_move" data-issue-id="{{.ID}}">
9+
<div class="pinned-issue-card gt-word-break" data-move-url="{{$.Link}}/move_pin" data-issue-id="{{.ID}}">
1010
{{if eq $.Project.CardType 1}}
1111
<div class="card-attachment-images">
1212
{{range (index $.issuesAttachmentMap .ID)}}
@@ -21,7 +21,7 @@
2121
</div>
2222
<a class="pinned-issue-title muted issue-title" href="{{.Link}}">{{.Title | RenderEmoji $.Context | RenderCodeBlock}}</a>
2323
{{if $.IsRepoAdmin}}
24-
<a role="button" class="pinned-issue-unpin muted gt-df gt-ac" data-tooltip-content={{$.locale.Tr "repo.issues.unpin_issue"}} data-issue-id="{{.ID}}" data-unpin-url="{{$.Link}}/unpin/{{.ID}}">
24+
<a role="button" class="pinned-issue-unpin muted gt-df gt-ac" data-tooltip-content={{$.locale.Tr "repo.issues.unpin_issue"}} data-issue-id="{{.ID}}" data-unpin-url="{{$.Link}}/unpin/{{.Index}}">
2525
{{svg "octicon-x" 16}}
2626
</a>
2727
{{end}}

‎web_src/css/repo.css

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3419,14 +3419,6 @@ tbody.commit-list {
34193419
background: var(--color-card);
34203420
}
34213421

3422-
.pinned-issue-card .meta a {
3423-
color: inherit;
3424-
}
3425-
3426-
.pinned-issue-card .meta a:hover {
3427-
color: var(--color-primary);
3428-
}
3429-
34303422
.pinned-issue-icon,
34313423
.pinned-issue-unpin {
34323424
margin-top: 1px;

0 commit comments

Comments
 (0)