Commit 19c83f25410e197512284c9523333fed178dd130

Authored by Nathan Broadbent
1 parent 6693dd85
Exists in master and in 1 other branch production

Refactored _issue_tracker_fields.html.haml by moving all field information into …

…issue tracker models. This not only keeps the view DRY, but also streamlines the process of creating new issue trackers. Next step is to write a rails generator so that we can run 'rails generate issue_tracker NewIssueTracker'. Also, this commit contains a fix for the apps/edit page, where issue trackers were not being saved correctly.
app/controllers/apps_controller.rb
... ... @@ -46,7 +46,7 @@ class AppsController < InheritedResources::Base
46 46 protected
47 47 def initialize_subclassed_issue_tracker
48 48 if params[:app][:issue_tracker_attributes] && tracker_type = params[:app][:issue_tracker_attributes][:type]
49   - if IssueTracker.subclasses.map(&:to_s).concat(["IssueTracker"]).include?(tracker_type.to_s)
  49 + if IssueTracker.subclasses.map(&:name).concat(["IssueTracker"]).include?(tracker_type)
50 50 @app.issue_tracker = tracker_type.constantize.new(params[:app][:issue_tracker_attributes])
51 51 end
52 52 end
... ...
app/models/issue_trackers/fogbugz_tracker.rb
1 1 class IssueTrackers::FogbugzTracker < IssueTracker
2 2 Label = "fogbugz"
3   - RequiredFields = %w(project_id account username password)
  3 + Fields = [
  4 + [:project_id, {
  5 + :label => "Area Name"
  6 + }],
  7 + [:account, {
  8 + :label => "FogBugz URL",
  9 + :placeholder => "abc from http://abc.fogbugz.com/"
  10 + }],
  11 + [:username, {
  12 + :placeholder => "Username/Email for your account"
  13 + }],
  14 + [:password, {
  15 + :placeholder => "Password for your account"
  16 + }]
  17 + ]
4 18  
5 19 def check_params
6   - if RequiredFields.detect {|f| self[f].blank? }
  20 + if Fields.detect {|f| self[f[0]].blank? }
7 21 errors.add :base, 'You must specify your FogBugz Area Name, FogBugz URL, Username, and Password'
8 22 end
9 23 end
... ...
app/models/issue_trackers/github_issues_tracker.rb 0 → 100644
... ... @@ -0,0 +1,32 @@
  1 +class IssueTrackers::GithubIssuesTracker < IssueTracker
  2 + Label = "github"
  3 + Fields = [
  4 + [:project_id, {
  5 + :label => "Account/Repository",
  6 + :placeholder => "errbit/errbit from https://github.com/errbit/errbit"
  7 + }],
  8 + [:username, {
  9 + :placeholder => "Your username on Github"
  10 + }],
  11 + [:api_token, {
  12 + :placeholder => "Your Github API Token"
  13 + }]
  14 + ]
  15 +
  16 + def check_params
  17 + if Fields.detect {|f| self[f[0]].blank? }
  18 + errors.add :base, 'You must specify your Github repository, username and API Token'
  19 + end
  20 + end
  21 +
  22 + def create_issue(err)
  23 + client = Octokit::Client.new(:login => username, :token => api_token)
  24 + issue = client.create_issue(project_id, issue_title(err), body_template.result(binding), options = {})
  25 + err.update_attribute :issue_link, issue.html_url
  26 + end
  27 +
  28 + def body_template
  29 + @@body_template ||= ERB.new(File.read(Rails.root + "app/views/issue_trackers/github_issues_body.txt.erb").gsub(/^\s*/, ''))
  30 + end
  31 +end
  32 +
... ...
app/models/issue_trackers/github_tracker.rb
... ... @@ -1,21 +0,0 @@
1   -class IssueTrackers::GithubTracker < IssueTracker
2   - Label = "github"
3   - RequiredFields = %w(project_id username api_token)
4   -
5   - def check_params
6   - if RequiredFields.detect {|f| self[f].blank? }
7   - errors.add :base, 'You must specify your Github repository, username and API token'
8   - end
9   - end
10   -
11   - def create_issue(err)
12   - client = Octokit::Client.new(:login => username, :token => api_token)
13   - issue = client.create_issue(project_id, issue_title(err), body_template.result(binding), options = {})
14   - err.update_attribute :issue_link, issue.html_url
15   - end
16   -
17   - def body_template
18   - @@body_template ||= ERB.new(File.read(Rails.root + "app/views/issue_trackers/github_body.txt.erb").gsub(/^\s*/, ''))
19   - end
20   -end
21   -
app/models/issue_trackers/lighthouse_tracker.rb
1 1 class IssueTrackers::LighthouseTracker < IssueTracker
2 2 Label = "lighthouseapp"
3   - RequiredFields = %w(account api_token project_id)
  3 + Fields = [
  4 + [:account, {
  5 + :placeholder => "abc from abc.lighthouseapp.com"
  6 + }],
  7 + [:api_token, {
  8 + :placeholder => "API Token for your account"
  9 + }],
  10 + [:project_id, {
  11 + :placeholder => "Lighthouse project"
  12 + }]
  13 + ]
