Commit 13faed5500c5ab03d3c71b995d9413f6ed27cbf8
Exists in
master
Merge pull request #1061 from mfrederickson/close-issue-feature
support #371 close issue
Showing
8 changed files
with
78 additions
and
8 deletions
Show diff stats
Gemfile.lock
@@ -40,7 +40,7 @@ GEM | @@ -40,7 +40,7 @@ GEM | ||
40 | minitest (~> 5.1) | 40 | minitest (~> 5.1) |
41 | thread_safe (~> 0.3, >= 0.3.4) | 41 | thread_safe (~> 0.3, >= 0.3.4) |
42 | tzinfo (~> 1.1) | 42 | tzinfo (~> 1.1) |
43 | - addressable (2.3.8) | 43 | + addressable (2.4.0) |
44 | airbrake (4.3.5) | 44 | airbrake (4.3.5) |
45 | builder | 45 | builder |
46 | multi_json | 46 | multi_json |
@@ -124,14 +124,14 @@ GEM | @@ -124,14 +124,14 @@ GEM | ||
124 | email_spec (1.6.0) | 124 | email_spec (1.6.0) |
125 | launchy (~> 2.1) | 125 | launchy (~> 2.1) |
126 | mail (~> 2.2) | 126 | mail (~> 2.2) |
127 | - errbit_github_plugin (0.2.0) | 127 | + errbit_github_plugin (0.3.0) |
128 | errbit_plugin | 128 | errbit_plugin |
129 | octokit | 129 | octokit |
130 | - errbit_plugin (0.5.0) | 130 | + errbit_plugin (0.6.0) |
131 | erubis (2.7.0) | 131 | erubis (2.7.0) |
132 | execjs (2.6.0) | 132 | execjs (2.6.0) |
133 | fabrication (2.13.2) | 133 | fabrication (2.13.2) |
134 | - faraday (0.9.1) | 134 | + faraday (0.9.2) |
135 | multipart-post (>= 1.2, < 3) | 135 | multipart-post (>= 1.2, < 3) |
136 | ffi (1.9.8-java) | 136 | ffi (1.9.8-java) |
137 | flowdock (0.6.0) | 137 | flowdock (0.6.0) |
@@ -220,8 +220,8 @@ GEM | @@ -220,8 +220,8 @@ GEM | ||
220 | multi_json (~> 1.3) | 220 | multi_json (~> 1.3) |
221 | multi_xml (~> 0.5) | 221 | multi_xml (~> 0.5) |
222 | rack (~> 1.2) | 222 | rack (~> 1.2) |
223 | - octokit (3.8.0) | ||
224 | - sawyer (~> 0.6.0, >= 0.5.3) | 223 | + octokit (4.3.0) |
224 | + sawyer (~> 0.7.0, >= 0.5.3) | ||
225 | omniauth (1.2.2) | 225 | omniauth (1.2.2) |
226 | hashie (>= 1.2, < 4) | 226 | hashie (>= 1.2, < 4) |
227 | rack (~> 1.0) | 227 | rack (~> 1.0) |
@@ -360,8 +360,8 @@ GEM | @@ -360,8 +360,8 @@ GEM | ||
360 | sprockets (>= 2.8, < 4.0) | 360 | sprockets (>= 2.8, < 4.0) |
361 | sprockets-rails (>= 2.0, < 4.0) | 361 | sprockets-rails (>= 2.0, < 4.0) |
362 | tilt (~> 1.1) | 362 | tilt (~> 1.1) |
363 | - sawyer (0.6.0) | ||
364 | - addressable (~> 2.3.5) | 363 | + sawyer (0.7.0) |
364 | + addressable (>= 2.3.5, < 2.5) | ||
365 | faraday (~> 0.8, < 0.10) | 365 | faraday (~> 0.8, < 0.10) |
366 | simplecov (0.10.0) | 366 | simplecov (0.10.0) |
367 | docile (~> 1.1.0) | 367 | docile (~> 1.1.0) |
app/controllers/problems_controller.rb
@@ -54,6 +54,13 @@ class ProblemsController < ApplicationController | @@ -54,6 +54,13 @@ class ProblemsController < ApplicationController | ||
54 | @comment = Comment.new | 54 | @comment = Comment.new |
55 | end | 55 | end |
56 | 56 | ||
57 | + def close_issue | ||
58 | + issue = Issue.new(problem: problem, user: current_user) | ||
59 | + flash[:error] = issue.errors.full_messages.join(', ') unless issue.close | ||
60 | + | ||
61 | + redirect_to app_problem_path(app, problem) | ||
62 | + end | ||
63 | + | ||
57 | def create_issue | 64 | def create_issue |
58 | issue = Issue.new(problem: problem, user: current_user) | 65 | issue = Issue.new(problem: problem, user: current_user) |
59 | issue.body = render_to_string(*issue.render_body_args) | 66 | issue.body = render_to_string(*issue.render_body_args) |
app/models/issue.rb
@@ -26,6 +26,23 @@ class Issue | @@ -26,6 +26,23 @@ class Issue | ||
26 | end | 26 | end |
27 | end | 27 | end |
28 | 28 | ||
29 | + def close | ||
30 | + errors.add :base, "This app has no issue tracker" unless issue_tracker | ||
31 | + return false if errors.present? | ||
32 | + | ||
33 | + tracker.errors.each { |k, err| errors.add k, err } | ||
34 | + return false if errors.present? | ||
35 | + | ||
36 | + if issue_tracker.respond_to? :close_issue | ||
37 | + issue_tracker.close_issue(problem.issue_link, user: user.as_document) | ||
38 | + end | ||
39 | + | ||
40 | + errors.empty? | ||
41 | + rescue => ex | ||
42 | + errors.add :base, "There was an error during issue closing: #{ex.message}" | ||
43 | + false | ||
44 | + end | ||
45 | + | ||
29 | def save | 46 | def save |
30 | errors.add :base, "The issue has no body" unless body | 47 | errors.add :base, "The issue has no body" unless body |
31 | errors.add :base, "This app has no issue tracker" unless issue_tracker | 48 | errors.add :base, "This app has no issue tracker" unless issue_tracker |
app/models/issue_tracker.rb
@@ -36,5 +36,6 @@ class IssueTracker | @@ -36,5 +36,6 @@ class IssueTracker | ||
36 | 36 | ||
37 | delegate :configured?, to: :tracker | 37 | delegate :configured?, to: :tracker |
38 | delegate :create_issue, to: :tracker | 38 | delegate :create_issue, to: :tracker |
39 | + delegate :close_issue, to: :tracker | ||
39 | delegate :url, to: :tracker | 40 | delegate :url, to: :tracker |
40 | end | 41 | end |
app/views/problems/_issue_tracker_links.html.haml
@@ -10,6 +10,11 @@ | @@ -10,6 +10,11 @@ | ||
10 | = link_to unlink_issue_app_problem_path(app, problem), :method => :delete, :data => { :confirm => "Unlink err issues?" }, :class => "unlink-issue issue-tracker-button" do | 10 | = link_to unlink_issue_app_problem_path(app, problem), :method => :delete, :data => { :confirm => "Unlink err issues?" }, :class => "unlink-issue issue-tracker-button" do |
11 | %img.button-icon{"src" => tracker_type.icons[:create]} | 11 | %img.button-icon{"src" => tracker_type.icons[:create]} |
12 | unlink issue | 12 | unlink issue |
13 | + - if tracker.tracker.respond_to? :close_issue | ||
14 | + %span | ||
15 | + = link_to close_issue_app_problem_path(app, problem), method: :post, :class => "close-issue" do | ||
16 | + %img.button-icon{"src" => tracker_type.icons[:create]} | ||
17 | + close issue | ||
13 | - elsif problem.issue_link == "pending" | 18 | - elsif problem.issue_link == "pending" |
14 | %span.disabled | 19 | %span.disabled |
15 | = link_to '#', :class => "create-issue issue-tracker-button" do | 20 | = link_to '#', :class => "create-issue issue-tracker-button" do |
config/routes.rb
@@ -42,6 +42,7 @@ Rails.application.routes.draw do | @@ -42,6 +42,7 @@ Rails.application.routes.draw do | ||
42 | put :resolve | 42 | put :resolve |
43 | put :unresolve | 43 | put :unresolve |
44 | post :create_issue | 44 | post :create_issue |
45 | + post :close_issue | ||
45 | delete :unlink_issue | 46 | delete :unlink_issue |
46 | end | 47 | end |
47 | end | 48 | end |
spec/controllers/problems_controller_spec.rb
@@ -262,6 +262,40 @@ describe ProblemsController, type: 'controller' do | @@ -262,6 +262,40 @@ describe ProblemsController, type: 'controller' do | ||
262 | end | 262 | end |
263 | end | 263 | end |
264 | 264 | ||
265 | + describe "POST /apps/:app_id/problems/:id/close_issue" do | ||
266 | + before { sign_in user } | ||
267 | + | ||
268 | + context "when app has a issue tracker" do | ||
269 | + let(:notice) { NoticeDecorator.new(Fabricate :notice) } | ||
270 | + let(:problem) { ProblemDecorator.new(notice.problem) } | ||
271 | + let(:issue_tracker) do | ||
272 | + Fabricate(:issue_tracker).tap do |t| | ||
273 | + t.instance_variable_set(:@tracker, ErrbitPlugin::MockIssueTracker.new(t.options)) | ||
274 | + end | ||
275 | + end | ||
276 | + | ||
277 | + before do | ||
278 | + problem.app.issue_tracker = issue_tracker | ||
279 | + allow(controller).to receive(:problem).and_return(problem) | ||
280 | + allow(controller).to receive(:current_user).and_return(user) | ||
281 | + end | ||
282 | + | ||
283 | + it "should redirect to problem page" do | ||
284 | + post :close_issue, app_id: problem.app.id, id: problem.id | ||
285 | + expect(response).to redirect_to(app_problem_path(problem.app, problem)) | ||
286 | + expect(flash[:error]).to be_blank | ||
287 | + end | ||
288 | + end | ||
289 | + | ||
290 | + context "when app has no issue tracker" do | ||
291 | + it "should redirect to problem page" do | ||
292 | + post :close_issue, app_id: problem.app.id, id: problem.id | ||
293 | + expect(response).to redirect_to(app_problem_path(problem.app, problem)) | ||
294 | + expect(flash[:error]).to eql "This app has no issue tracker" | ||
295 | + end | ||
296 | + end | ||
297 | + end | ||
298 | + | ||
265 | describe "DELETE /apps/:app_id/problems/:id/unlink_issue" do | 299 | describe "DELETE /apps/:app_id/problems/:id/unlink_issue" do |
266 | before(:each) do | 300 | before(:each) do |
267 | sign_in user | 301 | sign_in user |
spec/errbit_plugin/mock_issue_tracker.rb
@@ -38,6 +38,11 @@ module ErrbitPlugin | @@ -38,6 +38,11 @@ module ErrbitPlugin | ||
38 | "http://example.com/mock-errbit" | 38 | "http://example.com/mock-errbit" |
39 | end | 39 | end |
40 | 40 | ||
41 | + def close_issue(url, user) | ||
42 | + @output << [url, user] | ||
43 | + "http://example.com/mock-errbit" | ||
44 | + end | ||
45 | + | ||
41 | def url | 46 | def url |
42 | '' | 47 | '' |
43 | end | 48 | end |