Download as
Browse Code »

Commit cddea1b5a9995e211ea459bb22c24a0706141572

Authored by Nathan Broadbent
1 parent 4815e487
Exists in master and in 1 other branch production

Refactored issues. If a user links their GitHub account, they can now choose bet… 

…ween creating an issue on Github, or on their configured issue tracker.
Showing 12 changed files with 98 additions and 41 deletions   Show diff stats
app/assets/stylesheets/errbit.css
  Diff comments   View file @cddea1b
@@ -162,10 +162,10 @@ a.action { float: right; font-size: 0.9em;} @@ -162,10 +162,10 @@ a.action { float: right; font-size: 0.9em;}
162 162
163 /* Content Title and Comments */ 163 /* Content Title and Comments */
164 #content-title, #content-comments { 164 #content-title, #content-comments {
165 - padding: 30px 20px; 165 + padding: 43px 24px 37px;
166 border-top: 1px solid #FFF; 166 border-top: 1px solid #FFF;
167 border-bottom: 1px solid #FFF; 167 border-bottom: 1px solid #FFF;
168 - background-color: #ececec; 168 + background-color: #f2f2f2;
169 } 169 }
170 #content-comments { 170 #content-comments {
171 background-color: #ffffff; 171 background-color: #ffffff;
@@ -188,11 +188,13 @@ a.action { float: right; font-size: 0.9em;} @@ -188,11 +188,13 @@ a.action { float: right; font-size: 0.9em;}
188 /* Action Bar */ 188 /* Action Bar */
189 #action-bar { 189 #action-bar {
190 position: absolute; 190 position: absolute;
191 - top: 25px; right: 20px; 191 + text-align: right;
  192 + top: 22px; right: 24px;
192 } 193 }
193 #action-bar span { 194 #action-bar span {
194 display: inline-block; 195 display: inline-block;
195 margin-left: 18px; 196 margin-left: 18px;
  197 + margin-bottom: 16px;
196 text-decoration: none; 198 text-decoration: none;
197 color: #666; 199 color: #666;
198 background: #FFF url(images/button-bg.png) 0 bottom repeat-x; 200 background: #FFF url(images/button-bg.png) 0 bottom repeat-x;
@@ -667,10 +669,11 @@ table.tally tbody tr:first-child th { @@ -667,10 +669,11 @@ table.tally tbody tr:first-child th {
667 padding-top:0; 669 padding-top:0;
668 } 670 }
669 table.tally td.percent { 671 table.tally td.percent {
670 - width:4.5em; 672 + padding-right: 10px;
671 } 673 }
672 table.tally th.value { 674 table.tally th.value {
673 - text-transform:none; 675 + width: 100%;
  676 + text-transform: none;
674 } 677 }
675 678
676 /* Deploys table */ 679 /* Deploys table */
app/controllers/errs_controller.rb
  Diff comments   View file @cddea1b
@@ -5,6 +5,7 @@ class ErrsController < ApplicationController @@ -5,6 +5,7 @@ class ErrsController < ApplicationController
5 before_filter :find_problem, :except => [:index, :all, :destroy_several, :resolve_several, :unresolve_several, :merge_several, :unmerge_several] 5 before_filter :find_problem, :except => [:index, :all, :destroy_several, :resolve_several, :unresolve_several, :merge_several, :unmerge_several]
6 before_filter :find_selected_problems, :only => [:destroy_several, :resolve_several, :unresolve_several, :merge_several, :unmerge_several] 6 before_filter :find_selected_problems, :only => [:destroy_several, :resolve_several, :unresolve_several, :merge_several, :unmerge_several]
7 before_filter :set_sorting_params, :only => [:index, :all] 7 before_filter :set_sorting_params, :only => [:index, :all]
  8 + before_filter :set_tracker_params, :only => [:create_issue]
8 9
9 def index 10 def index
10 app_scope = current_user.admin? ? App.all : current_user.apps 11 app_scope = current_user.admin? ? App.all : current_user.apps
@@ -36,21 +37,38 @@ class ErrsController < ApplicationController @@ -36,21 +37,38 @@ class ErrsController < ApplicationController
36 end 37 end
37 38
38 def create_issue 39 def create_issue
39 - set_tracker_params 40 + # Create an issue on GitHub using user's github token
  41 + if params[:tracker] == 'user_github'
  42 + if !@app.github_repo?
  43 + flash[:error] = "This app doesn't have a GitHub repo set up."
  44 + elsif !current_user.github_account?
  45 + flash[:error] = "You haven't linked your Github account."
  46 + else
  47 + @tracker = GithubIssuesTracker.new(
  48 + :app => @app,
  49 + :login => current_user.github_login,
  50 + :oauth_token => current_user.github_oauth_token
  51 + )
  52 + end
  53 +
  54 + # Or, create an issue using the App's issue tracker
  55 + elsif @app.issue_tracker_configured?
  56 + @tracker = @app.issue_tracker