4 14  
5 15 def check_params
6   - if RequiredFields.detect {|f| self[f].blank? }
  16 + if Fields.detect {|f| self[f[0]].blank? }
7 17 errors.add :base, 'You must specify your Lighthouseapp account, API token and Project ID'
8 18 end
9 19 end
... ...
app/models/issue_trackers/mingle_tracker.rb
1 1 class IssueTrackers::MingleTracker < IssueTracker
2 2 Label = "mingle"
3   - RequiredFields = %w(account project_id username password)
  3 + Fields = [
  4 + [:account, {
  5 + :label => "Mingle URL",
  6 + :placeholder => "abc from http://abc.fogbugz.com/"
  7 + }],
  8 + [:project_id, {
  9 + :placeholder => "Mingle project"
  10 + }],
  11 + [:ticket_properties, {
  12 + :label => "Card Properties (comma separated key=value pairs)",
  13 + :placeholder => "card_type = Defect, defect_status = Open, priority = Essential"
  14 + }],
  15 + [:username, {
  16 + :label => "Sign-in name",
  17 + :placeholder => "Sign-in name for your account"
  18 + }],
  19 + [:password, {
  20 + :placeholder => "Password for your account"
  21 + }]
  22 + ]
4 23  
5 24 def check_params
6   - if RequiredFields.detect {|f| self[f].blank? } or !ticket_properties_hash["card_type"]
  25 + if Fields.detect {|f| self[f[0]].blank? } or !ticket_properties_hash["card_type"]
7 26 errors.add :base, 'You must specify your Mingle URL, Project ID, Card Type (in default card properties), Sign-in name, and Password'
8 27 end
9 28 end
... ...
app/models/issue_trackers/pivotal_labs_tracker.rb
1 1 class IssueTrackers::PivotalLabsTracker < IssueTracker
2 2 Label = "pivotal"
3   - RequiredFields = %w(api_token project_id)
  3 + Fields = [
  4 + [:api_token, {
  5 + :placeholder => "API Token for your account"
  6 + }],
  7 + [:project_id, {}]
  8 + ]
4 9  
5 10 def check_params
6   - if RequiredFields.detect {|f| self[f].blank? }
  11 + if Fields.detect {|f| self[f[0]].blank? }
7 12 errors.add :base, 'You must specify your Pivotal Tracker API token and Project ID'
8 13 end
9 14 end
... ...
app/models/issue_trackers/redmine_tracker.rb
1 1 class IssueTrackers::RedmineTracker < IssueTracker
2 2 Label = "redmine"
3   - RequiredFields = %w(account api_token project_id)
  3 + Fields = [
  4 + [:account, {
  5 + :label => "Redmine URL",
  6 + :placeholder => "e.g. http://www.redmine.org/"
  7 + }],
  8 + [:api_token, {
  9 + :placeholder => "API Token for your account"
  10 + }],
  11 + [:project_id, {}]
  12 + ]
4 13  
5 14 def check_params
6   - if RequiredFields.detect {|f| self[f].blank? }
  15 + if Fields.detect {|f| self[f[0]].blank? }
