From 46954dd061f904ed3315dc31734f33e13737bcff Mon Sep 17 00:00:00 2001 From: JakobDev Date: Fri, 26 May 2023 12:16:26 +0200 Subject: [PATCH 1/3] Followup to pinned Issues --- custom/conf/app.example.ini | 2 +- .../doc/administration/config-cheat-sheet.en-us.md | 2 +- models/issues/issue.go | 9 +++++++-- routers/api/v1/repo/issue_pin.go | 2 ++ routers/web/repo/issue_pin.go | 4 ++-- routers/web/web.go | 2 +- templates/repo/issue/list.tmpl | 2 +- web_src/css/repo.css | 4 ++-- 8 files changed, 17 insertions(+), 10 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 44445579bdeea..768d227780cce 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1048,7 +1048,7 @@ LEVEL = Info ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; List of reasons why a Pull Request or Issue can be locked ;LOCK_REASONS = Too heated,Off-topic,Resolved,Spam -;; Maximum number of pinned Issues +;; Maximum number of pinned Issues per repo ;; Set to 0 to disable pinning Issues ;MAX_PINNED = 3 diff --git a/docs/content/doc/administration/config-cheat-sheet.en-us.md b/docs/content/doc/administration/config-cheat-sheet.en-us.md index 1fa4abcef2999..24eb623eb93bd 100644 --- a/docs/content/doc/administration/config-cheat-sheet.en-us.md +++ b/docs/content/doc/administration/config-cheat-sheet.en-us.md @@ -141,7 +141,7 @@ In addition there is _`StaticRootPath`_ which can be set as a built-in at build ### Repository - Issue (`repository.issue`) - `LOCK_REASONS`: **Too heated,Off-topic,Resolved,Spam**: A list of reasons why a Pull Request or Issue can be locked -- `MAX_PINNED`: **3**: Maximum number of pinned Issues. Set to 0 to disable pinning Issues. +- `MAX_PINNED`: **3**: Maximum number of pinned Issues per Repo. Set to 0 to disable pinning Issues. ### Repository - Upload (`repository.upload`) diff --git a/models/issues/issue.go b/models/issues/issue.go index 5015824e9b34b..8eb618392bdd1 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -707,7 +707,7 @@ func (issue *Issue) Pin(ctx context.Context, user *user_model.User) error { // Check if the maximum allowed Pins reached if maxPin >= setting.Repository.Issue.MaxPinned { - return fmt.Errorf("You have reached the max number of pinned Issues") + return util.NewInvalidArgumentErrorf("You have reached the max number of pinned Issues") } _, err = db.GetEngine(ctx).Table("issue"). @@ -856,10 +856,15 @@ func GetPinnedIssues(ctx context.Context, repoID int64, isPull bool) ([]*Issue, // IsNewPinnedAllowed returns if a new Issue or Pull request can be pinned func IsNewPinAllowed(ctx context.Context, repoID int64, isPull bool) (bool, error) { var maxPin int - _, err := db.GetEngine(ctx).SQL("SELECT MAX(pin_order) FROM issue WHERE repo_id = ? AND is_pull = ?", repoID, isPull).Get(&maxPin) + _, 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) if err != nil { return false, err } return maxPin < setting.Repository.Issue.MaxPinned, nil } + +// IsErrIssueMaxPinReached returns if the error is, that the User can't pin more Issues +func IsErrIssueMaxPinReached(err error) bool { + return err.Error() == "You have reached the max number of pinned Issues" +} diff --git a/routers/api/v1/repo/issue_pin.go b/routers/api/v1/repo/issue_pin.go index c96ede45f51b4..e696840c19dcb 100644 --- a/routers/api/v1/repo/issue_pin.go +++ b/routers/api/v1/repo/issue_pin.go @@ -45,6 +45,8 @@ func PinIssue(ctx *context.APIContext) { if err != nil { if issues_model.IsErrIssueNotExist(err) { ctx.NotFound() + } else if issues_model.IsErrIssueMaxPinReached(err) { + ctx.Error(http.StatusBadGateway, "MaxPinReached", err) } else { ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) } diff --git a/routers/web/repo/issue_pin.go b/routers/web/repo/issue_pin.go index 13f2d02fe8450..6a7f9b26b17bb 100644 --- a/routers/web/repo/issue_pin.go +++ b/routers/web/repo/issue_pin.go @@ -33,9 +33,9 @@ func IssuePinOrUnpin(ctx *context.Context) { // IssueUnpin unpins a Issue func IssueUnpin(ctx *context.Context) { - issue, err := issues_model.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id")) + issue, err := issues_model.GetIssueByID(ctx, ctx.ParamsInt64(":id")) if err != nil { - ctx.Status(http.StatusNoContent) + ctx.Status(http.StatusInternalServerError) return } diff --git a/routers/web/web.go b/routers/web/web.go index a38638c48340f..bbea04f299827 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1026,7 +1026,7 @@ func registerRoutes(m *web.Route) { m.Post("/attachments", repo.UploadIssueAttachment) m.Post("/attachments/remove", repo.DeleteAttachment) m.Delete("/unpin/{id}", reqRepoAdmin, repo.IssueUnpin) - m.Post("/pin_move", reqRepoAdmin, repo.IssuePinMove) + m.Post("/move_pin", reqRepoAdmin, repo.IssuePinMove) }, context.RepoMustNotBeArchived()) m.Group("/comments/{id}", func() { m.Post("", repo.UpdateCommentContent) diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index dab6652d2172e..1b9a8b57eb3d8 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -6,7 +6,7 @@ {{if .PinnedIssues}}
{{range .PinnedIssues}} -
+
{{if eq $.Project.CardType 1}}
{{range (index $.issuesAttachmentMap .ID)}} diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 069bf014b8729..718d7e5ff44fe 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -3402,11 +3402,11 @@ tbody.commit-list { background: var(--color-card); } -.pinned-issue-card .meta a { +.pinned-issue-card a { color: inherit; } -.pinned-issue-card .meta a:hover { +.pinned-issue-card a:hover { color: var(--color-primary); } From 975efd572d1592c817e6ead615795df6fec2ebb1 Mon Sep 17 00:00:00 2001 From: JakobDev Date: Sun, 28 May 2023 14:02:13 +0200 Subject: [PATCH 2/3] Some changes --- models/issues/issue.go | 6 ++++-- routers/api/v1/repo/issue_pin.go | 15 +++++++++++---- routers/web/repo/issue_pin.go | 9 +++++++++ tests/integration/api_issue_pin_test.go | 2 +- web_src/css/repo.css | 8 -------- 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/models/issues/issue.go b/models/issues/issue.go index 8eb618392bdd1..eab18f4892ce9 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -687,6 +687,8 @@ func (issue *Issue) HasOriginalAuthor() bool { return issue.OriginalAuthor != "" && issue.OriginalAuthorID != 0 } +var ErrIssueMaxPinReached = util.NewInvalidArgumentErrorf("the max number of pinned issues has been readched") + // IsPinned returns if a Issue is pinned func (issue *Issue) IsPinned() bool { return issue.PinOrder != 0 @@ -707,7 +709,7 @@ func (issue *Issue) Pin(ctx context.Context, user *user_model.User) error { // Check if the maximum allowed Pins reached if maxPin >= setting.Repository.Issue.MaxPinned { - return util.NewInvalidArgumentErrorf("You have reached the max number of pinned Issues") + return ErrIssueMaxPinReached } _, err = db.GetEngine(ctx).Table("issue"). @@ -866,5 +868,5 @@ func IsNewPinAllowed(ctx context.Context, repoID int64, isPull bool) (bool, erro // IsErrIssueMaxPinReached returns if the error is, that the User can't pin more Issues func IsErrIssueMaxPinReached(err error) bool { - return err.Error() == "You have reached the max number of pinned Issues" + return err == ErrIssueMaxPinReached } diff --git a/routers/api/v1/repo/issue_pin.go b/routers/api/v1/repo/issue_pin.go index e696840c19dcb..9d292e06613e2 100644 --- a/routers/api/v1/repo/issue_pin.go +++ b/routers/api/v1/repo/issue_pin.go @@ -46,7 +46,7 @@ func PinIssue(ctx *context.APIContext) { if issues_model.IsErrIssueNotExist(err) { ctx.NotFound() } else if issues_model.IsErrIssueMaxPinReached(err) { - ctx.Error(http.StatusBadGateway, "MaxPinReached", err) + ctx.Error(http.StatusBadRequest, "MaxPinReached", err) } else { ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) } @@ -57,11 +57,13 @@ func PinIssue(ctx *context.APIContext) { err = issue.LoadRepo(ctx) if err != nil { ctx.Error(http.StatusInternalServerError, "LoadRepo", err) + return } err = issue.Pin(ctx, ctx.Doer) if err != nil { ctx.Error(http.StatusInternalServerError, "PinIssue", err) + return } ctx.Status(http.StatusNoContent) @@ -110,11 +112,13 @@ func UnpinIssue(ctx *context.APIContext) { err = issue.LoadRepo(ctx) if err != nil { ctx.Error(http.StatusInternalServerError, "LoadRepo", err) + return } err = issue.Unpin(ctx, ctx.Doer) if err != nil { ctx.Error(http.StatusInternalServerError, "UnpinIssue", err) + return } ctx.Status(http.StatusNoContent) @@ -168,6 +172,7 @@ func MoveIssuePin(ctx *context.APIContext) { err = issue.MovePin(ctx, int(ctx.ParamsInt64(":position"))) if err != nil { ctx.Error(http.StatusInternalServerError, "MovePin", err) + return } ctx.Status(http.StatusNoContent) @@ -196,11 +201,12 @@ func ListPinnedIssues(ctx *context.APIContext) { // "$ref": "#/responses/IssueList" issues, err := issues_model.GetPinnedIssues(ctx, ctx.Repo.Repository.ID, false) - if err == nil { - ctx.JSON(http.StatusOK, convert.ToAPIIssueList(ctx, issues)) - } else { + if err != nil { ctx.Error(http.StatusInternalServerError, "LoadPinnedIssues", err) + return } + + ctx.JSON(http.StatusOK, convert.ToAPIIssueList(ctx, issues)) } // ListPinnedPullRequests returns a list of all pinned PRs @@ -227,6 +233,7 @@ func ListPinnedPullRequests(ctx *context.APIContext) { issues, err := issues_model.GetPinnedIssues(ctx, ctx.Repo.Repository.ID, true) if err != nil { ctx.Error(http.StatusInternalServerError, "LoadPinnedPullRequests", err) + return } apiPrs := make([]*api.PullRequest, len(issues)) diff --git a/routers/web/repo/issue_pin.go b/routers/web/repo/issue_pin.go index 6a7f9b26b17bb..5e1ee9cfa149d 100644 --- a/routers/web/repo/issue_pin.go +++ b/routers/web/repo/issue_pin.go @@ -9,6 +9,7 @@ import ( issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/modules/log" ) // IssuePinOrUnpin pin or unpin a Issue @@ -19,12 +20,14 @@ func IssuePinOrUnpin(ctx *context.Context) { err := issue.LoadRepo(ctx) if err != nil { ctx.Status(http.StatusInternalServerError) + log.Error(err.Error()) return } err = issue.PinOrUnpin(ctx, ctx.Doer) if err != nil { ctx.Status(http.StatusInternalServerError) + log.Error(err.Error()) return } @@ -36,6 +39,7 @@ func IssueUnpin(ctx *context.Context) { issue, err := issues_model.GetIssueByID(ctx, ctx.ParamsInt64(":id")) if err != nil { ctx.Status(http.StatusInternalServerError) + log.Error(err.Error()) return } @@ -43,12 +47,14 @@ func IssueUnpin(ctx *context.Context) { err = issue.LoadRepo(ctx) if err != nil { ctx.Status(http.StatusInternalServerError) + log.Error(err.Error()) return } err = issue.Unpin(ctx, ctx.Doer) if err != nil { ctx.Status(http.StatusInternalServerError) + log.Error(err.Error()) } ctx.Status(http.StatusNoContent) @@ -69,18 +75,21 @@ func IssuePinMove(ctx *context.Context) { form := &movePinIssueForm{} if err := json.NewDecoder(ctx.Req.Body).Decode(&form); err != nil { ctx.Status(http.StatusInternalServerError) + log.Error(err.Error()) return } issue, err := issues_model.GetIssueByID(ctx, form.ID) if err != nil { ctx.Status(http.StatusInternalServerError) + log.Error(err.Error()) return } err = issue.MovePin(ctx, form.Position) if err != nil { ctx.Status(http.StatusInternalServerError) + log.Error(err.Error()) return } diff --git a/tests/integration/api_issue_pin_test.go b/tests/integration/api_issue_pin_test.go index 65be1d74f2d33..2d8f2b0c2afe2 100644 --- a/tests/integration/api_issue_pin_test.go +++ b/tests/integration/api_issue_pin_test.go @@ -148,7 +148,7 @@ func TestAPIListPinnedIssues(t *testing.T) { assert.NoError(t, unittest.LoadFixtures()) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: repo.ID}) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: repo.ID, IsPull: false}) owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) session := loginUser(t, owner.Name) diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 718d7e5ff44fe..4142dfa0d559e 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -3402,14 +3402,6 @@ tbody.commit-list { background: var(--color-card); } -.pinned-issue-card a { - color: inherit; -} - -.pinned-issue-card a:hover { - color: var(--color-primary); -} - .pinned-issue-icon, .pinned-issue-unpin { margin-top: 1px; From ccc1876ccdede9f5a69c75c37ba6e87b2916f6e2 Mon Sep 17 00:00:00 2001 From: JakobDev Date: Tue, 30 May 2023 16:38:17 +0200 Subject: [PATCH 3/3] Push changes --- routers/api/v1/repo/issue_pin.go | 1 - routers/web/repo/issue_pin.go | 3 ++- routers/web/web.go | 2 +- templates/repo/issue/list.tmpl | 2 +- tests/integration/api_issue_pin_test.go | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/routers/api/v1/repo/issue_pin.go b/routers/api/v1/repo/issue_pin.go index 9d292e06613e2..1e774d4afa07a 100644 --- a/routers/api/v1/repo/issue_pin.go +++ b/routers/api/v1/repo/issue_pin.go @@ -200,7 +200,6 @@ func ListPinnedIssues(ctx *context.APIContext) { // "200": // "$ref": "#/responses/IssueList" issues, err := issues_model.GetPinnedIssues(ctx, ctx.Repo.Repository.ID, false) - if err != nil { ctx.Error(http.StatusInternalServerError, "LoadPinnedIssues", err) return diff --git a/routers/web/repo/issue_pin.go b/routers/web/repo/issue_pin.go index 5e1ee9cfa149d..6586372fc5310 100644 --- a/routers/web/repo/issue_pin.go +++ b/routers/web/repo/issue_pin.go @@ -36,7 +36,7 @@ func IssuePinOrUnpin(ctx *context.Context) { // IssueUnpin unpins a Issue func IssueUnpin(ctx *context.Context) { - issue, err := issues_model.GetIssueByID(ctx, ctx.ParamsInt64(":id")) + issue, err := issues_model.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { ctx.Status(http.StatusInternalServerError) log.Error(err.Error()) @@ -55,6 +55,7 @@ func IssueUnpin(ctx *context.Context) { if err != nil { ctx.Status(http.StatusInternalServerError) log.Error(err.Error()) + return } ctx.Status(http.StatusNoContent) diff --git a/routers/web/web.go b/routers/web/web.go index bbea04f299827..a70e9cc3d37a7 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1025,7 +1025,7 @@ func registerRoutes(m *web.Route) { m.Post("/resolve_conversation", reqRepoIssuesOrPullsReader, repo.UpdateResolveConversation) m.Post("/attachments", repo.UploadIssueAttachment) m.Post("/attachments/remove", repo.DeleteAttachment) - m.Delete("/unpin/{id}", reqRepoAdmin, repo.IssueUnpin) + m.Delete("/unpin/{index}", reqRepoAdmin, repo.IssueUnpin) m.Post("/move_pin", reqRepoAdmin, repo.IssuePinMove) }, context.RepoMustNotBeArchived()) m.Group("/comments/{id}", func() { diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 1b9a8b57eb3d8..05aa86fd79884 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -21,7 +21,7 @@
{{.Title | RenderEmoji $.Context | RenderCodeBlock}} {{if $.IsRepoAdmin}} - + {{svg "octicon-x" 16}} {{end}} diff --git a/tests/integration/api_issue_pin_test.go b/tests/integration/api_issue_pin_test.go index 2d8f2b0c2afe2..65be1d74f2d33 100644 --- a/tests/integration/api_issue_pin_test.go +++ b/tests/integration/api_issue_pin_test.go @@ -148,7 +148,7 @@ func TestAPIListPinnedIssues(t *testing.T) { assert.NoError(t, unittest.LoadFixtures()) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: repo.ID, IsPull: false}) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: repo.ID}) owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) session := loginUser(t, owner.Name)