40 57
41 - if @app.issue_tracker 58 + # Otherwise, display error about missing tracker configuration.
  59 + else
  60 + flash[:error] = "This app has no issue tracker setup."
  61 + end
  62 +
  63 + if flash[:error].blank? && @tracker
42 begin 64 begin
43 - @app.issue_tracker.create_issue @problem, current_user 65 + @tracker.create_issue @problem, current_user
44 rescue Exception => ex 66 rescue Exception => ex
45 - flash[:error] = ex.message 67 + Rails.logger.error "Error during issue creation: " << ex.message
  68 + flash[:error] = "There was an error during issue creation: #{ex.message}"
46 end 69 end
47 - else  
48 - flash[:error] = "This app has no issue tracker setup."  
49 end 70 end
50 - redirect_to app_err_path(@app, @problem)  
51 - rescue ActiveResource::ConnectionError => e  
52 - Rails.logger.error e.to_s  
53 - flash[:error] = "There was an error during issue creation. Check your tracker settings or try again later." 71 +
54 redirect_to app_err_path(@app, @problem) 72 redirect_to app_err_path(@app, @problem)
55 end 73 end
56 74
app/models/issue_trackers/github_issues_tracker.rb
  Diff comments   View file @cddea1b
1 class IssueTrackers::GithubIssuesTracker < IssueTracker 1 class IssueTrackers::GithubIssuesTracker < IssueTracker
2 Label = "github" 2 Label = "github"
  3 + Note = 'Please configure your github repository in the <strong>GITHUB REPO</strong> field above.<br/>' <<
  4 + 'Instead of providing your username & password, you can link your Github account ' <<
  5 + 'to your user profile, and allow Errbit to create issues using your OAuth token.'
  6 +
3 Fields = [ 7 Fields = [
4 - [:project_id, {  
5 - :label => "Account/Repository",  
6 - :placeholder => "errbit/errbit from https://github.com/errbit/errbit"  
7 - }],  
8 [:username, { 8 [:username, {
9 :placeholder => "Your username on GitHub" 9 :placeholder => "Your username on GitHub"
10 }], 10 }],
@@ -13,6 +13,8 @@ class IssueTrackers::GithubIssuesTracker &lt; IssueTracker @@ -13,6 +13,8 @@ class IssueTrackers::GithubIssuesTracker &lt; IssueTracker
13 }] 13 }]
14 ] 14 ]
15 15
  16 + attr_accessor :oauth_token
  17 +
16 def check_params 18 def check_params
17 if Fields.detect {|f| self[f[0]].blank? } 19 if Fields.detect {|f| self[f[0]].blank? }
18 errors.add :base, 'You must specify your GitHub repository, username and password' 20 errors.add :base, 'You must specify your GitHub repository, username and password'
@@ -20,9 +22,15 @@ class IssueTrackers::GithubIssuesTracker &lt; IssueTracker @@ -20,9 +22,15 @@ class IssueTrackers::GithubIssuesTracker &lt; IssueTracker
20 end 22 end
21 23
22 def create_issue(problem, reported_by = nil) 24 def create_issue(problem, reported_by = nil)
23 - client = Octokit::Client.new(:login => username, :password => password) 25 + # Login using OAuth token, if given.
  26 + if oauth_token
  27 + client = Octokit::Client.new(:login => username, :oauth_token => oauth_token)
  28 + else
  29 + client = Octokit::Client.new(:login => username, :password => password)
  30 + end
  31 +
