Skip to content

Commit

Permalink
[Backport] [API] Allow only specific Colums to be updated on Issue (#…
Browse files Browse the repository at this point in the history
…9539) (#9580)

* Fix #9189 - API Allow only specific Colums to be updated on Issue (#9539)

* dont insert "-1" in any case to issue.poster_id

* Make sure API cant override importand fields

* code format

* add Test for IssueEdit

* load milestone and return it on IssueEdit via API

* extend Test for TestAPIEditIssue

* fix TEST

* make sure Poster is loaded

* keep code format on backport as it is
  • Loading branch information
6543 authored and techknowlogick committed Jan 2, 2020
1 parent cb3fe4c commit 17c691f
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 23 deletions.
59 changes: 59 additions & 0 deletions integrations/api_issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"net/http"
"testing"
"time"

"code.gitea.io/gitea/models"
api "code.gitea.io/gitea/modules/structs"
Expand Down Expand Up @@ -62,3 +63,61 @@ func TestAPICreateIssue(t *testing.T) {
Title: title,
})
}

func TestAPIEditIssue(t *testing.T) {
prepareTestEnv(t)

issueBefore := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 9}).(*models.Issue)
repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: issueBefore.RepoID}).(*models.Repository)
owner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User)
assert.NoError(t, issueBefore.LoadAttributes())
assert.Equal(t, int64(1019307200), int64(issueBefore.DeadlineUnix))
assert.Equal(t, api.StateOpen, issueBefore.State())

session := loginUser(t, owner.Name)
token := getTokenForLoggedInUser(t, session)

// update values of issue
issueState := "closed"
removeDeadline := time.Unix(0, 0)
milestone := int64(4)
body := "new content!"
title := "new title from api set"

urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d?token=%s", owner.Name, repo.Name, issueBefore.Index, token)
req := NewRequestWithJSON(t, "PATCH", urlStr, api.EditIssueOption{
State: &issueState,
Deadline: &removeDeadline,
Milestone: &milestone,
Body: &body,
Title: title,

// ToDo change more
})
resp := session.MakeRequest(t, req, http.StatusCreated)
var apiIssue api.Issue
DecodeJSON(t, resp, &apiIssue)

issueAfter := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 9}).(*models.Issue)

// check deleted user
assert.Equal(t, int64(500), issueAfter.PosterID)
assert.NoError(t, issueAfter.LoadAttributes())
assert.Equal(t, int64(-1), issueAfter.PosterID)
assert.Equal(t, int64(-1), issueBefore.PosterID)
assert.Equal(t, int64(-1), apiIssue.Poster.ID)

// API response
assert.Equal(t, api.StateClosed, apiIssue.State)
assert.Equal(t, milestone, apiIssue.Milestone.ID)
assert.Equal(t, body, apiIssue.Body)
assert.True(t, apiIssue.Deadline == nil)
assert.Equal(t, title, apiIssue.Title)

// in database
assert.Equal(t, api.StateClosed, issueAfter.State())
assert.Equal(t, milestone, issueAfter.MilestoneID)
assert.Equal(t, int64(0), int64(issueAfter.DeadlineUnix))
assert.Equal(t, body, issueAfter.Content)
assert.Equal(t, title, issueAfter.Title)
}
15 changes: 14 additions & 1 deletion models/fixtures/issue.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,17 @@
is_closed: false
is_pull: true
created_unix: 946684820
updated_unix: 978307180
updated_unix: 978307180

-
id: 9
repo_id: 42
index: 1
poster_id: 500
name: issue from deleted account
content: content from deleted account
is_closed: false
is_pull: false
created_unix: 946684830
updated_unix: 999307200
deadline_unix: 1019307200
8 changes: 8 additions & 0 deletions models/fixtures/milestone.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,11 @@
content: content3
is_closed: true
num_issues: 0

-
id: 4
repo_id: 42
name: milestone of repo42
content: content random
is_closed: false
num_issues: 0
3 changes: 2 additions & 1 deletion models/fixtures/repository.yml
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,8 @@
is_private: false
num_stars: 0
num_forks: 0
num_issues: 0
num_issues: 1
num_milestones: 1
is_mirror: false

