Commit 5a454d1159699ad479d9c3123a18d5df24691549
Exists in
master
and in
1 other branch
Merge pull request #774 from errbit/refacor_issue_tracker
Refacorting issue tracker
Showing
19 changed files
with
314 additions
and
252 deletions
Show diff stats
Gemfile
@@ -27,8 +27,8 @@ gem 'rails_autolink' | @@ -27,8 +27,8 @@ gem 'rails_autolink' | ||
27 | gem 'hoptoad_notifier', "~> 2.4" | 27 | gem 'hoptoad_notifier', "~> 2.4" |
28 | gem 'draper', :require => false | 28 | gem 'draper', :require => false |
29 | 29 | ||
30 | -gem 'errbit_plugin' | ||
31 | -gem 'errbit_github_plugin' | 30 | +gem 'errbit_plugin', github: 'errbit/errbit_plugin' |
31 | +gem 'errbit_github_plugin', github: 'errbit/errbit_github_plugin' | ||
32 | 32 | ||
33 | # Notification services | 33 | # Notification services |
34 | # --------------------------------------- | 34 | # --------------------------------------- |
Gemfile.lock
1 | +GIT | ||
2 | + remote: git://github.com/errbit/errbit_github_plugin.git | ||
3 | + revision: 5900200e6d460fe94feaea3e59824437d54f1029 | ||
4 | + specs: | ||
5 | + errbit_github_plugin (0.1.0) | ||
6 | + errbit_plugin | ||
7 | + octokit | ||
8 | + | ||
9 | +GIT | ||
10 | + remote: git://github.com/errbit/errbit_plugin.git | ||
11 | + revision: 6a0b93d71a6b4545198f0c68b99b8e8f9281c17e | ||
12 | + specs: | ||
13 | + errbit_plugin (0.4.0) | ||
14 | + | ||
1 | GEM | 15 | GEM |
2 | remote: https://rubygems.org/ | 16 | remote: https://rubygems.org/ |
3 | specs: | 17 | specs: |
@@ -88,10 +102,6 @@ GEM | @@ -88,10 +102,6 @@ GEM | ||
88 | email_spec (1.5.0) | 102 | email_spec (1.5.0) |
89 | launchy (~> 2.1) | 103 | launchy (~> 2.1) |
90 | mail (~> 2.2) | 104 | mail (~> 2.2) |
91 | - errbit_github_plugin (0.1.0) | ||
92 | - errbit_plugin | ||
93 | - octokit | ||
94 | - errbit_plugin (0.4.0) | ||
95 | erubis (2.7.0) | 105 | erubis (2.7.0) |
96 | execjs (2.0.2) | 106 | execjs (2.0.2) |
97 | fabrication (2.9.0) | 107 | fabrication (2.9.0) |
@@ -181,8 +191,8 @@ GEM | @@ -181,8 +191,8 @@ GEM | ||
181 | jwt (~> 0.1.4) | 191 | jwt (~> 0.1.4) |
182 | multi_json (~> 1.0) | 192 | multi_json (~> 1.0) |
183 | rack (~> 1.2) | 193 | rack (~> 1.2) |
184 | - octokit (3.3.1) | ||
185 | - sawyer (~> 0.5.3) | 194 | + octokit (3.7.0) |
195 | + sawyer (~> 0.6.0, >= 0.5.3) | ||
186 | omniauth (1.1.4) | 196 | omniauth (1.1.4) |
187 | hashie (>= 1.2, < 3) | 197 | hashie (>= 1.2, < 3) |
188 | rack | 198 | rack |
@@ -267,7 +277,7 @@ GEM | @@ -267,7 +277,7 @@ GEM | ||
267 | json | 277 | json |
268 | rest-client | 278 | rest-client |
269 | safe_yaml (1.0.4) | 279 | safe_yaml (1.0.4) |
270 | - sawyer (0.5.5) | 280 | + sawyer (0.6.0) |
271 | addressable (~> 2.3.5) | 281 | addressable (~> 2.3.5) |
272 | faraday (~> 0.8, < 0.10) | 282 | faraday (~> 0.8, < 0.10) |
273 | simplecov (0.7.1) | 283 | simplecov (0.7.1) |
@@ -334,8 +344,8 @@ DEPENDENCIES | @@ -334,8 +344,8 @@ DEPENDENCIES | ||
334 | devise | 344 | devise |
335 | draper | 345 | draper |
336 | email_spec | 346 | email_spec |
337 | - errbit_github_plugin | ||
338 | - errbit_plugin | 347 | + errbit_github_plugin! |
348 | + errbit_plugin! | ||
339 | execjs | 349 | execjs |
340 | fabrication | 350 | fabrication |
341 | flowdock | 351 | flowdock |
app/controllers/problems_controller.rb
@@ -6,7 +6,6 @@ | @@ -6,7 +6,6 @@ | ||
6 | # COLLECTION => :index, :all, :destroy_several, :resolve_several, :unresolve_several, :merge_several, :unmerge_several, :search | 6 | # COLLECTION => :index, :all, :destroy_several, :resolve_several, :unresolve_several, :merge_several, :unmerge_several, :search |
7 | class ProblemsController < ApplicationController | 7 | class ProblemsController < ApplicationController |
8 | 8 | ||
9 | - | ||
10 | include ProblemsSearcher | 9 | include ProblemsSearcher |
11 | 10 | ||
12 | before_filter :need_selected_problem, :only => [ | 11 | before_filter :need_selected_problem, :only => [ |
@@ -62,10 +61,12 @@ class ProblemsController < ApplicationController | @@ -62,10 +61,12 @@ class ProblemsController < ApplicationController | ||
62 | end | 61 | end |
63 | 62 | ||
64 | def create_issue | 63 | def create_issue |
65 | - issue_creation = IssueCreation.new(problem, current_user, params[:tracker], request) | 64 | + body = render_to_string "issue_trackers/issue", layout: false, formats: [:txt] |
65 | + title = "[#{ problem.environment }][#{ problem.where }] #{problem.message.to_s.truncate(100)}" | ||
66 | 66 | ||
67 | - unless issue_creation.execute | ||
68 | - flash[:error] = issue_creation.errors.full_messages.join(', ') | 67 | + issue = Issue.new(problem: problem, user: current_user, title: title, body: body) |
68 | + unless issue.save | ||
69 | + flash[:error] = issue.errors.full_messages.join(', ') | ||
69 | end | 70 | end |
70 | 71 | ||
71 | redirect_to app_problem_path(app, problem) | 72 | redirect_to app_problem_path(app, problem) |
app/interactors/issue_creation.rb
@@ -1,55 +0,0 @@ | @@ -1,55 +0,0 @@ | ||
1 | -class IssueCreation | ||
2 | - include ActiveModel::Validations | ||
3 | - | ||
4 | - attr_reader :problem, :user, :tracker_name | ||
5 | - | ||
6 | - delegate :app, :to => :problem | ||
7 | - | ||
8 | - def initialize(problem, user, tracker_name, request) | ||
9 | - @problem = problem | ||
10 | - @user = user | ||
11 | - @tracker_name = tracker_name | ||
12 | - IssueTracker.update_url_options(request) | ||
13 | - end | ||
14 | - | ||
15 | - def execute | ||
16 | - tracker.create_issue(problem, user) if tracker | ||
17 | - errors.empty? | ||
18 | - rescue => ex | ||
19 | - Rails.logger.error "Error during issue creation: " << ex.message | ||
20 | - errors.add :base, "There was an error during issue creation: #{ex.message}" | ||
21 | - false | ||
22 | - end | ||
23 | - | ||
24 | - private | ||
25 | - | ||
26 | - def tracker | ||
27 | - return @tracker if @tracker | ||
28 | - | ||
29 | - # Create an issue on GitHub using user's github token | ||
30 | - if tracker_name == 'user_github' | ||
31 | - if !app.github_repo? | ||
32 | - errors.add :base, "This app doesn't have a GitHub repo set up." | ||
33 | - elsif !user.github_account? | ||
34 | - errors.add :base, "You haven't linked your Github account." | ||
35 | - else | ||
36 | - @tracker = ErrbitGithubPlugin::IssueTracker.new( | ||
37 | - app, { | ||
38 | - :username => user.github_login, | ||
39 | - :oauth_token => user.github_oauth_token | ||
40 | - } | ||
41 | - ) | ||
42 | - end | ||
43 | - | ||
44 | - # Or, create an issue using the App's issue tracker | ||
45 | - elsif app.issue_tracker_configured? | ||
46 | - @tracker = app.issue_tracker | ||
47 | - | ||
48 | - # Otherwise, display error about missing tracker configuration. | ||
49 | - else | ||
50 | - errors.add :base, "This app has no issue tracker setup." | ||
51 | - end | ||
52 | - | ||
53 | - @tracker | ||
54 | - end | ||
55 | -end |
@@ -0,0 +1,31 @@ | @@ -0,0 +1,31 @@ | ||
1 | +class Issue | ||
2 | + include ActiveModel::Model | ||
3 | + attr_accessor :problem, :user, :title, :body | ||
4 | + | ||
5 | + def intialize(problem: nil, user: nil, title: nil, body: nil) | ||
6 | + @problem, @user, @title, @body = problem, user, title, body | ||
7 | + end | ||
8 | + | ||
9 | + def issue_tracker | ||
10 | + problem.app.issue_tracker | ||
11 | + end | ||
12 | + | ||
13 | + def save | ||
14 | + if issue_tracker | ||
15 | + issue_tracker.tracker.errors.each do |k, err| | ||
16 | + errors.add k, err | ||
17 | + end | ||
18 | + return false if errors.present? | ||
19 | + | ||
20 | + url = issue_tracker.create_issue(title, body, user: user.as_document) | ||
21 | + problem.update_attributes(issue_link: url, issue_type: issue_tracker.tracker.class.label) | ||
22 | + else | ||
23 | + errors.add :base, "This app has no issue tracker setup." | ||
24 | + end | ||
25 | + | ||
26 | + errors.empty? | ||
27 | + rescue => ex | ||
28 | + errors.add :base, "There was an error during issue creation: #{ex.message}" | ||
29 | + false | ||
30 | + end | ||
31 | +end |
app/models/issue_tracker.rb
@@ -2,11 +2,6 @@ class IssueTracker | @@ -2,11 +2,6 @@ class IssueTracker | ||
2 | include Mongoid::Document | 2 | include Mongoid::Document |
3 | include Mongoid::Timestamps | 3 | include Mongoid::Timestamps |
4 | 4 | ||
5 | - include Rails.application.routes.url_helpers | ||
6 | - | ||
7 | - default_url_options[:host] = ActionMailer::Base.default_url_options[:host] | ||
8 | - default_url_options[:port] = ActionMailer::Base.default_url_options[:port] | ||
9 | - | ||
10 | embedded_in :app, :inverse_of => :issue_tracker | 5 | embedded_in :app, :inverse_of => :issue_tracker |
11 | 6 | ||
12 | field :type_tracker, :type => String | 7 | field :type_tracker, :type => String |
@@ -14,22 +9,12 @@ class IssueTracker | @@ -14,22 +9,12 @@ class IssueTracker | ||
14 | 9 | ||
15 | validate :validate_tracker | 10 | validate :validate_tracker |
16 | 11 | ||
17 | - ## | ||
18 | - # Update default_url_option with valid data from the request information | ||
19 | - # | ||
20 | - # @param [ Request ] a request with host, port and protocol | ||
21 | - # | ||
22 | - def self.update_url_options(request) | ||
23 | - IssueTracker.default_url_options[:host] = request.host | ||
24 | - IssueTracker.default_url_options[:port] = request.port | ||
25 | - IssueTracker.default_url_options[:protocol] = request.scheme | ||
26 | - end | ||
27 | - | ||
28 | def tracker | 12 | def tracker |
29 | - klass = ErrbitPlugin::Registry.issue_trackers[self.type_tracker] | ||
30 | - klass = ErrbitPlugin::NoneIssueTracker unless klass | ||
31 | - | ||
32 | - @tracker = klass.new(app, self.options) | 13 | + @tracker ||= |
14 | + begin | ||
15 | + klass = ErrbitPlugin::Registry.issue_trackers[self.type_tracker] || ErrbitPlugin::NoneIssueTracker | ||
16 | + klass.new(options.merge(github_repo: app.github_repo, bitbucket_repo: app.bitbucket_repo)) | ||
17 | + end | ||
33 | end | 18 | end |
34 | 19 | ||
35 | # Allow the tracker to validate its own params | 20 | # Allow the tracker to validate its own params |
@@ -0,0 +1,45 @@ | @@ -0,0 +1,45 @@ | ||
1 | +[See this exception on Errbit](<%= app_problem_url problem.app, problem %> "See this exception on Errbit") | ||
2 | +<% if notice = problem.notices.first %> | ||
3 | +# <%= notice.message %> # | ||
4 | +## Summary ## | ||
5 | +<% if notice.request['url'].present? %> | ||
6 | +### URL ### | ||
7 | +[<%= notice.request['url'] %>](<%= notice.request['url'] %>)" | ||
8 | +<% end %> | ||
9 | +### Where ### | ||
10 | +<%= notice.where %> | ||
11 | + | ||
12 | +### Occured ### | ||
13 | +<%= notice.created_at.to_s(:micro) %> | ||
14 | + | ||
15 | +### Similar ### | ||
16 | +<%= (notice.problem.notices_count - 1).to_s %> | ||
17 | + | ||
18 | +## Params ## | ||
19 | +``` | ||
20 | +<%= pretty_hash(notice.params) %> | ||
21 | +``` | ||
22 | + | ||
23 | +## Session ## | ||
24 | +``` | ||
25 | +<%= pretty_hash(notice.session) %> | ||
26 | +``` | ||
27 | + | ||
28 | +## Backtrace ## | ||
29 | +``` | ||
30 | +<% notice.backtrace_lines.each do |line| %><%= line.number %>: <%= line.file_relative %> -> **<%= line.method %>** | ||
31 | +<% end %> | ||
32 | +``` | ||
33 | + | ||
34 | +## Environment ## | ||
35 | + | ||
36 | +<table> | ||
37 | +<% for key, val in notice.env_vars %> | ||
38 | + <tr> | ||
39 | + <td><%= key %>:</td> | ||
40 | + <td><%= val %></td> | ||
41 | + </tr> | ||
42 | +<% end %> | ||
43 | +</table> | ||
44 | +<% end %> | ||
45 | + |
app/views/problems/_issue_tracker_links.html.haml
1 | -- if app.issue_tracker_configured? || current_user.github_account? | 1 | +- if app.issue_tracker_configured? |
2 | - if problem.issue_link.present? | 2 | - if problem.issue_link.present? |
3 | %span= link_to 'go to issue', problem.issue_link, :class => "#{problem.issue_type}_goto goto-issue" | 3 | %span= link_to 'go to issue', problem.issue_link, :class => "#{problem.issue_type}_goto goto-issue" |
4 | = link_to 'unlink issue', unlink_issue_app_problem_path(app, problem), :method => :delete, :data => { :confirm => "Unlink err issues?" }, :class => "unlink-issue" | 4 | = link_to 'unlink issue', unlink_issue_app_problem_path(app, problem), :method => :delete, :data => { :confirm => "Unlink err issues?" }, :class => "unlink-issue" |
@@ -6,10 +6,4 @@ | @@ -6,10 +6,4 @@ | ||
6 | %span.disabled= link_to 'creating...', '#', :class => "#{problem.issue_type}_inactive create-issue" | 6 | %span.disabled= link_to 'creating...', '#', :class => "#{problem.issue_type}_inactive create-issue" |
7 | = link_to 'retry', create_issue_app_problem_path(app, problem), :method => :post | 7 | = link_to 'retry', create_issue_app_problem_path(app, problem), :method => :post |
8 | - else | 8 | - else |
9 | - - if app.issue_tracker_configured? && !app.issue_tracker.type_tracker.eql?('github') | ||
10 | - %span= link_to 'create issue', create_issue_app_problem_path(app, problem), :method => :post, :class => "#{app.issue_tracker.type_tracker}_create create-issue" | ||
11 | - - elsif app.github_repo? | ||
12 | - - if current_user.can_create_github_issues? | ||
13 | - %span= link_to 'create issue', create_issue_app_problem_path(app, problem, :tracker => 'user_github'), :method => :post, :class => "github_create create-issue" | ||
14 | - - elsif app.issue_tracker_configured? && app.issue_tracker.type_tracker.eql?('github') | ||
15 | - %span= link_to 'create issue', create_issue_app_problem_path(app, problem), :method => :post, :class => "github_create create-issue" | 9 | + %span= link_to 'create issue', create_issue_app_problem_path(app, problem), method: :post, class: "#{app.issue_tracker.type_tracker}_create create-issue" |
spec/acceptance/app_regenerate_api_key_spec.rb
@@ -73,6 +73,8 @@ feature "Create an application" do | @@ -73,6 +73,8 @@ feature "Create an application" do | ||
73 | click_on I18n.t('apps.index.new_app') | 73 | click_on I18n.t('apps.index.new_app') |
74 | fill_in 'app_name', :with => 'My new app' | 74 | fill_in 'app_name', :with => 'My new app' |
75 | find('.label_radio.github').click | 75 | find('.label_radio.github').click |
76 | + | ||
77 | + fill_in 'app_github_repo', with: 'foo/bar' | ||
76 | within ".github.tracker_params" do | 78 | within ".github.tracker_params" do |
77 | fill_in 'app_issue_tracker_attributes_options_username', :with => 'token' | 79 | fill_in 'app_issue_tracker_attributes_options_username', :with => 'token' |
78 | fill_in 'app_issue_tracker_attributes_options_password', :with => 'pass' | 80 | fill_in 'app_issue_tracker_attributes_options_password', :with => 'pass' |
spec/controllers/apps_controller_spec.rb
@@ -308,42 +308,6 @@ describe AppsController do | @@ -308,42 +308,6 @@ describe AppsController do | ||
308 | expect(@app.issue_tracker_configured?).to eq false | 308 | expect(@app.issue_tracker_configured?).to eq false |
309 | end | 309 | end |
310 | end | 310 | end |
311 | - | ||
312 | - ErrbitPlugin::Registry.issue_trackers.each do |key, klass| | ||
313 | - context key do | ||
314 | - it "should save tracker params" do | ||
315 | - params = { | ||
316 | - :options => klass.fields.inject({}){|hash,f| hash[f[0]] = "test_value"; hash }, | ||
317 | - :type_tracker => key.dup.to_s | ||
318 | - } | ||
319 | - put :update, :id => @app.id, :app => {:issue_tracker_attributes => params} | ||
320 | - | ||
321 | - @app.reload | ||
322 | - | ||
323 | - tracker = @app.issue_tracker | ||
324 | - expect(tracker.tracker).to be_a(ErrbitPlugin::Registry.issue_trackers[key]) | ||
325 | - klass.fields.each do |field, field_info| | ||
326 | - case field | ||
327 | - when :ticket_properties; tracker.send(field.to_sym).should == 'card_type = defect' | ||
328 | - else tracker.options[field.to_s].should == 'test_value' | ||
329 | - end | ||
330 | - end | ||
331 | - end | ||
332 | - | ||
333 | - it "should show validation notice when sufficient params are not present" do | ||
334 | - # Leave out one required param | ||
335 | - # TODO. previous test was not relevant because one params can be enough. So put noone | ||
336 | - put :update, :id => @app.id, :app => { | ||
337 | - :issue_tracker_attributes => { | ||
338 | - :type_tracker => key.dup.to_s | ||
339 | - } | ||
340 | - } | ||
341 | - | ||
342 | - @app.reload | ||
343 | - expect(@app.issue_tracker_configured?).to eq false | ||
344 | - end | ||
345 | - end | ||
346 | - end | ||
347 | end | 311 | end |
348 | end | 312 | end |
349 | 313 |
spec/controllers/problems_controller_spec.rb
@@ -12,9 +12,7 @@ describe ProblemsController do | @@ -12,9 +12,7 @@ describe ProblemsController do | ||
12 | let(:admin) { Fabricate(:admin) } | 12 | let(:admin) { Fabricate(:admin) } |
13 | let(:problem) { Fabricate(:problem, :app => app, :environment => "production") } | 13 | let(:problem) { Fabricate(:problem, :app => app, :environment => "production") } |
14 | 14 | ||
15 | - | ||
16 | describe "GET /problems" do | 15 | describe "GET /problems" do |
17 | - #render_views | ||
18 | context 'when logged in as an admin' do | 16 | context 'when logged in as an admin' do |
19 | before(:each) do | 17 | before(:each) do |
20 | sign_in admin | 18 | sign_in admin |
@@ -257,45 +255,66 @@ describe ProblemsController do | @@ -257,45 +255,66 @@ describe ProblemsController do | ||
257 | end | 255 | end |
258 | 256 | ||
259 | describe "POST /apps/:app_id/problems/:id/create_issue" do | 257 | describe "POST /apps/:app_id/problems/:id/create_issue" do |
258 | + before { sign_in admin } | ||
259 | + | ||
260 | + context "when app has a issue tracker" do | ||
261 | + let(:notice) { Fabricate :notice } | ||
262 | + let(:problem) { notice.problem } | ||
263 | + let(:issue_tracker) do | ||
264 | + Fabricate(:issue_tracker).tap do |t| | ||
265 | + t.instance_variable_set(:@tracker, ErrbitPlugin::MockIssueTracker.new(t.options)) | ||
266 | + end | ||
267 | + end | ||
260 | 268 | ||
261 | - before(:each) do | ||
262 | - sign_in admin | ||
263 | - end | 269 | + before do |
270 | + problem.app.issue_tracker = issue_tracker | ||
271 | + controller.stub(:problem).and_return(problem) | ||
272 | + controller.stub(:current_user).and_return(admin) | ||
273 | + end | ||
264 | 274 | ||
265 | - context "successful issue creation" do | ||
266 | - context "lighthouseapp tracker" do | ||
267 | - let(:notice) { Fabricate :notice } | ||
268 | - let(:problem) { notice.problem } | 275 | + it "should redirect to problem page" do |
276 | + post :create_issue, app_id: problem.app.id, id: problem.id | ||
277 | + expect(response).to redirect_to(app_problem_path(problem.app, problem)) | ||
278 | + expect(flash[:error]).to be_blank | ||
279 | + end | ||
269 | 280 | ||
270 | - before(:each) do | ||
271 | - controller.stub(:problem).and_return(problem) | ||
272 | - controller.stub(:current_user).and_return(admin) | ||
273 | - IssueCreation.should_receive(:new).with(problem, admin, nil, request).and_return(double(:execute => true)) | ||
274 | - post :create_issue, :app_id => problem.app.id, :id => problem.id | ||
275 | - end | 281 | + it "should save the right title" do |
282 | + post :create_issue, app_id: problem.app.id, id: problem.id | ||
283 | + title = "[#{ problem.environment }][#{ problem.where }] #{problem.message.to_s.truncate(100)}" | ||
284 | + line = issue_tracker.tracker.output.shift | ||
285 | + expect(line[0]).to eq(title) | ||
286 | + end | ||
276 | 287 | ||
277 | - it "should redirect to problem page" do | ||
278 | - expect(response).to redirect_to( app_problem_path(problem.app, problem) ) | ||
279 | - expect(flash[:error]).to be_blank | ||
280 | - end | 288 | + it "should renders the issue body" do |
289 | + post :create_issue, app_id: problem.app.id, id: problem.id, format: 'html' | ||
290 | + expect(response).to render_template("issue_trackers/issue") | ||
281 | end | 291 | end |
282 | - end | ||
283 | 292 | ||
284 | - context "error during request to a tracker" do | ||
285 | - before(:each) do | ||
286 | - IssueCreation.should_receive(:new).with(problem, admin, nil, request).and_return( | ||
287 | - double(:execute => false, :errors => double(:full_messages => ['not create'])) | ||
288 | - ) | ||
289 | - controller.stub(:problem).and_return(problem) | ||
290 | - post :create_issue, :app_id => problem.app.id, :id => problem.id | 293 | + it "should update the problem" do |
294 | + post :create_issue, app_id: problem.app.id, id: problem.id | ||
295 | + expect(problem.issue_link).to eq("http://example.com/mock-errbit") | ||
296 | + expect(problem.issue_type).to eq("mock") | ||
291 | end | 297 | end |
292 | 298 | ||
299 | + | ||
300 | + context "when rendering views" do | ||
301 | + render_views | ||
302 | + | ||
303 | + it "should save the right body" do | ||
304 | + post :create_issue, app_id: problem.app.id, id: problem.id, format: 'html' | ||
305 | + line = issue_tracker.tracker.output.shift | ||
306 | + expect(line[1]).to include(app_problem_url problem.app, problem) | ||
307 | + end | ||
308 | + end | ||
309 | + end | ||
310 | + | ||
311 | + context "when app has no issue tracker" do | ||
293 | it "should redirect to problem page" do | 312 | it "should redirect to problem page" do |
313 | + post :create_issue, app_id: problem.app.id, id: problem.id | ||
294 | expect(response).to redirect_to( app_problem_path(problem.app, problem) ) | 314 | expect(response).to redirect_to( app_problem_path(problem.app, problem) ) |
295 | - expect(flash[:error]).to eql 'not create' | 315 | + expect(flash[:error]).to eql "This app has no issue tracker setup." |
296 | end | 316 | end |
297 | end | 317 | end |
298 | - | ||
299 | end | 318 | end |
300 | 319 | ||
301 | describe "DELETE /apps/:app_id/problems/:id/unlink_issue" do | 320 | describe "DELETE /apps/:app_id/problems/:id/unlink_issue" do |
@@ -0,0 +1,45 @@ | @@ -0,0 +1,45 @@ | ||
1 | +module ErrbitPlugin | ||
2 | + class MockIssueTracker < IssueTracker | ||
3 | + def self.label | ||
4 | + 'mock' | ||
5 | + end | ||
6 | + | ||
7 | + def self.note | ||
8 | + 'A fake issue tracker to help in testing purpose' | ||
9 | + end | ||
10 | + | ||
11 | + def self.fields | ||
12 | + { | ||
13 | + :foo => {:label => 'foo'}, | ||
14 | + :bar => {:label => 'bar'} | ||
15 | + } | ||
16 | + end | ||
17 | + | ||
18 | + attr_accessor :output | ||
19 | + | ||
20 | + def initialize(*) | ||
21 | + super | ||
22 | + @output = [] | ||
23 | + end | ||
24 | + | ||
25 | + def configured? | ||
26 | + !errors.any? | ||
27 | + end | ||
28 | + | ||
29 | + def errors | ||
30 | + errors = [] | ||
31 | + errors << [:base, 'foo is required'] unless options[:foo] | ||
32 | + errors << [:base, 'bar is required'] unless options[:bar] | ||
33 | + errors | ||
34 | + end | ||
35 | + | ||
36 | + def create_issue(title, body, user) | ||
37 | + @output << [title, body, user] | ||
38 | + "http://example.com/mock-errbit" | ||
39 | + end | ||
40 | + | ||
41 | + def url; ''; end | ||
42 | + | ||
43 | + def comments_allowed?; false; end | ||
44 | + end | ||
45 | +end |
spec/fabricators/issue_tracker_fabricator.rb
spec/interactors/issue_creation_spec.rb
@@ -1,63 +0,0 @@ | @@ -1,63 +0,0 @@ | ||
1 | -require 'spec_helper' | ||
2 | - | ||
3 | -describe IssueCreation do | ||
4 | - subject(:issue_creation) do | ||
5 | - IssueCreation.new(problem, user, tracker_name, request) | ||
6 | - end | ||
7 | - | ||
8 | - let(:request) do | ||
9 | - double(:request, | ||
10 | - :host => 'github.com', | ||
11 | - :port => '80', | ||
12 | - :scheme => 'http' | ||
13 | - ) | ||
14 | - end | ||
15 | - let(:problem) { notice.problem } | ||
16 | - let(:notice) { Fabricate(:notice) } | ||
17 | - let(:user) { Fabricate(:admin) } | ||
18 | - let(:errors) { issue_creation.errors[:base] } | ||
19 | - let(:tracker_name) { nil } | ||
20 | - | ||
21 | - it "adds the error when issue tracker isn't configured" do | ||
22 | - issue_creation.execute | ||
23 | - expect(errors).to include("This app has no issue tracker setup.") | ||
24 | - end | ||
25 | - | ||
26 | - it 'creates an issue if issue tracker is configured' do | ||
27 | - problem.app.issue_tracker = Fabricate(:issue_tracker) | ||
28 | - issue_creation.execute | ||
29 | - expect(errors).to be_empty | ||
30 | - end | ||
31 | - | ||
32 | - context "with user's github" do | ||
33 | - let(:tracker_name) { 'user_github' } | ||
34 | - | ||
35 | - it "adds the error when repo isn't set up" do | ||
36 | - issue_creation.execute | ||
37 | - expect(errors).to include("This app doesn't have a GitHub repo set up.") | ||
38 | - end | ||
39 | - | ||
40 | - context 'with repo set up' do | ||
41 | - before do | ||
42 | - notice.app.update_attribute(:github_repo, 'errbit/errbit') | ||
43 | - end | ||
44 | - | ||
45 | - it "adds the error when github account isn't linked" do | ||
46 | - issue_creation.execute | ||
47 | - expect(errors).to include("You haven't linked your Github account.") | ||
48 | - end | ||
49 | - | ||
50 | - it 'creates an issue if github account is linked' do | ||
51 | - user.github_login = 'admin' | ||
52 | - user.github_oauth_token = 'oauthtoken' | ||
53 | - user.save! | ||
54 | - | ||
55 | - ErrbitGithubPlugin::IssueTracker.should_receive(:new).and_return( | ||
56 | - double(:create_issue => true) | ||
57 | - ) | ||
58 | - issue_creation.execute | ||
59 | - expect(errors).to be_empty | ||
60 | - end | ||
61 | - end | ||
62 | - end | ||
63 | -end |
@@ -0,0 +1,92 @@ | @@ -0,0 +1,92 @@ | ||
1 | +require "spec_helper" | ||
2 | + | ||
3 | +describe Issue do | ||
4 | + | ||
5 | + subject(:issue) { Issue.new(problem: problem, user: user, title: title, body: body) } | ||
6 | + | ||
7 | + let(:problem) { notice.problem } | ||
8 | + let(:notice) { Fabricate(:notice) } | ||
9 | + let(:user) { Fabricate(:admin) } | ||
10 | + let(:issue_tracker) do | ||
11 | + Fabricate(:issue_tracker).tap do |t| | ||
12 | + t.instance_variable_set(:@tracker, ErrbitPlugin::MockIssueTracker.new(t.options)) | ||
13 | + end | ||
14 | + end | ||
15 | + let(:errors) { issue.errors[:base] } | ||
16 | + | ||
17 | + context "when app has no issue tracker" do | ||
18 | + let(:title) { "Foo" } | ||
19 | + let(:body) { "barrr" } | ||
20 | + | ||
21 | + context "#save" do | ||
22 | + it "returns false" do | ||
23 | + expect(issue.save).to be false | ||
24 | + end | ||
25 | + | ||
26 | + it "returns an error" do | ||
27 | + issue.save | ||
28 | + expect(errors).to include("This app has no issue tracker setup.") | ||
29 | + end | ||
30 | + end | ||
31 | + end | ||
32 | + | ||
33 | + context "when has no title" do | ||
34 | + let(:body) { "barrr" } | ||
35 | + | ||
36 | + pending "returns an error" do | ||
37 | + end | ||
38 | + end | ||
39 | + | ||
40 | + context "when has no body" do | ||
41 | + let(:title) { "Foo" } | ||
42 | + | ||
43 | + pending "returns an error" do | ||
44 | + end | ||
45 | + end | ||
46 | + | ||
47 | + context "when app has a issue tracker" do | ||
48 | + let(:title) { "Foo" } | ||
49 | + let(:body) { "barrr" } | ||
50 | + | ||
51 | + before do | ||
52 | + problem.app.issue_tracker = issue_tracker | ||
53 | + end | ||
54 | + | ||
55 | + context "#save" do | ||
56 | + | ||
57 | + context "when issue tracker has errors" do | ||
58 | + before do | ||
59 | + issue_tracker.tracker.options.clear | ||
60 | + end | ||
61 | + | ||
62 | + it("returns false") { expect(issue.save).to be false } | ||
63 | + it "adds the errors" do | ||
64 | + issue.save | ||
65 | + expect(errors).to include("foo is required") | ||
66 | + expect(errors).to include("bar is required") | ||
67 | + end | ||
68 | + end | ||
69 | + | ||
70 | + it "creates the issue" do | ||
71 | + issue.save | ||
72 | + expect(issue_tracker.tracker.output.count).to be 1 | ||
73 | + end | ||
74 | + | ||
75 | + it "returns true" do | ||
76 | + expect(issue.save).to be true | ||
77 | + end | ||
78 | + | ||
79 | + it "sends the title" do | ||
80 | + issue.save | ||
81 | + saved_issue = issue_tracker.tracker.output.first | ||
82 | + expect(saved_issue.first).to be title | ||
83 | + end | ||
84 | + | ||
85 | + it "sends the body" do | ||
86 | + issue.save | ||
87 | + saved_issue = issue_tracker.tracker.output.first | ||
88 | + expect(saved_issue[1]).to be body | ||
89 | + end | ||
90 | + end | ||
91 | + end | ||
92 | +end |
spec/models/issue_tracker_spec.rb
@@ -12,8 +12,11 @@ describe IssueTracker do | @@ -12,8 +12,11 @@ describe IssueTracker do | ||
12 | 12 | ||
13 | describe "#tracker" do | 13 | describe "#tracker" do |
14 | context "with type_tracker class not exist" do | 14 | context "with type_tracker class not exist" do |
15 | - it 'return NullIssueTracker' do | ||
16 | - expect(IssueTracker.new(:type_tracker => 'Foo').tracker).to be_a ErrbitPlugin::NoneIssueTracker | 15 | + let(:app) { Fabricate(:app) } |
16 | + | ||
17 | + it 'return NoneIssueTracker' do | ||
18 | + issue_tracker = IssueTracker.new(type_tracker: 'Foo', app: app) | ||
19 | + expect(issue_tracker.tracker).to be_a ErrbitPlugin::NoneIssueTracker | ||
17 | end | 20 | end |
18 | end | 21 | end |
19 | end | 22 | end |
spec/models/problem_spec.rb
@@ -391,11 +391,13 @@ describe Problem do | @@ -391,11 +391,13 @@ describe Problem do | ||
391 | 391 | ||
392 | context "with issue_tracker valid associate to app" do | 392 | context "with issue_tracker valid associate to app" do |
393 | let(:issue_tracker) do | 393 | let(:issue_tracker) do |
394 | - Fabricate(:issue_tracker) | 394 | + Fabricate(:issue_tracker).tap do |t| |
395 | + t.instance_variable_set(:@tracker, ErrbitPlugin::MockIssueTracker.new(t.options)) | ||
396 | + end | ||
395 | end | 397 | end |
396 | 398 | ||
397 | it 'return the issue_tracker label' do | 399 | it 'return the issue_tracker label' do |
398 | - expect(problem.issue_type).to eql 'fake' | 400 | + expect(problem.issue_type).to eql 'mock' |
399 | end | 401 | end |
400 | end | 402 | end |
401 | 403 |
spec/spec_helper.rb
@@ -25,8 +25,7 @@ require 'webmock/rspec' | @@ -25,8 +25,7 @@ require 'webmock/rspec' | ||
25 | require 'xmpp4r' | 25 | require 'xmpp4r' |
26 | require 'xmpp4r/muc' | 26 | require 'xmpp4r/muc' |
27 | require 'mongoid-rspec' | 27 | require 'mongoid-rspec' |
28 | -require 'errbit_plugin/issue_trackers/fake' | ||
29 | - | 28 | +require 'errbit_plugin/mock_issue_tracker' |
30 | 29 | ||
31 | # Requires supporting files with custom matchers and macros, etc, | 30 | # Requires supporting files with custom matchers and macros, etc, |
32 | # in ./support/ and its subdirectories. | 31 | # in ./support/ and its subdirectories. |
spec/views/problems/show.html.haml_spec.rb
@@ -4,15 +4,13 @@ describe "problems/show.html.haml" do | @@ -4,15 +4,13 @@ describe "problems/show.html.haml" do | ||
4 | let(:problem) { Fabricate(:problem) } | 4 | let(:problem) { Fabricate(:problem) } |
5 | let(:comment) { Fabricate(:comment) } | 5 | let(:comment) { Fabricate(:comment) } |
6 | let(:pivotal_tracker) { | 6 | let(:pivotal_tracker) { |
7 | - Class.new(ErrbitPlugin::IssueTracker) do | 7 | + Class.new(ErrbitPlugin::MockIssueTracker) do |
8 | def self.label; 'pivotal'; end | 8 | def self.label; 'pivotal'; end |
9 | - def initialize(app, params); end | ||
10 | def configured?; true; end | 9 | def configured?; true; end |
11 | end | 10 | end |
12 | } | 11 | } |
13 | let(:github_tracker) { | 12 | let(:github_tracker) { |
14 | - Class.new(ErrbitPlugin::IssueTracker) do | ||
15 | - def initialize(app, params); end | 13 | + Class.new(ErrbitPlugin::MockIssueTracker) do |
16 | def label; 'github'; end | 14 | def label; 'github'; end |
17 | def configured?; true; end | 15 | def configured?; true; end |
18 | end | 16 | end |
@@ -36,8 +34,9 @@ describe "problems/show.html.haml" do | @@ -36,8 +34,9 @@ describe "problems/show.html.haml" do | ||
36 | end | 34 | end |
37 | 35 | ||
38 | def with_issue_tracker(tracker, problem) | 36 | def with_issue_tracker(tracker, problem) |
39 | - problem.app.issue_tracker = IssueTracker.new :type_tracker => tracker, :options => {:api_token => "token token token", :project_id => "1234"} | ||
40 | ErrbitPlugin::Registry.stub(:issue_trackers).and_return(trackers) | 37 | ErrbitPlugin::Registry.stub(:issue_trackers).and_return(trackers) |
38 | + problem.app.issue_tracker = IssueTracker.new :type_tracker => tracker, :options => {:api_token => "token token token", :project_id => "1234"} | ||
39 | + | ||
41 | view.stub(:problem).and_return(problem) | 40 | view.stub(:problem).and_return(problem) |
42 | view.stub(:app).and_return(problem.app) | 41 | view.stub(:app).and_return(problem.app) |
43 | end | 42 | end |
@@ -82,26 +81,16 @@ describe "problems/show.html.haml" do | @@ -82,26 +81,16 @@ describe "problems/show.html.haml" do | ||
82 | end | 81 | end |
83 | 82 | ||
84 | context 'create issue links' do | 83 | context 'create issue links' do |
85 | - it 'should allow creating issue for github if current user has linked their github account' do | ||
86 | - user = Fabricate(:user, :github_login => 'test_user', :github_oauth_token => 'abcdef') | ||
87 | - controller.stub(:current_user) { user } | ||
88 | - | ||
89 | - problem = Fabricate(:problem_with_comments, :app => Fabricate(:app, :github_repo => "test_user/test_repo")) | ||
90 | - view.stub(:problem).and_return(problem) | ||
91 | - view.stub(:app).and_return(problem.app) | ||
92 | - render | ||
93 | - | ||
94 | - expect(action_bar).to have_selector("span a.github_create.create-issue", :text => 'create issue') | ||
95 | - end | 84 | + let(:app) { Fabricate(:app, :github_repo => "test_user/test_repo") } |
96 | 85 | ||
97 | it 'should allow creating issue for github if application has a github tracker' do | 86 | it 'should allow creating issue for github if application has a github tracker' do |
98 | - problem = Fabricate(:problem_with_comments, :app => Fabricate(:app, :github_repo => "test_user/test_repo")) | 87 | + problem = Fabricate(:problem_with_comments, app: app) |
99 | with_issue_tracker("github", problem) | 88 | with_issue_tracker("github", problem) |
100 | view.stub(:problem).and_return(problem) | 89 | view.stub(:problem).and_return(problem) |
101 | view.stub(:app).and_return(problem.app) | 90 | view.stub(:app).and_return(problem.app) |
102 | render | 91 | render |
103 | 92 | ||
104 | - expect(action_bar).to have_selector("span a.github_create.create-issue", :text => 'create issue') | 93 | + expect(action_bar).to have_selector("span a.github_create.create-issue", text: 'create issue') |
105 | end | 94 | end |
106 | 95 | ||
107 | context "without issue tracker associate on app" do | 96 | context "without issue tracker associate on app" do |