24 begin 32 begin
25 - issue = client.create_issue(project_id, issue_title(problem), body_template.result(binding).unpack('C*').pack('U*'), options = {}) 33 + issue = client.create_issue(app.github_repo, issue_title(problem), body_template.result(binding).unpack('C*').pack('U*'), options = {})
26 problem.update_attribute :issue_link, issue.html_url 34 problem.update_attribute :issue_link, issue.html_url
27 rescue Octokit::Unauthorized 35 rescue Octokit::Unauthorized
28 raise IssueTrackers::AuthenticationError, "Could not authenticate with GitHub. Please check your username and password." 36 raise IssueTrackers::AuthenticationError, "Could not authenticate with GitHub. Please check your username and password."
@@ -34,6 +42,6 @@ class IssueTrackers::GithubIssuesTracker &lt; IssueTracker @@ -34,6 +42,6 @@ class IssueTrackers::GithubIssuesTracker &lt; IssueTracker
34 end 42 end
35 43
36 def url 44 def url
37 - "https://github.com/#{project_id}/issues" 45 + "https://github.com/#{app.github_repo}/issues"
38 end 46 end
39 end 47 end
app/models/issue_trackers/mingle_tracker.rb
  Diff comments   View file @cddea1b
1 class IssueTrackers::MingleTracker < IssueTracker 1 class IssueTrackers::MingleTracker < IssueTracker
2 Label = "mingle" 2 Label = "mingle"
  3 + Note = "Note: <strong>CARD PROPERTIES</strong> must be comma separated <em>key = value</em> pairs"
  4 +