-
Expand Down
40 changes: 24 additions & 16 deletions models/issue.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2014 The Gogs Authors. All rights reserved.
// Copyright 2020 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

Expand Down Expand Up @@ -238,6 +239,16 @@ func (issue *Issue) loadReactions(e Engine) (err error) {
return nil
}

func (issue *Issue) loadMilestone(e Engine) (err error) {
if issue.Milestone == nil && issue.MilestoneID > 0 {
issue.Milestone, err = getMilestoneByRepoID(e, issue.RepoID, issue.MilestoneID)
if err != nil && !IsErrMilestoneNotExist(err) {
return fmt.Errorf("getMilestoneByRepoID [repo_id: %d, milestone_id: %d]: %v", issue.RepoID, issue.MilestoneID, err)
}
}
return nil
}

func (issue *Issue) loadAttributes(e Engine) (err error) {
if err = issue.loadRepo(e); err != nil {
return
Expand All @@ -251,11 +262,8 @@ func (issue *Issue) loadAttributes(e Engine) (err error) {
return
}

if issue.Milestone == nil && issue.MilestoneID > 0 {
issue.Milestone, err = getMilestoneByRepoID(e, issue.RepoID, issue.MilestoneID)
if err != nil && !IsErrMilestoneNotExist(err) {
return fmt.Errorf("getMilestoneByRepoID [repo_id: %d, milestone_id: %d]: %v", issue.RepoID, issue.MilestoneID, err)
}
if err = issue.loadMilestone(e); err != nil {
return
}

if err = issue.loadAssignees(e); err != nil {
Expand Down Expand Up @@ -295,6 +303,11 @@ func (issue *Issue) LoadAttributes() error {
return issue.loadAttributes(x)
}

// LoadMilestone load milestone of this issue.
func (issue *Issue) LoadMilestone() error {
return issue.loadMilestone(x)
}

// GetIsRead load the `IsRead` field of the issue
func (issue *Issue) GetIsRead(userID int64) error {
issueUser := &IssueUser{IssueID: issue.ID, UID: userID}
Expand Down Expand Up @@ -1730,22 +1743,17 @@ func SearchIssueIDsByKeyword(kw string, repoID int64, limit, start int) (int64,
return total, ids, nil
}

func updateIssue(e Engine, issue *Issue) error {
_, err := e.ID(issue.ID).AllCols().Update(issue)
if err != nil {
return err
}
return nil
}

// UpdateIssue updates all fields of given issue.
func UpdateIssue(issue *Issue) error {
// UpdateIssueByAPI updates all allowed fields of given issue.
func UpdateIssueByAPI(issue *Issue) error {
sess := x.NewSession()
defer sess.Close()
if err := sess.Begin(); err != nil {
return err
}
if err := updateIssue(sess, issue); err != nil {
if _, err := sess.ID(issue.ID).Cols(
"name", "is_closed", "content", "milestone_id", "priority",
"deadline_unix", "updated_unix", "closed_unix", "is_locked").
Update(issue); err != nil {
return err
}
if err := issue.neuterCrossReferences(sess); err != nil {
Expand Down
10 changes: 7 additions & 3 deletions routers/api/v1/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,8 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
}
}

if err = models.UpdateIssue(issue); err != nil {
ctx.Error(500, "UpdateIssue", err)
if err = models.UpdateIssueByAPI(issue); err != nil {
ctx.InternalServerError(err)
return
}
if form.State != nil {
Expand All @@ -372,7 +372,11 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
// Refetch from database to assign some automatic values
issue, err = models.GetIssueByID(issue.ID)
if err != nil {
ctx.Error(500, "GetIssueByID", err)
ctx.InternalServerError(err)
return
}
if err = issue.LoadMilestone(); err != nil {
ctx.InternalServerError(err)
return
}
ctx.JSON(201, issue.APIFormat())
Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,8 @@ func EditPullRequest(ctx *context.APIContext, form api.EditPullRequestOption) {
}
}

if err = models.UpdateIssue(issue); err != nil {
ctx.Error(500, "UpdateIssue", err)
if err = models.UpdateIssueByAPI(issue); err != nil {
ctx.InternalServerError(err)
return
}
if form.State != nil {
Expand Down

0 comments on commit 17c691f

Please sign in to comment.