Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add UpdateIssueWithOptions #658

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions cloud/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type IssueService service

// UpdateQueryOptions specifies the optional parameters to the Edit issue
type UpdateQueryOptions struct {
NotifyUsers bool `url:"notifyUsers,omitempty"`
NotifyUsers bool `url:"notifyUsers"` // can't be omitted as this means it's omitted when false which isn't desired as this defaults to true
OverrideScreenSecurity bool `url:"overrideScreenSecurity,omitempty"`
OverrideEditableFlag bool `url:"overrideEditableFlag,omitempty"`
}
Expand Down Expand Up @@ -559,7 +559,7 @@ type GetWorklogsQueryOptions struct {
}

type AddWorklogQueryOptions struct {
NotifyUsers bool `url:"notifyUsers,omitempty"`
NotifyUsers bool `url:"notifyUsers"` // can't be omitted as this means it's omitted when false which isn't desired as this defaults to true
AdjustEstimate string `url:"adjustEstimate,omitempty"`
NewEstimate string `url:"newEstimate,omitempty"`
ReduceBy string `url:"reduceBy,omitempty"`
Expand Down Expand Up @@ -868,8 +868,24 @@ func (s *IssueService) Update(ctx context.Context, issue *Issue, opts *UpdateQue
// TODO Double check this method if this works as expected, is using the latest API and the response is complete
// This double check effort is done for v2 - Remove this two lines if this is completed.
func (s *IssueService) UpdateIssue(ctx context.Context, jiraID string, data map[string]interface{}) (*Response, error) {
apiEndpoint := fmt.Sprintf("rest/api/2/issue/%v", jiraID)
req, err := s.client.NewRequest(ctx, http.MethodPut, apiEndpoint, data)
return s.UpdateIssueWithOptions(ctx, jiraID, data, &UpdateQueryOptions{})
}

// UpdateIssueWithOptions updates an issue from a JSON patch,
// while also specifying query params. The issue is found by key.
//
// Jira API docs: https://developer.atlassian.com/cloud/jira/platform/rest/v2/api-group-issues/#api-rest-api-2-issue-issueidorkey-put
// Caller must close resp.Body
//
// TODO Double check this method if this works as expected, is using the latest API and the response is complete
// This double check effort is done for v2 - Remove this two lines if this is completed.
func (s *IssueService) UpdateIssueWithOptions(ctx context.Context, jiraID string, data map[string]interface{}, opts *UpdateQueryOptions) (*Response, error) {
a := fmt.Sprintf("rest/api/2/issue/%v", jiraID)
url, err := addOptions(a, opts)
if err != nil {
return nil, err
}
req, err := s.client.NewRequest(ctx, http.MethodPut, url, data)
if err != nil {
return nil, err
}
Expand Down
24 changes: 24 additions & 0 deletions cloud/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,30 @@ func TestIssueService_UpdateIssue(t *testing.T) {

}

func TestIssueService_UpdateIssueWithOptions(t *testing.T) {
setup()
defer teardown()
testMux.HandleFunc("/rest/api/2/issue/PROJ-9001", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, http.MethodPut)
testRequestURL(t, r, "/rest/api/2/issue/PROJ-9001")
testRequestParams(t, r, map[string]string{"notifyUsers": "true"})

w.WriteHeader(http.StatusNoContent)
})
jID := "PROJ-9001"
i := make(map[string]interface{})
fields := make(map[string]interface{})
i["fields"] = fields
resp, err := testClient.Issue.client.Issue.UpdateIssueWithOptions(context.Background(), jID, i, &UpdateQueryOptions{NotifyUsers: false})
if resp == nil {
t.Error("Expected resp. resp is nil")
}
if err != nil {
t.Errorf("Error given: %s", err)
}

}

func TestIssueService_AddComment(t *testing.T) {
setup()
defer teardown()
Expand Down
13 changes: 13 additions & 0 deletions cloud/jira_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,19 @@ func testRequestParams(t *testing.T, r *http.Request, want map[string]string) {

}

func Test_addOptions(t *testing.T) {
v, err := addOptions("rest/api/2/issue/123", &UpdateQueryOptions{NotifyUsers: false})
if err != nil {
t.Errorf("Expected no error. Got: %+v", err)
}

expectedOutput := "rest/api/2/issue/123?notifyUsers=false"
if v != expectedOutput {
t.Errorf("Expected: %+v, got: %+v", expectedOutput, v)
}

}

func TestNewClient_WrongUrl(t *testing.T) {
c, err := NewClient("://issues.apache.org/jira/", nil)

Expand Down
4 changes: 2 additions & 2 deletions onpremise/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type IssueService service

// UpdateQueryOptions specifies the optional parameters to the Edit issue
type UpdateQueryOptions struct {
NotifyUsers bool `url:"notifyUsers,omitempty"`
NotifyUsers bool `url:"notifyUsers"` // can't be omitted as this means it's omitted when false which isn't desired as this defaults to true
OverrideScreenSecurity bool `url:"overrideScreenSecurity,omitempty"`
OverrideEditableFlag bool `url:"overrideEditableFlag,omitempty"`
}
Expand Down Expand Up @@ -839,7 +839,7 @@ func (s *IssueService) Create(ctx context.Context, issue *Issue) (*Issue, *Respo
// This double check effort is done for v2 - Remove this two lines if this is completed.
func (s *IssueService) Update(ctx context.Context, issue *Issue, opts *UpdateQueryOptions) (*Issue, *Response, error) {
apiEndpoint := fmt.Sprintf("rest/api/2/issue/%v", issue.Key)
url, err := addOptions(apiEndpoint, opts)
url, err := addOptions(apiEndpoint, *opts)
if err != nil {
return nil, nil, err
}
Expand Down
54 changes: 54 additions & 0 deletions onpremise/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,60 @@ func TestIssueService_Update(t *testing.T) {
}
}

func TestIssueService_Update_with_false_opts(t *testing.T) {
setup()
defer teardown()
testMux.HandleFunc("/rest/api/2/issue/PROJ-9001", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, http.MethodPut)
testRequestURL(t, r, "/rest/api/2/issue/PROJ-9001")
testRequestParams(t, r, map[string]string{"notifyUsers": "false"})
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fails without the omitEmpty changes


w.WriteHeader(http.StatusNoContent)
})

i := &Issue{
Key: "PROJ-9001",
Fields: &IssueFields{
Description: "example bug report",
},
}
issue, _, err := testClient.Issue.Update(context.Background(), i, &UpdateQueryOptions{NotifyUsers: false})
if issue == nil {
t.Error("Expected issue. Issue is nil")
}
if err != nil {
t.Errorf("Error given: %s", err)
}

}

func TestIssueService_Update_with_multiple_opts(t *testing.T) {
setup()
defer teardown()
testMux.HandleFunc("/rest/api/2/issue/PROJ-9001", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, http.MethodPut)
testRequestURL(t, r, "/rest/api/2/issue/PROJ-9001")
testRequestParams(t, r, map[string]string{"notifyUsers": "false", "overrideScreenSecurity": "true"})

w.WriteHeader(http.StatusNoContent)
})

i := &Issue{
Key: "PROJ-9001",
Fields: &IssueFields{
Description: "example bug report",
},
}
issue, _, err := testClient.Issue.Update(context.Background(), i, &UpdateQueryOptions{NotifyUsers: false, OverrideScreenSecurity: true})
if issue == nil {
t.Error("Expected issue. Issue is nil")
}
if err != nil {
t.Errorf("Error given: %s", err)
}

}

func TestIssueService_UpdateIssue(t *testing.T) {
setup()
defer teardown()
Expand Down