3 Fields = [ 5 Fields = [
4 [:account, { 6 [:account, {
5 :label => "Mingle URL", 7 :label => "Mingle URL",
@@ -9,7 +11,7 @@ class IssueTrackers::MingleTracker &lt; IssueTracker @@ -9,7 +11,7 @@ class IssueTrackers::MingleTracker &lt; IssueTracker
9 :placeholder => "Mingle project" 11 :placeholder => "Mingle project"
10 }], 12 }],
11 [:ticket_properties, { 13 [:ticket_properties, {
12 - :label => "Card Properties (comma separated key=value pairs)", 14 + :label => "Card Properties",
13 :placeholder => "card_type = Defect, defect_status = Open, priority = Essential" 15 :placeholder => "card_type = Defect, defect_status = Open, priority = Essential"
14 }], 16 }],
15 [:username, { 17 [:username, {
app/models/user.rb
  Diff comments   View file @cddea1b
@@ -44,6 +44,10 @@ class User @@ -44,6 +44,10 @@ class User
44 github_login.present? ? false : super 44 github_login.present? ? false : super
45 end 45 end
46 46
  47 + def github_account?
  48 + github_login.present? && github_oauth_token.present?
  49 + end
  50 +
47 protected 51 protected
48 52
49 def destroy_watchers 53 def destroy_watchers
app/views/apps/_issue_tracker_fields.html.haml
  Diff comments   View file @cddea1b
@@ -15,6 +15,8 @@ @@ -15,6 +15,8 @@
15 %p When no issue tracker has been configured, you will be able to leave comments on errors. 15 %p When no issue tracker has been configured, you will be able to leave comments on errors.
16 - IssueTracker.subclasses.each do |tracker| 16 - IssueTracker.subclasses.each do |tracker|
17 %div.tracker_params{:class => (w.object.is_a?(tracker) ? 'chosen ' : '') << tracker::Label} 17 %div.tracker_params{:class => (w.object.is_a?(tracker) ? 'chosen ' : '') << tracker::Label}
  18 + - if defined?(tracker::Note)
  19 + %p= tracker::Note.html_safe
18 - tracker::Fields.each do |field, field_info| 20 - tracker::Fields.each do |field, field_info|
19 = w.label field, field_info[:label] || field.to_s.titleize 21 = w.label field, field_info[:label] || field.to_s.titleize
20 - field_type = field == :password ? :password_field : :text_field 22 - field_type = field == :password ? :password_field : :text_field
app/views/errs/_issue_tracker_links.html.haml 0 → 100644
  Diff comments   View file @cddea1b
@@ -0,0 +1,12 @@ @@ -0,0 +1,12 @@
  1 +- if @app.issue_tracker_configured? || current_user.github_account?
  2 + - if @problem.issue_link.present?
  3 + %span= link_to 'go to issue', @problem.issue_link, :class => "#{@app.issue_tracker.class::Label}_goto goto-issue"
  4 + = link_to 'unlink issue', unlink_issue_app_err_path(@app, @problem), :method => :delete, :confirm => "Unlink err issues?", :class => "unlink-issue"
  5 + - elsif @problem.issue_link == "pending"
  6 + %span.disabled= link_to 'creating...', '#', :class => "#{@app.issue_tracker.class::Label}_inactive create-issue"
  7 + = link_to 'retry', create_issue_app_err_path(@app, @problem), :method => :post
  8 + - else
  9 + - if current_user.github_account? && @app.github_repo?
  10 + %span= link_to 'create issue', create_issue_app_err_path(@app, @problem, :tracker => 'user_github'), :method => :post, :class => "github_create create-issue"
  11 + - if @app.issue_tracker_configured? && !@app.issue_tracker.is_a?(GithubIssuesTracker)
  12 + %span= link_to 'create issue', create_issue_app_err_path(@app, @problem), :method => :post, :class => "#{@app.issue_tracker.class::Label}_create create-issue"
app/views/errs/show.html.haml
  Diff comments   View file @cddea1b
1 - content_for :page_title, @problem.message 1 - content_for :page_title, @problem.message
2 -- content_for :title, @problem.error_class 2 +- content_for :title, @problem.error_class || truncate(@problem.message, :length => 32)
3 - content_for :meta do 3 - content_for :meta do
4 %strong App: 4 %strong App:
5 = link_to @app.name, app_path(@app) 5 = link_to @app.name, app_path(@app)
@@ -11,20 +11,13 @@ @@ -11,20 +11,13 @@
11 %strong Last Notice: 11 %strong Last Notice:
12 = last_notice_at(@problem).to_s(:precise) 12 = last_notice_at(@problem).to_s(:precise)
13 - content_for :action_bar do 13 - content_for :action_bar do
14 - - if current_user.authentication_token  
15 - %span= link_to 'iCal', app_err_path(:app_id => @app.id, :id => @problem.id, :format => "ics", :auth_token => current_user.authentication_token), :class => "calendar_link"  
16 - - if @problem.app.issue_tracker_configured?  
17 - - if @problem.issue_link.blank?  
18 - %span= link_to 'create issue', create_issue_app_err_path(@app, @problem), :method => :post, :class => "#{@app.issue_tracker.class::Label}_create create-issue"  
19 - - elsif @problem.issue_link == "pending"  
20 - %span.disabled= link_to 'creating...', '#', :class => "#{@app.issue_tracker.class::Label}_inactive create-issue"  
21 - = link_to 'retry', create_issue_app_err_path(@app, @problem), :method => :post  
22 - - else  
23 - %span= link_to 'go to issue', @problem.issue_link, :class => "#{@app.issue_tracker.class::Label}_goto goto-issue"  
24 - = link_to 'unlink issue', unlink_issue_app_err_path(@app, @problem), :method => :delete, :confirm => "Unlink err issues?", :class => "unlink-issue"  
25 - if @problem.unresolved? 14 - if @problem.unresolved?
26 %span= link_to 'resolve', resolve_app_err_path(@app, @problem), :method => :put, :confirm => err_confirm, :class => 'resolve' 15 %span= link_to 'resolve', resolve_app_err_path(@app, @problem), :method => :put, :confirm => err_confirm, :class => 'resolve'
27 - %span= link_to 'up', (request.env['HTTP_REFERER'] ? :back : app_errs_path(@app)), :class => 'up' 16 + - if current_user.authentication_token
  17 + %span= link_to 'iCal', app_err_path(:app_id => @app.id, :id => @problem.id, :format => "ics", :auth_token => current_user.authentication_token), :class => "calendar_link"
  18 + %span>= link_to 'up', (request.env['HTTP_REFERER'] ? :back : app_errs_path(@app)), :class => 'up'
  19 + %br
  20 + = render :partial => "issue_tracker_links"
28 21
29 - if Errbit::Config.allow_comments_with_issue_tracker || !@app.issue_tracker_configured? || @problem.comments.any? 22 - if Errbit::Config.allow_comments_with_issue_tracker || !@app.issue_tracker_configured? || @problem.comments.any?
30 - content_for :comments do 23 - content_for :comments do
app/views/shared/_link_github_account.html.haml
  Diff comments   View file @cddea1b
1 - if Errbit::Config.github_authentication && user == current_user 1 - if Errbit::Config.github_authentication && user == current_user
2 - - if user.github_login && user.github_oauth_token 2 + - if user.github_account?
3 %span.unlink_github= link_to "Unlink GitHub account", unlink_github_user_path(user), :method => :delete, :confirm => "Are you sure?" 3 %span.unlink_github= link_to "Unlink GitHub account", unlink_github_user_path(user), :method => :delete, :confirm => "Are you sure?"
4 - else 4 - else
5 %span.github= link_to "Link GitHub account", user_omniauth_authorize_path(:github) 5 %span.github= link_to "Link GitHub account", user_omniauth_authorize_path(:github)
spec/controllers/errs_controller_spec.rb
  Diff comments   View file @cddea1b
@@ -330,7 +330,7 @@ describe ErrsController do @@ -330,7 +330,7 @@ describe ErrsController do
330 end 330 end
331 331
332 it "should notify of connection error" do 332 it "should notify of connection error" do
333 - flash[:error].should == "There was an error during issue creation. Check your tracker settings or try again later." 333 + flash[:error].should include("There was an error during issue creation:")
334 end 334 end
335 end 335 end
336 end 336 end
spec/models/issue_trackers/github_issues_tracker_spec.rb
  Diff comments   View file @cddea1b
@@ -2,12 +2,14 @@ require &#39;spec_helper&#39; @@ -2,12 +2,14 @@ require &#39;spec_helper&#39;
2 2
3 describe IssueTrackers::GithubIssuesTracker do 3 describe IssueTrackers::GithubIssuesTracker do
4 it "should create an issue on GitHub Issues with problem params, and set issue link for problem" do 4 it "should create an issue on GitHub Issues with problem params, and set issue link for problem" do
  5 + repo = "test_user/test_repo"
5 notice = Fabricate :notice 6 notice = Fabricate :notice
  7 + notice.app.github_repo = repo
6 tracker = Fabricate :github_issues_tracker, :app => notice.app 8 tracker = Fabricate :github_issues_tracker, :app => notice.app
7 problem = notice.problem 9 problem = notice.problem
8 10
9 number = 5 11 number = 5
10 - @issue_link = "https://github.com/#{tracker.project_id}/issues/#{number}" 12 + @issue_link = "https://github.com/#{repo}/issues/#{number}"
11 body = <<EOF 13 body = <<EOF
12 { 14 {
13 "position": 1.0, 15 "position": 1.0,
@@ -23,13 +25,13 @@ describe IssueTrackers::GithubIssuesTracker do @@ -23,13 +25,13 @@ describe IssueTrackers::GithubIssuesTracker do
23 } 25 }
24 EOF 26 EOF
25 27
26 - stub_request(:post, "https://#{tracker.username}:#{tracker.password}@api.github.com/repos/#{tracker.project_id}/issues"). 28 + stub_request(:post, "https://#{tracker.username}:#{tracker.password}@api.github.com/repos/#{repo}/issues").
27 to_return(:status => 201, :headers => {'Location' => @issue_link}, :body => body ) 29 to_return(:status => 201, :headers => {'Location' => @issue_link}, :body => body )
28 30
29 problem.app.issue_tracker.create_issue(problem) 31 problem.app.issue_tracker.create_issue(problem)
30 problem.reload 32 problem.reload
31 33
32 - requested = have_requested(:post, "https://#{tracker.username}:#{tracker.password}@api.github.com/repos/#{tracker.project_id}/issues") 34 + requested = have_requested(:post, "https://#{tracker.username}:#{tracker.password}@api.github.com/repos/#{repo}/issues")
33 WebMock.should requested.with(:body => /[production][foo#bar] FooError: Too Much Bar/) 35 WebMock.should requested.with(:body => /[production][foo#bar] FooError: Too Much Bar/)
34 WebMock.should requested.with(:body => /See this exception on Errbit/) 36 WebMock.should requested.with(:body => /See this exception on Errbit/)
35 37
spec/views/errs/show.html.haml_spec.rb
  Diff comments   View file @cddea1b
@@ -56,6 +56,19 @@ describe &quot;errs/show.html.haml&quot; do @@ -56,6 +56,19 @@ describe &quot;errs/show.html.haml&quot; do
56 action_bar.should have_selector("span a.up[href='#{app_errs_path(problem.app)}']", :text => 'up') 56 action_bar.should have_selector("span a.up[href='#{app_errs_path(problem.app)}']", :text => 'up')
57 end 57 end
58 58
  59 + context 'create issue links' do
  60 + it 'should allow creating issue for github if current user has linked their github account' do
  61 + user = Fabricate(:user, :github_login => 'test_user', :github_oauth_token => 'abcdef')
  62 + controller.stub(:current_user) { user }
  63 +
  64 + problem = Fabricate(:problem_with_comments, :app => Fabricate(:app, :github_repo => "test_user/test_repo"))
  65 + assign :problem, problem
  66 + assign :app, problem.app
  67 + render
  68 +
  69 + action_bar.should have_selector("span a.github_create.create-issue", :text => 'create issue')
  70 + end
  71 + end
59 end 72 end
60 73
61 describe "content_for :comments with comments disabled for configured issue tracker" do 74 describe "content_for :comments with comments disabled for configured issue tracker" do