7 16 errors.add :base, 'You must specify your Redmine URL, API token and Project ID'
8 17 end
9 18 end
... ...
app/views/apps/_issue_tracker_fields.html.haml
... ... @@ -6,76 +6,21 @@
6 6 = label_tag :type_none, :for => label_for_attr(w, 'type_issuetracker'), :class => "label_radio none" do
7 7 = w.radio_button :type, "IssueTracker", 'data-section' => 'none'
8 8 (None)
9   - = label_tag :type_github, :for => label_for_attr(w, 'type_githubtracker'), :class => "label_radio github" do
10   - = w.radio_button :type, "GithubTracker", 'data-section' => 'github'
11   - Github Issues
12   - = label_tag :type_lighthouseapp, :for => label_for_attr(w, 'type_lighthousetracker'), :class => "label_radio lighthouseapp" do
13   - = w.radio_button :type, "LighthouseTracker", 'data-section' => 'lighthouse'
14   - Lighthouse
15   - = label_tag :type_redmine, :for => label_for_attr(w, 'type_redminetracker'), :class => "label_radio redmine" do
16   - = w.radio_button :type, "RedmineTracker", 'data-section' => 'redmine'
17   - Redmine
18   - = label_tag :type_pivotal, :for => label_for_attr(w, 'type_pivotallabstracker'), :class => "label_radio pivotal" do
19   - = w.radio_button :type, "PivotalLabsTracker", 'data-section' => 'pivotal'
20   - Pivotal Tracker
21   - = label_tag :type_fogbugz, :for => label_for_attr(w, 'type_fogbugztracker'), :class => "label_radio fogbugz" do
22   - = w.radio_button :type, "FogbugzTracker", 'data-section' => 'fogbugz'
23   - FogBugz
24   - = label_tag :type_mingle, :for => label_for_attr(w, 'type_mingletracker'), :class => "label_radio mingle" do
25   - = w.radio_button :type, "MingleTracker", 'data-section' => 'mingle'
26   - Mingle
  9 + - IssueTracker.subclasses.each do |tracker|
  10 + = label_tag "type_#{tracker::Label}:", :for => label_for_attr(w, "type_#{tracker.name.downcase.gsub(':','')}"), :class => "label_radio #{tracker::Label}" do
  11 + = w.radio_button :type, tracker.name, 'data-section' => tracker::Label
  12 + = tracker.name[/::(.*)Tracker/,1].titleize
27 13  
28 14 %div.tracker_params.none{:class => (w.object && !(w.object.class < IssueTracker)) ? 'chosen' : nil}
29 15 %p When no issue tracker has been configured, you will be able to leave comments on errors.
30   - %div.tracker_params.github{:class => w.object.is_a?(GithubTracker) ? 'chosen' : nil}
31   - = w.label :project_id, "Account/Repository"
32   - = w.text_field :project_id, :placeholder => "errbit/errbit from https://github.com/errbit/errbit"
33   - = w.label :username, "Username"
34   - = w.text_field :username, :placeholder => "Your username on Github"
35   - = w.label :api_token, "Token"
36   - = w.text_field :api_token, :placeholder => "Your API Token"
37   - %div.tracker_params.lighthouse{:class => w.object.is_a?(LighthouseTracker) ? 'chosen' : nil}
38   - = w.label :account, "Account"
39   - = w.text_field :account, :placeholder => "abc from abc.lighthouseapp.com"
40   - = w.label :api_token, "API token"
41   - = w.text_field :api_token, :placeholder => "API Token for your account"
42   - = w.label :project_id, "Project ID"
43   - = w.text_field :project_id
44   - %div.tracker_params.redmine{:class => w.object.is_a?(RedmineTracker) ? 'chosen' : nil}
45   - = w.label :account, "Redmine URL"
46   - = w.text_field :account, :placeholder => "like http://www.redmine.org/"
47   - = w.label :api_token, "API token"
48   - = w.text_field :api_token, :placeholder => "API Token for your account"
49   - = w.label :project_id, "Project ID"
50   - = w.text_field :project_id
51   - %div.tracker_params.pivotal{:class => w.object.is_a?(PivotalLabsTracker) ? 'chosen' : nil}
52   - = w.label :project_id, "Project ID"
53   - = w.text_field :project_id
54   - = w.label :api_token, "API token"
55   - = w.text_field :api_token, :placeholder => "API Token for your account"
56   - %div.tracker_params.fogbugz{:class => w.object.is_a?(FogbugzTracker) ? 'chosen' : nil}
57   - = w.label :project_id, "Area Name"
58   - = w.text_field :project_id
59   - = w.label :account, "FogBugz URL"
60   - = w.text_field :account, :placeholder => "abc from http://abc.fogbugz.com/"
61   - = w.label :username, 'Username'
62   - = w.text_field :username, :placeholder => 'Username/Email for your account'
63   - = w.label :password, 'Password'
64   - = w.password_field :password, :placeholder => 'Password for your account'
65   - %div.tracker_params.mingle{:class => w.object.is_a?(MingleTracker) ? 'chosen' : nil}
66   - = w.label :account, "Mingle URL"
67   - = w.text_field :account, :placeholder => "http://mingle.yoursite.com/"
68   - = w.label :project_id, "Project ID"
69   - = w.text_field :project_id
70   - = w.label :ticket_properties, "Card Properties (comma separated key=value pairs)"
71   - = w.text_field :ticket_properties, :placeholder => "card_type = Defect, defect_status = Open, priority = Essential"
72   - = w.label :username, 'Sign-in name'
73   - = w.text_field :username, :placeholder => 'Sign-in name for your account'
74   - = w.label :password, 'Password'
75   - = w.password_field :password, :placeholder => 'Password for your account'
  16 + - IssueTracker.subclasses.each do |tracker|
  17 + %div.tracker_params{:class => (w.object.is_a?(tracker) ? 'chosen ' : '') << tracker::Label}
  18 + - tracker::Fields.each do |field, field_info|
  19 + = w.label field, field_info[:label] || field.to_s.titleize
  20 + = w.text_field field, :placeholder => field_info[:placeholder]
