From aeec1e9fd30fc6a0125fb180c87a46558bd6f5bf Mon Sep 17 00:00:00 2001 From: Oliver Palmer Date: Wed, 13 Sep 2017 21:17:07 -0400 Subject: [PATCH 1/8] add AbandonChange --- CHANGELOG.md | 4 ++++ changes.go | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6de41f..f02eb35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ first. For more complete details see ## Versions +### 0.5.1 + +* Added the `AbandonChange` function. + ### 0.5.0 **WARNING**: This release includes breaking changes. diff --git a/changes.go b/changes.go index 81450e2..e6ac1df 100644 --- a/changes.go +++ b/changes.go @@ -31,9 +31,17 @@ type GitPersonInfo struct { TZ int `json:"tz"` } +// NotifyInfo entity contains detailed information about who should be +// notified about an update +type NotifyInfo struct { + Accounts []AccountInfo `json:"accounts"` +} + // AbandonInput entity contains information for abandoning a change. type AbandonInput struct { - Message string `json:"message,omitempty"` + Message string `json:"message,omitempty"` + Notify string `json:"notify"` + NotifyDetails []NotifyInfo `json:"notify_details"` } // ApprovalInfo entity contains information about an approval from a user for a label on a change. @@ -706,9 +714,29 @@ func (s *ChangesService) SubmitChange(changeID string, input *SubmitInput) (*Cha return v, resp, err } +// AbandonChange abandons a change. +// +// Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#abandon-change +func (s *ChangesService) AbandonChange(changeID string, input *AbandonInput) (*ChangeInfo, *Response, error) { + u := fmt.Sprintf("changes/%s/abandon", changeID) + + req, err := s.client.NewRequest("POST", u, input) + if err != nil { + return nil, nil, err + } + + v := new(ChangeInfo) + + resp, err := s.client.Do(req, v) + if resp.StatusCode == http.StatusConflict { + body, _ := ioutil.ReadAll(resp.Body) + err = errors.New(string(body[:])) + } + return v, resp, err +} + /* Missing Change Endpoints - Abandon Change Restore Change Rebase Change Revert Change From 0412c6febfcc06174621031bc010836aa7e00254 Mon Sep 17 00:00:00 2001 From: Oliver Palmer Date: Wed, 13 Sep 2017 21:27:58 -0400 Subject: [PATCH 2/8] add RebaseChange --- CHANGELOG.md | 2 +- changes.go | 28 +++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f02eb35..6b05dd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ first. For more complete details see ### 0.5.1 -* Added the `AbandonChange` function. +* Added the `AbandonChange` and `RebaseChange` functions. ### 0.5.0 diff --git a/changes.go b/changes.go index e6ac1df..451735e 100644 --- a/changes.go +++ b/changes.go @@ -716,6 +716,9 @@ func (s *ChangesService) SubmitChange(changeID string, input *SubmitInput) (*Cha // AbandonChange abandons a change. // +// The request body does not need to include a AbandonInput entity if no review +// comment is added. +// // Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#abandon-change func (s *ChangesService) AbandonChange(changeID string, input *AbandonInput) (*ChangeInfo, *Response, error) { u := fmt.Sprintf("changes/%s/abandon", changeID) @@ -735,9 +738,32 @@ func (s *ChangesService) AbandonChange(changeID string, input *AbandonInput) (*C return v, resp, err } +// RebaseChange rebases a change. +// +// Optionally, the parent revision can be changed to another patch set through +// the RebaseInput entity. +// +// Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#rebase-change +func (s *ChangesService) RebaseChange(changeID string, input *RebaseInput) (*ChangeInfo, *Response, error) { + u := fmt.Sprintf("changes/%s/rebase", changeID) + + req, err := s.client.NewRequest("POST", u, input) + if err != nil { + return nil, nil, err + } + + v := new(ChangeInfo) + + resp, err := s.client.Do(req, v) + if resp.StatusCode == http.StatusConflict { + body, _ := ioutil.ReadAll(resp.Body) + err = errors.New(string(body[:])) + } + return v, resp, err +} + /* Missing Change Endpoints Restore Change - Rebase Change Revert Change */ From 8cd4eeb54ae44680877da86f58554b6226c0b955 Mon Sep 17 00:00:00 2001 From: Oliver Palmer Date: Wed, 13 Sep 2017 21:41:56 -0400 Subject: [PATCH 3/8] added RestoreChange --- CHANGELOG.md | 2 +- changes.go | 26 +++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b05dd7..e4769f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ first. For more complete details see ### 0.5.1 -* Added the `AbandonChange` and `RebaseChange` functions. +* Added the `AbandonChange`, `RebaseChange` and `RestoreChange` functions. ### 0.5.0 diff --git a/changes.go b/changes.go index 451735e..effe0eb 100644 --- a/changes.go +++ b/changes.go @@ -762,8 +762,32 @@ func (s *ChangesService) RebaseChange(changeID string, input *RebaseInput) (*Cha return v, resp, err } +// RestoreChange restores a change. +// +// The request body does not need to include a RestoreInput entity if no review +// comment is added. +// +// Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#restore-change +func (s *ChangesService) RestoreChange(changeID string, input *RestoreInput) (*ChangeInfo, *Response, error) { + u := fmt.Sprintf("changes/%s/restore", changeID) + + req, err := s.client.NewRequest("POST", u, input) + if err != nil { + return nil, nil, err + } + + v := new(ChangeInfo) + + resp, err := s.client.Do(req, v) + if resp.StatusCode == http.StatusConflict { + body, _ := ioutil.ReadAll(resp.Body) + err = errors.New(string(body[:])) + } + return v, resp, err +} + + /* Missing Change Endpoints - Restore Change Revert Change */ From 9fa2048c427fc3cef39b54c6238a2e7ce9f45b9d Mon Sep 17 00:00:00 2001 From: Oliver Palmer Date: Wed, 13 Sep 2017 21:48:01 -0400 Subject: [PATCH 4/8] added RevertChange --- CHANGELOG.md | 3 ++- changes.go | 27 +++++++++++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4769f9..d40df86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,8 @@ first. For more complete details see ### 0.5.1 -* Added the `AbandonChange`, `RebaseChange` and `RestoreChange` functions. +* Added the `AbandonChange`, `RebaseChange`, `RestoreChange` and + `RevertChange` functions. ### 0.5.0 diff --git a/changes.go b/changes.go index effe0eb..d5f3c97 100644 --- a/changes.go +++ b/changes.go @@ -786,8 +786,27 @@ func (s *ChangesService) RestoreChange(changeID string, input *RestoreInput) (*C return v, resp, err } +// RevertChange reverts a change. +// +// The request body does not need to include a RevertInput entity if no +// review comment is added. +// +// Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#revert-change +func (s *ChangesService) RevertChange(changeID string, input *RevertInput) (*ChangeInfo, *Response, error) { + u := fmt.Sprintf("changes/%s/revert", changeID) + + req, err := s.client.NewRequest("POST", u, input) + if err != nil { + return nil, nil, err + } + + v := new(ChangeInfo) + + resp, err := s.client.Do(req, v) + if resp.StatusCode == http.StatusConflict { + body, _ := ioutil.ReadAll(resp.Body) + err = errors.New(string(body[:])) + } + return v, resp, err +} -/* -Missing Change Endpoints - Revert Change -*/ From 53d3be9d04c03fdc11390e7450db834c38789838 Mon Sep 17 00:00:00 2001 From: Oliver Palmer Date: Wed, 13 Sep 2017 21:54:46 -0400 Subject: [PATCH 5/8] fmt fix --- changes.go | 1 - 1 file changed, 1 deletion(-) diff --git a/changes.go b/changes.go index d5f3c97..dbaf7ea 100644 --- a/changes.go +++ b/changes.go @@ -809,4 +809,3 @@ func (s *ChangesService) RevertChange(changeID string, input *RevertInput) (*Cha } return v, resp, err } - From b512feb00845fa3e9ad1502f54a305fb318a961a Mon Sep 17 00:00:00 2001 From: Oliver Palmer Date: Thu, 14 Sep 2017 08:05:09 -0400 Subject: [PATCH 6/8] adding tests --- changes_test.go | 191 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 191 insertions(+) diff --git a/changes_test.go b/changes_test.go index 70c4c3a..ddb1b97 100644 --- a/changes_test.go +++ b/changes_test.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "testing" "github.com/andygrunwald/go-gerrit" ) @@ -77,3 +78,193 @@ func ExampleChangesService_PublishChangeEdit() { panic(err) } } + +func TestChangesService_SubmitChange(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/changes/123/submit" { + t.Errorf("%s != /changes/123/submit", r.URL.Path) + } + fmt.Fprint(w, `{"id": "123"}`) + })) + defer ts.Close() + + client, err := gerrit.NewClient(ts.URL, nil) + if err != nil { + t.Error(err) + } + info, _, err := client.Changes.SubmitChange("123", nil) + if err != nil { + t.Error(err) + } + if info.ID != "123" { + t.Error("Invalid id") + } +} + +func TestChangesService_SubmitChange_Conflict(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusConflict) + })) + defer ts.Close() + + client, err := gerrit.NewClient(ts.URL, nil) + if err != nil { + t.Error(err) + } + _, response, _ := client.Changes.SubmitChange("123", nil) + if response.StatusCode != http.StatusConflict { + t.Error() + } +} + +func TestChangesService_AbandonChange(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/changes/123/abandon" { + t.Errorf("%s != /changes/123/abandon", r.URL.Path) + } + fmt.Fprint(w, `{"id": "123"}`) + })) + defer ts.Close() + + client, err := gerrit.NewClient(ts.URL, nil) + if err != nil { + t.Error(err) + } + info, _, err := client.Changes.AbandonChange("123", nil) + if err != nil { + t.Error(err) + } + if info.ID != "123" { + t.Error("Invalid id") + } +} + +func TestChangesService_AbandonChange_Conflict(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusConflict) + })) + defer ts.Close() + + client, err := gerrit.NewClient(ts.URL, nil) + if err != nil { + t.Error(err) + } + _, response, _ := client.Changes.AbandonChange("123", nil) + if response.StatusCode != http.StatusConflict { + t.Error() + } +} + +func TestChangesService_RebaseChange(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/changes/123/rebase" { + t.Errorf("%s != /changes/123/rebase", r.URL.Path) + } + fmt.Fprint(w, `{"id": "123"}`) + })) + defer ts.Close() + + client, err := gerrit.NewClient(ts.URL, nil) + if err != nil { + t.Error(err) + } + info, _, err := client.Changes.RebaseChange("123", nil) + if err != nil { + t.Error(err) + } + if info.ID != "123" { + t.Error("Invalid id") + } +} + +func TestChangesService_RebaseChange_Conflict(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusConflict) + })) + defer ts.Close() + + client, err := gerrit.NewClient(ts.URL, nil) + if err != nil { + t.Error(err) + } + _, response, _ := client.Changes.RebaseChange("123", nil) + if response.StatusCode != http.StatusConflict { + t.Error() + } +} + +func TestChangesService_RestoreChange(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/changes/123/restore" { + t.Errorf("%s != /changes/123/restore", r.URL.Path) + } + fmt.Fprint(w, `{"id": "123"}`) + })) + defer ts.Close() + + client, err := gerrit.NewClient(ts.URL, nil) + if err != nil { + t.Error(err) + } + info, _, err := client.Changes.RestoreChange("123", nil) + if err != nil { + t.Error(err) + } + if info.ID != "123" { + t.Error("Invalid id") + } +} + +func TestChangesService_RestoreChange_Conflict(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusConflict) + })) + defer ts.Close() + + client, err := gerrit.NewClient(ts.URL, nil) + if err != nil { + t.Error(err) + } + _, response, _ := client.Changes.RestoreChange("123", nil) + if response.StatusCode != http.StatusConflict { + t.Error() + } +} + +func TestChangesService_RevertChange(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/changes/123/revert" { + t.Errorf("%s != /changes/123/revert", r.URL.Path) + } + fmt.Fprint(w, `{"id": "123"}`) + })) + defer ts.Close() + + client, err := gerrit.NewClient(ts.URL, nil) + if err != nil { + t.Error(err) + } + info, _, err := client.Changes.RevertChange("123", nil) + if err != nil { + t.Error(err) + } + if info.ID != "123" { + t.Error("Invalid id") + } +} + +func TestChangesService_RevertChange_Conflict(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusConflict) + })) + defer ts.Close() + + client, err := gerrit.NewClient(ts.URL, nil) + if err != nil { + t.Error(err) + } + _, response, _ := client.Changes.RevertChange("123", nil) + if response.StatusCode != http.StatusConflict { + t.Error() + } +} From 75d549bc68104277b28b30051490acdab6a504b0 Mon Sep 17 00:00:00 2001 From: Oliver Palmer Date: Thu, 14 Sep 2017 08:14:54 -0400 Subject: [PATCH 7/8] create internal function to consolidate code --- changes.go | 86 +++++++++++------------------------------------------- 1 file changed, 17 insertions(+), 69 deletions(-) diff --git a/changes.go b/changes.go index dbaf7ea..c40bd75 100644 --- a/changes.go +++ b/changes.go @@ -691,21 +691,16 @@ func (s *ChangesService) FixChange(changeID string, input *FixInput) (*ChangeInf return v, resp, err } -// SubmitChange submits a change. -// -// The request body only needs to include a SubmitInput entity if submitting on behalf of another user. -// -// Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#submit-change -func (s *ChangesService) SubmitChange(changeID string, input *SubmitInput) (*ChangeInfo, *Response, error) { - u := fmt.Sprintf("changes/%s/submit", changeID) - +// change is an internal function to consolidate code used by SubmitChange, +// AbandonChange and other similar functions. +func (s *ChangesService) change(tail string, changeID string, input interface{}) (*ChangeInfo, *Response, error) { + u := fmt.Sprintf("changes/%s/%s", changeID, tail) req, err := s.client.NewRequest("POST", u, input) if err != nil { return nil, nil, err } v := new(ChangeInfo) - resp, err := s.client.Do(req, v) if resp.StatusCode == http.StatusConflict { body, _ := ioutil.ReadAll(resp.Body) @@ -714,6 +709,15 @@ func (s *ChangesService) SubmitChange(changeID string, input *SubmitInput) (*Cha return v, resp, err } +// SubmitChange submits a change. +// +// The request body only needs to include a SubmitInput entity if submitting on behalf of another user. +// +// Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#submit-change +func (s *ChangesService) SubmitChange(changeID string, input *SubmitInput) (*ChangeInfo, *Response, error) { + return s.change("submit", changeID, input) +} + // AbandonChange abandons a change. // // The request body does not need to include a AbandonInput entity if no review @@ -721,21 +725,7 @@ func (s *ChangesService) SubmitChange(changeID string, input *SubmitInput) (*Cha // // Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#abandon-change func (s *ChangesService) AbandonChange(changeID string, input *AbandonInput) (*ChangeInfo, *Response, error) { - u := fmt.Sprintf("changes/%s/abandon", changeID) - - req, err := s.client.NewRequest("POST", u, input) - if err != nil { - return nil, nil, err - } - - v := new(ChangeInfo) - - resp, err := s.client.Do(req, v) - if resp.StatusCode == http.StatusConflict { - body, _ := ioutil.ReadAll(resp.Body) - err = errors.New(string(body[:])) - } - return v, resp, err + return s.change("abandon", changeID, input) } // RebaseChange rebases a change. @@ -745,21 +735,7 @@ func (s *ChangesService) AbandonChange(changeID string, input *AbandonInput) (*C // // Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#rebase-change func (s *ChangesService) RebaseChange(changeID string, input *RebaseInput) (*ChangeInfo, *Response, error) { - u := fmt.Sprintf("changes/%s/rebase", changeID) - - req, err := s.client.NewRequest("POST", u, input) - if err != nil { - return nil, nil, err - } - - v := new(ChangeInfo) - - resp, err := s.client.Do(req, v) - if resp.StatusCode == http.StatusConflict { - body, _ := ioutil.ReadAll(resp.Body) - err = errors.New(string(body[:])) - } - return v, resp, err + return s.change("rebase", changeID, input) } // RestoreChange restores a change. @@ -769,21 +745,7 @@ func (s *ChangesService) RebaseChange(changeID string, input *RebaseInput) (*Cha // // Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#restore-change func (s *ChangesService) RestoreChange(changeID string, input *RestoreInput) (*ChangeInfo, *Response, error) { - u := fmt.Sprintf("changes/%s/restore", changeID) - - req, err := s.client.NewRequest("POST", u, input) - if err != nil { - return nil, nil, err - } - - v := new(ChangeInfo) - - resp, err := s.client.Do(req, v) - if resp.StatusCode == http.StatusConflict { - body, _ := ioutil.ReadAll(resp.Body) - err = errors.New(string(body[:])) - } - return v, resp, err + return s.change("restore", changeID, input) } // RevertChange reverts a change. @@ -793,19 +755,5 @@ func (s *ChangesService) RestoreChange(changeID string, input *RestoreInput) (*C // // Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#revert-change func (s *ChangesService) RevertChange(changeID string, input *RevertInput) (*ChangeInfo, *Response, error) { - u := fmt.Sprintf("changes/%s/revert", changeID) - - req, err := s.client.NewRequest("POST", u, input) - if err != nil { - return nil, nil, err - } - - v := new(ChangeInfo) - - resp, err := s.client.Do(req, v) - if resp.StatusCode == http.StatusConflict { - body, _ := ioutil.ReadAll(resp.Body) - err = errors.New(string(body[:])) - } - return v, resp, err + return s.change("revert", changeID, input) } From 6c23053f05dc9c5181610e69d66409cd3d87c017 Mon Sep 17 00:00:00 2001 From: Oliver Palmer Date: Thu, 14 Sep 2017 08:15:58 -0400 Subject: [PATCH 8/8] fix tests for Go 1.6 --- changes_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/changes_test.go b/changes_test.go index ddb1b97..9f7c97d 100644 --- a/changes_test.go +++ b/changes_test.go @@ -113,7 +113,7 @@ func TestChangesService_SubmitChange_Conflict(t *testing.T) { } _, response, _ := client.Changes.SubmitChange("123", nil) if response.StatusCode != http.StatusConflict { - t.Error() + t.Error("Expected 409 code") } } @@ -151,7 +151,7 @@ func TestChangesService_AbandonChange_Conflict(t *testing.T) { } _, response, _ := client.Changes.AbandonChange("123", nil) if response.StatusCode != http.StatusConflict { - t.Error() + t.Error("Expected 409 code") } } @@ -189,7 +189,7 @@ func TestChangesService_RebaseChange_Conflict(t *testing.T) { } _, response, _ := client.Changes.RebaseChange("123", nil) if response.StatusCode != http.StatusConflict { - t.Error() + t.Error("Expected 409 code") } } @@ -227,7 +227,7 @@ func TestChangesService_RestoreChange_Conflict(t *testing.T) { } _, response, _ := client.Changes.RestoreChange("123", nil) if response.StatusCode != http.StatusConflict { - t.Error() + t.Error("Expected 409 code") } } @@ -265,6 +265,6 @@ func TestChangesService_RevertChange_Conflict(t *testing.T) { } _, response, _ := client.Changes.RevertChange("123", nil) if response.StatusCode != http.StatusConflict { - t.Error() + t.Error("Expected 409 code") } }