From 66504c589d34c417a1e45475469672a03532694f Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Thu, 15 Aug 2019 01:34:20 -0300 Subject: [PATCH 01/10] Implement index parameter on api/create issue --- models/issue.go | 10 ++++--- models/issue_test.go | 51 ++++++++++++++++++++++++++++++++++ modules/structs/issue.go | 1 + routers/api/v1/repo/issue.go | 1 + templates/swagger/v1_json.tmpl | 5 ++++ 5 files changed, 64 insertions(+), 4 deletions(-) diff --git a/models/issue.go b/models/issue.go index 7561083e0fc69..3fc451a65631a 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1054,11 +1054,13 @@ func getMaxIndexOfIssue(e Engine, repoID int64) (int64, error) { func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { opts.Issue.Title = strings.TrimSpace(opts.Issue.Title) - maxIndex, err := getMaxIndexOfIssue(e, opts.Issue.RepoID) - if err != nil { - return err + if opts.Issue.Index == 0 { + maxIndex, err := getMaxIndexOfIssue(e, opts.Issue.RepoID) + if err != nil { + return err + } + opts.Issue.Index = maxIndex + 1 } - opts.Issue.Index = maxIndex + 1 if opts.Issue.MilestoneID > 0 { milestone, err := getMilestoneByRepoID(e, opts.Issue.RepoID, opts.Issue.MilestoneID) diff --git a/models/issue_test.go b/models/issue_test.go index 1a7e45ae02bda..6e8f3ef9430a6 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -320,3 +320,54 @@ func TestIssue_SearchIssueIDsByKeyword(t *testing.T) { assert.EqualValues(t, 1, total) assert.EqualValues(t, []int64{1}, ids) } + +func TestIssueCreateWithID(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + repo := AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository) + user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + index := int64(3000) + + issue := &Issue{ + Index: index, + RepoID: repo.ID, + Repo: repo, + Title: "TestIssueCreateWithID", + PosterID: 2, + Poster: user, + Content: "Issue body", + } + + err := NewIssue(repo, issue, nil, nil, nil) + assert.NoError(t, err) + + issue = &Issue{ + Index: index, + RepoID: repo.ID, + Repo: repo, + Title: "repeated TestIssueCreateWithID", + PosterID: 2, + Poster: user, + Content: "Issue body", + } + + err = NewIssue(repo, issue, nil, nil, nil) + assert.Error(t, err) + + issue = AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: index}).(*Issue) + + assert.Equal(t, "TestIssueCreateWithID", issue.Title) + + issue = &Issue{ + RepoID: repo.ID, + Repo: repo, + Title: "sequential TestIssueCreateWithID", + PosterID: 2, + Poster: user, + Content: "Issue body", + } + + err = NewIssue(repo, issue, nil, nil, nil) + assert.NoError(t, err) + assert.Equal(t, index+1, issue.Index) +} diff --git a/modules/structs/issue.go b/modules/structs/issue.go index 58fd7344b4f27..795fd99efc0d7 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -67,6 +67,7 @@ type ListIssueOption struct { // CreateIssueOption options to create one issue type CreateIssueOption struct { + Index int64 `json:"id"` // required:true Title string `json:"title" binding:"Required"` Body string `json:"body"` diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index daaa3d59856aa..8fa4deeb56747 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -189,6 +189,7 @@ func CreateIssue(ctx *context.APIContext, form api.CreateIssueOption) { } issue := &models.Issue{ + Index: form.Index, RepoID: ctx.Repo.Repository.ID, Repo: ctx.Repo.Repository, Title: form.Title, diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 4ae7f5a49ebfe..265a8fab75efa 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7350,6 +7350,11 @@ "format": "date-time", "x-go-name": "Deadline" }, + "id": { + "type": "integer", + "format": "int64", + "x-go-name": "Index" + }, "labels": { "description": "list of label ids", "type": "array", From 194cf50c374b723e657fef6f8c1a8833d0fe5c9a Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Thu, 15 Aug 2019 13:05:08 -0300 Subject: [PATCH 02/10] Add integration test --- integrations/api_issue_test.go | 66 ++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/integrations/api_issue_test.go b/integrations/api_issue_test.go index 24535057e2475..3df83d0776f1d 100644 --- a/integrations/api_issue_test.go +++ b/integrations/api_issue_test.go @@ -62,3 +62,69 @@ func TestAPICreateIssue(t *testing.T) { Title: title, }) } + +func TestAPICreateIssueID(t *testing.T) { + prepareTestEnv(t) + const body, firstTitle, dupTitle, freeTitle = "apiTestBody", "apiTestTitle-first", "apiTestTitle-dup", "apiTestTitle-free" + const firstIndex = int64(3000) + + repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) + owner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + + session := loginUser(t, owner.Name) + token := getTokenForLoggedInUser(t, session) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues?state=all&token=%s", owner.Name, repo.Name, token) + + // Must create with index 3000 + req := NewRequestWithJSON(t, "POST", urlStr, &api.CreateIssueOption{ + Index: firstIndex, + Body: body, + Title: firstTitle, + Assignee: owner.Name, + }) + resp := session.MakeRequest(t, req, http.StatusCreated) + var apiIssue api.Issue + DecodeJSON(t, resp, &apiIssue) + assert.Equal(t, apiIssue.Index, firstIndex) + assert.Equal(t, apiIssue.Body, body) + assert.Equal(t, apiIssue.Title, firstTitle) + + // Must fail + req = NewRequestWithJSON(t, "POST", urlStr, &api.CreateIssueOption{ + Index: firstIndex, + Body: body, + Title: dupTitle, + Assignee: owner.Name, + }) + resp = session.MakeRequest(t, req, http.StatusInternalServerError) + + // Must be the first one created + models.AssertExistsAndLoadBean(t, &models.Issue{ + Index: firstIndex, + RepoID: repo.ID, + AssigneeID: owner.ID, + Content: body, + Title: firstTitle, + }) + + // Must create with index firstIndex + 1 + req = NewRequestWithJSON(t, "POST", urlStr, &api.CreateIssueOption{ + Body: body, + Title: freeTitle, + Assignee: owner.Name, + }) + resp= session.MakeRequest(t, req, http.StatusCreated) + DecodeJSON(t, resp, &apiIssue) + assert.Equal(t, apiIssue.Index, firstIndex + 1) + assert.Equal(t, apiIssue.Body, body) + assert.Equal(t, apiIssue.Title, freeTitle) + + // Must be the last one created + models.AssertExistsAndLoadBean(t, &models.Issue{ + Index: firstIndex + 1, + RepoID: repo.ID, + AssigneeID: owner.ID, + Content: body, + Title: freeTitle, + }) +} From 54ef2854f378239c3b4bc90d74b47440776ab3d0 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Thu, 15 Aug 2019 14:20:17 -0300 Subject: [PATCH 03/10] Corrected fmt --- integrations/api_issue_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/integrations/api_issue_test.go b/integrations/api_issue_test.go index 3df83d0776f1d..20a6f55da086a 100644 --- a/integrations/api_issue_test.go +++ b/integrations/api_issue_test.go @@ -100,7 +100,7 @@ func TestAPICreateIssueID(t *testing.T) { // Must be the first one created models.AssertExistsAndLoadBean(t, &models.Issue{ - Index: firstIndex, + Index: firstIndex, RepoID: repo.ID, AssigneeID: owner.ID, Content: body, @@ -113,15 +113,15 @@ func TestAPICreateIssueID(t *testing.T) { Title: freeTitle, Assignee: owner.Name, }) - resp= session.MakeRequest(t, req, http.StatusCreated) + resp = session.MakeRequest(t, req, http.StatusCreated) DecodeJSON(t, resp, &apiIssue) - assert.Equal(t, apiIssue.Index, firstIndex + 1) + assert.Equal(t, apiIssue.Index, firstIndex+1) assert.Equal(t, apiIssue.Body, body) assert.Equal(t, apiIssue.Title, freeTitle) // Must be the last one created models.AssertExistsAndLoadBean(t, &models.Issue{ - Index: firstIndex + 1, + Index: firstIndex + 1, RepoID: repo.ID, AssigneeID: owner.ID, Content: body, From 57835343a2ef9e0adc9f262fcb05804ab6b69141 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Thu, 15 Aug 2019 21:11:48 -0300 Subject: [PATCH 04/10] Only admins can set issue index --- models/issue.go | 8 ++++++++ models/issue_test.go | 16 ++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/models/issue.go b/models/issue.go index b1f6c6ff6d52a..995399db54e83 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1061,6 +1061,14 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { return err } opts.Issue.Index = maxIndex + 1 + } else { + perm, err := GetUserRepoPermission(opts.Repo, doer) + if err != nil { + return err + } + if !perm.IsAdmin() { + return ErrUserDoesNotHaveAccessToRepo{UserID: doer.ID, RepoName: opts.Repo.Name} + } } if opts.Issue.MilestoneID > 0 { diff --git a/models/issue_test.go b/models/issue_test.go index 6e8f3ef9430a6..d12daf4b6f800 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -370,4 +370,20 @@ func TestIssueCreateWithID(t *testing.T) { err = NewIssue(repo, issue, nil, nil, nil) assert.NoError(t, err) assert.Equal(t, index+1, issue.Index) + + user = AssertExistsAndLoadBean(t, &User{ID: 24}).(*User) + + issue = &Issue{ + Index: index + 2, + RepoID: repo.ID, + Repo: repo, + Title: "not admin TestIssueCreateWithID", + PosterID: 2, + Poster: user, + Content: "Issue body", + } + + expectedError := ErrUserDoesNotHaveAccessToRepo{UserID: user.ID, RepoName: repo.Name}.Error() + err = NewIssue(repo, issue, nil, nil, nil) + assert.EqualError(t, err, expectedError) } From 370701def52872f44c7d789e4fe8aeac3d84837f Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Fri, 16 Aug 2019 00:22:34 -0300 Subject: [PATCH 05/10] Fix wording. --- models/issue_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue_test.go b/models/issue_test.go index d12daf4b6f800..9fbf8f8d601e2 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -345,7 +345,7 @@ func TestIssueCreateWithID(t *testing.T) { Index: index, RepoID: repo.ID, Repo: repo, - Title: "repeated TestIssueCreateWithID", + Title: "duplicate TestIssueCreateWithID", PosterID: 2, Poster: user, Content: "Issue body", From 18731bf952f4379655061b317f33e3b1cab99a0e Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Fri, 16 Aug 2019 02:23:38 -0300 Subject: [PATCH 06/10] Fix support PR sets opts.Issue.Index != 0 --- models/issue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue.go b/models/issue.go index 995399db54e83..98d11fb560add 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1055,7 +1055,7 @@ func getMaxIndexOfIssue(e Engine, repoID int64) (int64, error) { func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { opts.Issue.Title = strings.TrimSpace(opts.Issue.Title) - if opts.Issue.Index == 0 { + if opts.IsPull || opts.Issue.Index == 0 { maxIndex, err := getMaxIndexOfIssue(e, opts.Issue.RepoID) if err != nil { return err From e5c0201b2583a0e78beaa8eb28e35826f32fe7f5 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Fri, 16 Aug 2019 03:03:23 -0300 Subject: [PATCH 07/10] Account for edge case max(index)==2^64-1 --- models/issue.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/models/issue.go b/models/issue.go index 98d11fb560add..fa6879312b7d6 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1061,6 +1061,9 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { return err } opts.Issue.Index = maxIndex + 1 + if opts.Issue.Index == 0 { + return fmt.Errorf("Index numbers depleted") + } } else { perm, err := GetUserRepoPermission(opts.Repo, doer) if err != nil { From 7dd8c686da90e5ce6fa1fac8fd723cfffbdcee7b Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Fri, 16 Aug 2019 14:10:49 -0300 Subject: [PATCH 08/10] Correct index max value & index json prop name --- integrations/api_issue_test.go | 21 +++++++++- models/issue.go | 21 +++++----- models/issue_test.go | 76 ++++++++++++++++++++++++++++------ modules/structs/issue.go | 2 +- templates/swagger/v1_json.tmpl | 2 +- 5 files changed, 95 insertions(+), 27 deletions(-) diff --git a/integrations/api_issue_test.go b/integrations/api_issue_test.go index 20a6f55da086a..3c77092b8ee40 100644 --- a/integrations/api_issue_test.go +++ b/integrations/api_issue_test.go @@ -65,13 +65,17 @@ func TestAPICreateIssue(t *testing.T) { func TestAPICreateIssueID(t *testing.T) { prepareTestEnv(t) - const body, firstTitle, dupTitle, freeTitle = "apiTestBody", "apiTestTitle-first", "apiTestTitle-dup", "apiTestTitle-free" + const body, firstTitle, dupTitle, freeTitle, notAllowedTitle = "apiTestBody", "apiTestTitle-first", "apiTestTitle-dup", "apiTestTitle-free", "apiTestTitle-notAllowed" const firstIndex = int64(3000) repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) + admin := models.AssertExistsAndLoadBean(t, &models.User{ID: 1}).(*models.User) owner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) - session := loginUser(t, owner.Name) + assert.True(t, admin.IsAdmin) + assert.False(t, owner.IsAdmin) + + session := loginUser(t, admin.Name) token := getTokenForLoggedInUser(t, session) urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues?state=all&token=%s", owner.Name, repo.Name, token) @@ -127,4 +131,17 @@ func TestAPICreateIssueID(t *testing.T) { Content: body, Title: freeTitle, }) + + session = loginUser(t, owner.Name) + token = getTokenForLoggedInUser(t, session) + urlStr = fmt.Sprintf("/api/v1/repos/%s/%s/issues?state=all&token=%s", owner.Name, repo.Name, token) + + // Must fail create with index + req = NewRequestWithJSON(t, "POST", urlStr, &api.CreateIssueOption{ + Index: firstIndex + 2, + Body: body, + Title: notAllowedTitle, + Assignee: owner.Name, + }) + resp = session.MakeRequest(t, req, http.StatusBadRequest) } diff --git a/models/issue.go b/models/issue.go index fa6879312b7d6..c24f8adc2223a 100644 --- a/models/issue.go +++ b/models/issue.go @@ -75,6 +75,9 @@ var ( const issueTasksRegexpStr = `(^\s*[-*]\s\[[\sx]\]\s.)|(\n\s*[-*]\s\[[\sx]\]\s.)` const issueTasksDoneRegexpStr = `(^\s*[-*]\s\[[x]\]\s.)|(\n\s*[-*]\s\[[x]\]\s.)` +// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER +const issueMaxAllowableIndex = int64(2<<53 - 1) + func init() { issueTasksPat = regexp.MustCompile(issueTasksRegexpStr) issueTasksDonePat = regexp.MustCompile(issueTasksDoneRegexpStr) @@ -1061,17 +1064,13 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { return err } opts.Issue.Index = maxIndex + 1 - if opts.Issue.Index == 0 { - return fmt.Errorf("Index numbers depleted") - } - } else { - perm, err := GetUserRepoPermission(opts.Repo, doer) - if err != nil { - return err - } - if !perm.IsAdmin() { - return ErrUserDoesNotHaveAccessToRepo{UserID: doer.ID, RepoName: opts.Repo.Name} - } + } else if !doer.IsAdmin { + // Require admin to specify Issue.Index + return ErrUserDoesNotHaveAccessToRepo{UserID: doer.ID, RepoName: opts.Repo.Name} + } + + if opts.Issue.Index < 1 || opts.Issue.Index > issueMaxAllowableIndex { + return fmt.Errorf("Issue number out of range or max issue number reached") } if opts.Issue.MilestoneID > 0 { diff --git a/models/issue_test.go b/models/issue_test.go index 9fbf8f8d601e2..190e10f197fc8 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -325,16 +325,19 @@ func TestIssueCreateWithID(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) repo := AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository) - user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + admin := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User) + notadmin := AssertExistsAndLoadBean(t, &User{ID: repo.OwnerID}).(*User) index := int64(3000) + assert.True(t, admin.IsAdmin) + assert.False(t, notadmin.IsAdmin) issue := &Issue{ Index: index, RepoID: repo.ID, Repo: repo, Title: "TestIssueCreateWithID", - PosterID: 2, - Poster: user, + PosterID: admin.ID, + Poster: admin, Content: "Issue body", } @@ -346,8 +349,8 @@ func TestIssueCreateWithID(t *testing.T) { RepoID: repo.ID, Repo: repo, Title: "duplicate TestIssueCreateWithID", - PosterID: 2, - Poster: user, + PosterID: admin.ID, + Poster: admin, Content: "Issue body", } @@ -358,12 +361,38 @@ func TestIssueCreateWithID(t *testing.T) { assert.Equal(t, "TestIssueCreateWithID", issue.Title) + issue = &Issue{ + Index: -1, + RepoID: repo.ID, + Repo: repo, + Title: "neg index TestIssueCreateWithID", + PosterID: admin.ID, + Poster: admin, + Content: "Issue body", + } + + err = NewIssue(repo, issue, nil, nil, nil) + assert.Error(t, err) + + issue = &Issue{ + Index: issueMaxAllowableIndex + 1, + RepoID: repo.ID, + Repo: repo, + Title: "index out of range TestIssueCreateWithID", + PosterID: admin.ID, + Poster: admin, + Content: "Issue body", + } + + err = NewIssue(repo, issue, nil, nil, nil) + assert.Error(t, err) + issue = &Issue{ RepoID: repo.ID, Repo: repo, Title: "sequential TestIssueCreateWithID", - PosterID: 2, - Poster: user, + PosterID: notadmin.ID, + Poster: notadmin, Content: "Issue body", } @@ -371,19 +400,42 @@ func TestIssueCreateWithID(t *testing.T) { assert.NoError(t, err) assert.Equal(t, index+1, issue.Index) - user = AssertExistsAndLoadBean(t, &User{ID: 24}).(*User) - issue = &Issue{ Index: index + 2, RepoID: repo.ID, Repo: repo, Title: "not admin TestIssueCreateWithID", - PosterID: 2, - Poster: user, + PosterID: notadmin.ID, + Poster: notadmin, Content: "Issue body", } - expectedError := ErrUserDoesNotHaveAccessToRepo{UserID: user.ID, RepoName: repo.Name}.Error() + expectedError := ErrUserDoesNotHaveAccessToRepo{UserID: notadmin.ID, RepoName: repo.Name}.Error() err = NewIssue(repo, issue, nil, nil, nil) assert.EqualError(t, err, expectedError) + + issue = &Issue{ + Index: issueMaxAllowableIndex, + RepoID: repo.ID, + Repo: repo, + Title: "index barely in range TestIssueCreateWithID", + PosterID: admin.ID, + Poster: admin, + Content: "Issue body", + } + + err = NewIssue(repo, issue, nil, nil, nil) + assert.NoError(t, err) + + issue = &Issue{ + RepoID: repo.ID, + Repo: repo, + Title: "out of index space TestIssueCreateWithID", + PosterID: notadmin.ID, + Poster: notadmin, + Content: "Issue body", + } + + err = NewIssue(repo, issue, nil, nil, nil) + assert.Error(t, err) } diff --git a/modules/structs/issue.go b/modules/structs/issue.go index 795fd99efc0d7..7b5a47d1ae5e6 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -67,7 +67,7 @@ type ListIssueOption struct { // CreateIssueOption options to create one issue type CreateIssueOption struct { - Index int64 `json:"id"` + Index int64 `json:"index"` // required:true Title string `json:"title" binding:"Required"` Body string `json:"body"` diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 4d6abd5d13086..799249bb3ca6f 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7350,7 +7350,7 @@ "format": "date-time", "x-go-name": "Deadline" }, - "id": { + "index": { "type": "integer", "format": "int64", "x-go-name": "Index" From 2475f429911c15646c6f5b4424c30793c4977f12 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Fri, 16 Aug 2019 14:19:50 -0300 Subject: [PATCH 09/10] Max issue index re-set to max int64 --- models/issue.go | 5 +---- models/issue_test.go | 15 +-------------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/models/issue.go b/models/issue.go index c24f8adc2223a..d214649ac7ad4 100644 --- a/models/issue.go +++ b/models/issue.go @@ -75,9 +75,6 @@ var ( const issueTasksRegexpStr = `(^\s*[-*]\s\[[\sx]\]\s.)|(\n\s*[-*]\s\[[\sx]\]\s.)` const issueTasksDoneRegexpStr = `(^\s*[-*]\s\[[x]\]\s.)|(\n\s*[-*]\s\[[x]\]\s.)` -// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER -const issueMaxAllowableIndex = int64(2<<53 - 1) - func init() { issueTasksPat = regexp.MustCompile(issueTasksRegexpStr) issueTasksDonePat = regexp.MustCompile(issueTasksDoneRegexpStr) @@ -1069,7 +1066,7 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { return ErrUserDoesNotHaveAccessToRepo{UserID: doer.ID, RepoName: opts.Repo.Name} } - if opts.Issue.Index < 1 || opts.Issue.Index > issueMaxAllowableIndex { + if opts.Issue.Index < 1 { return fmt.Errorf("Issue number out of range or max issue number reached") } diff --git a/models/issue_test.go b/models/issue_test.go index 190e10f197fc8..30bac8fa79287 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -374,19 +374,6 @@ func TestIssueCreateWithID(t *testing.T) { err = NewIssue(repo, issue, nil, nil, nil) assert.Error(t, err) - issue = &Issue{ - Index: issueMaxAllowableIndex + 1, - RepoID: repo.ID, - Repo: repo, - Title: "index out of range TestIssueCreateWithID", - PosterID: admin.ID, - Poster: admin, - Content: "Issue body", - } - - err = NewIssue(repo, issue, nil, nil, nil) - assert.Error(t, err) - issue = &Issue{ RepoID: repo.ID, Repo: repo, @@ -415,7 +402,7 @@ func TestIssueCreateWithID(t *testing.T) { assert.EqualError(t, err, expectedError) issue = &Issue{ - Index: issueMaxAllowableIndex, + Index: 0x7fffffffffffffff, RepoID: repo.ID, Repo: repo, Title: "index barely in range TestIssueCreateWithID", From 73afec13ad4c57fa8f702f2b372cf128516d87c0 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sat, 17 Aug 2019 13:24:34 -0300 Subject: [PATCH 10/10] Issue.Index should not be calculated at CreatePullRequest() --- models/issue.go | 2 +- routers/api/v1/repo/pull.go | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/models/issue.go b/models/issue.go index d214649ac7ad4..fafcdee050028 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1055,7 +1055,7 @@ func getMaxIndexOfIssue(e Engine, repoID int64) (int64, error) { func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { opts.Issue.Title = strings.TrimSpace(opts.Issue.Title) - if opts.IsPull || opts.Issue.Index == 0 { + if opts.Issue.Index == 0 { maxIndex, err := getMaxIndexOfIssue(e, opts.Issue.RepoID) if err != nil { return err diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 8168c6b010eba..5ac542ec207b2 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -252,15 +252,8 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption deadlineUnix = timeutil.TimeStamp(form.Deadline.Unix()) } - maxIndex, err := models.GetMaxIndexOfIssue(repo.ID) - if err != nil { - ctx.ServerError("GetPatch", err) - return - } - prIssue := &models.Issue{ RepoID: repo.ID, - Index: maxIndex + 1, Title: form.Title, PosterID: ctx.User.ID, Poster: ctx.User,