76 21  
77 22 .image_preloader
78   - - %w(none github lighthouseapp redmine pivotal fogbugz mingle).each do |tracker|
  23 + - (IssueTracker.subclasses.map{|t| t::Label } << 'none').each do |tracker|
79 24 = image_tag "#{tracker}_inactive.png"
80 25 = image_tag "#{tracker}_create.png"
81 26  
... ...
app/views/issue_trackers/github_body.txt.erb
... ... @@ -1,45 +0,0 @@
1   -[See this exception on Errbit](<%= app_err_url err.app, err %> "See this exception on Errbit")
2   -<% if notice = err.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.err.where %>
11   -
12   -### Occured ###
13   -<%= notice.created_at.to_s(:micro) %>
14   -
15   -### Similar ###
16   -<%= (notice.err.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   -<% for line in notice.backtrace %><%= line['number'] %>: <%= line['file'].sub(/^\[PROJECT_ROOT\]/, '') %> -> **<%= 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/issue_trackers/github_issues_body.txt.erb 0 → 100644
... ... @@ -0,0 +1,45 @@
  1 +[See this exception on Errbit](<%= app_err_url err.app, err %> "See this exception on Errbit")
  2 +<% if notice = err.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.err.where %>
  11 +
  12 +### Occured ###
  13 +<%= notice.created_at.to_s(:micro) %>
  14 +
  15 +### Similar ###
  16 +<%= (notice.err.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 +<% for line in notice.backtrace %><%= line['number'] %>: <%= line['file'].sub(/^\[PROJECT_ROOT\]/, '') %> -> **<%= 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 +
... ...
spec/controllers/apps_controller_spec.rb
... ... @@ -276,7 +276,7 @@ describe AppsController do
276 276 IssueTracker.subclasses.each do |tracker_klass|
277 277 context tracker_klass do
278 278 it "should save tracker params" do
279   - params = tracker_klass::RequiredFields.inject({}){|hash,f| hash[f] = "test_value"; hash }
  279 + params = tracker_klass::Fields.inject({}){|hash,f| hash[f[0]] = "test_value"; hash }
280 280 params['ticket_properties'] = "card_type = defect" if tracker_klass == MingleTracker
281 281 params['type'] = tracker_klass.to_s
282 282 put :update, :id => @app.id, :app => {:issue_tracker_attributes => params}
... ... @@ -284,14 +284,17 @@ describe AppsController do
284 284 @app.reload
285 285 tracker = @app.issue_tracker
286 286 tracker.should be_a(tracker_klass)
287   - tracker_klass::RequiredFields.each do |required|
288   - tracker.send(required.to_sym).should == 'test_value'
  287 + tracker_klass::Fields.each do |field, field_info|
  288 + case field
  289 + when :ticket_properties; tracker.send(field.to_sym).should == 'card_type = defect'
  290 + else tracker.send(field.to_sym).should == 'test_value'
  291 + end
289 292 end
290 293 end
291 294  
292 295 it "should show validation notice when sufficient params are not present" do
293 296 # Leave out one required param
294   - params = tracker_klass::RequiredFields[1..-1].inject({}){|hash,f| hash[f] = "test_value"; hash }
  297 + params = tracker_klass::Fields[1..-1].inject({}){|hash,f| hash[f[0]] = "test_value"; hash }
295 298 params['type'] = tracker_klass.to_s
296 299 put :update, :id => @app.id, :app => {:issue_tracker_attributes => params}
297 300  
... ...
spec/factories/issue_tracker_factories.rb
... ... @@ -20,7 +20,7 @@ Factory.define :mingle_tracker, :parent =&gt; :issue_tracker, :class =&gt; :mingle_tra
20 20 e.ticket_properties 'card_type = Defect, defect_status = open, priority = essential'
21 21 end
22 22  
23   -Factory.define :github_tracker, :parent => :issue_tracker, :class => :github_tracker do |e|
  23 +Factory.define :github_issues_tracker, :parent => :issue_tracker, :class => :github_issues_tracker do |e|
24 24 e.project_id 'test_account/test_project'
25 25 e.username 'test_username'
26 26 end
... ...
spec/models/issue_trackers/github_issues_tracker_spec.rb 0 → 100644
... ... @@ -0,0 +1,41 @@
  1 +require 'spec_helper'
  2 +
  3 +describe GithubIssuesTracker do
  4 + it "should create an issue on Github Issues with err params, and set issue link for err" do
  5 + notice = Factory :notice
  6 + tracker = Factory :github_issues_tracker, :app => notice.err.app
  7 + err = notice.err
  8 +
  9 + number = 5
  10 + @issue_link = "https://github.com/#{tracker.project_id}/issues/#{number}"
  11 + body = <<EOF
  12 +{
  13 + "issue": {
  14 + "position": 1.0,
  15 + "number": #{number},
  16 + "votes": 0,
  17 + "created_at": "2010/01/21 13:45:59 -0800",
  18 + "comments": 0,
  19 + "body": "Test Body",
  20 + "title": "Test Issue",
  21 + "user": "test_user",
  22 + "state": "open",
  23 + "html_url": "#{@issue_link}"
  24 + }
  25 +}
  26 +EOF
  27 + stub_request(:post, "https://#{tracker.username}%2Ftoken:#{tracker.api_token}@github.com/api/v2/json/issues/open/#{tracker.project_id}").
  28 + to_return(:status => 201, :headers => {'Location' => @issue_link}, :body => body )
  29 +
  30 + err.app.issue_tracker.create_issue(err)
  31 + err.reload
  32 +
  33 + requested = have_requested(:post, "https://#{tracker.username}%2Ftoken:#{tracker.api_token}@github.com/api/v2/json/issues/open/#{tracker.project_id}")
  34 + WebMock.should requested.with(:headers => {'Content-Type' => 'application/x-www-form-urlencoded'})
  35 + WebMock.should requested.with(:body => /title=%5Bproduction%5D%5Bfoo%23bar%5D%20FooError%3A%20Too%20Much%20Bar/)
  36 + WebMock.should requested.with(:body => /See%20this%20exception%20on%20Errbit/)
  37 +
  38 + err.issue_link.should == @issue_link
  39 + end
  40 +end
  41 +
... ...
spec/models/issue_trackers/github_tracker_spec.rb
... ... @@ -1,41 +0,0 @@
1   -require 'spec_helper'
2   -
3   -describe GithubTracker do
4   - it "should create an issue on Github Issues with err params, and set issue link for err" do
5   - notice = Factory :notice
6   - tracker = Factory :github_tracker, :app => notice.err.app
7   - err = notice.err
8   -
9   - number = 5
10   - @issue_link = "https://github.com/#{tracker.project_id}/issues/#{number}"
11   - body = <<EOF
12   -{
13   - "issue": {
14   - "position": 1.0,
15   - "number": #{number},
16   - "votes": 0,
17   - "created_at": "2010/01/21 13:45:59 -0800",
18   - "comments": 0,
19   - "body": "Test Body",
20   - "title": "Test Issue",
21   - "user": "test_user",
22   - "state": "open",
23   - "html_url": "#{@issue_link}"
24   - }
25   -}
26   -EOF
27   - stub_request(:post, "https://#{tracker.username}%2Ftoken:#{tracker.api_token}@github.com/api/v2/json/issues/open/#{tracker.project_id}").
28   - to_return(:status => 201, :headers => {'Location' => @issue_link}, :body => body )
29   -
30   - err.app.issue_tracker.create_issue(err)
31   - err.reload
32   -
33   - requested = have_requested(:post, "https://#{tracker.username}%2Ftoken:#{tracker.api_token}@github.com/api/v2/json/issues/open/#{tracker.project_id}")
34   - WebMock.should requested.with(:headers => {'Content-Type' => 'application/x-www-form-urlencoded'})
35   - WebMock.should requested.with(:body => /title=%5Bproduction%5D%5Bfoo%23bar%5D%20FooError%3A%20Too%20Much%20Bar/)
36   - WebMock.should requested.with(:body => /See%20this%20exception%20on%20Errbit/)
37   -
38   - err.issue_link.should == @issue_link
39   - end
40   -end
41   -