Merge pull request #774 from errbit/refacor_issue_tracker

Refacorting issue tracker

Gemfile

@@ -27,8 +27,8 @@ gem 'rails_autolink'
 gem 'hoptoad_notifier', "~> 2.4"
 gem 'draper', :require => false
 
-gem 'errbit_plugin'
-gem 'errbit_github_plugin'
+gem 'errbit_plugin', github: 'errbit/errbit_plugin'
+gem 'errbit_github_plugin', github: 'errbit/errbit_github_plugin'
 
 # Notification services
 # ---------------------------------------

Gemfile.lock

@@ -1,3 +1,17 @@
+GIT
+  remote: git://github.com/errbit/errbit_github_plugin.git
+  revision: 5900200e6d460fe94feaea3e59824437d54f1029
+  specs:
+    errbit_github_plugin (0.1.0)
+      errbit_plugin
+      octokit
+
+GIT
+  remote: git://github.com/errbit/errbit_plugin.git
+  revision: 6a0b93d71a6b4545198f0c68b99b8e8f9281c17e
+  specs:
+    errbit_plugin (0.4.0)
+
 GEM
   remote: https://rubygems.org/
   specs:
@@ -88,10 +102,6 @@ GEM
     email_spec (1.5.0)
       launchy (~> 2.1)
       mail (~> 2.2)
-    errbit_github_plugin (0.1.0)
-      errbit_plugin
-      octokit
-    errbit_plugin (0.4.0)
     erubis (2.7.0)
     execjs (2.0.2)
     fabrication (2.9.0)
@@ -181,8 +191,8 @@ GEM
       jwt (~> 0.1.4)
       multi_json (~> 1.0)
       rack (~> 1.2)
-    octokit (3.3.1)
-      sawyer (~> 0.5.3)
+    octokit (3.7.0)
+      sawyer (~> 0.6.0, >= 0.5.3)
     omniauth (1.1.4)
       hashie (>= 1.2, < 3)
       rack
@@ -267,7 +277,7 @@ GEM
       json
       rest-client
     safe_yaml (1.0.4)
-    sawyer (0.5.5)
+    sawyer (0.6.0)
       addressable (~> 2.3.5)
       faraday (~> 0.8, < 0.10)
     simplecov (0.7.1)
@@ -334,8 +344,8 @@ DEPENDENCIES
   devise
   draper
   email_spec
-  errbit_github_plugin
-  errbit_plugin
+  errbit_github_plugin!
+  errbit_plugin!
   execjs
   fabrication
   flowdock

app/controllers/problems_controller.rb

@@ -6,7 +6,6 @@
 # COLLECTION => :index, :all, :destroy_several, :resolve_several, :unresolve_several, :merge_several, :unmerge_several, :search
 class ProblemsController < ApplicationController
 
-
   include ProblemsSearcher
 
   before_filter :need_selected_problem, :only => [
@@ -62,10 +61,12 @@ def show
   end
 
   def create_issue
-    issue_creation = IssueCreation.new(problem, current_user, params[:tracker], request)
+    body = render_to_string "issue_trackers/issue", layout: false, formats: [:txt]
+    title = "[#{ problem.environment }][#{ problem.where }] #{problem.message.to_s.truncate(100)}"
 
-    unless issue_creation.execute
-      flash[:error] = issue_creation.errors.full_messages.join(', ')
+    issue = Issue.new(problem: problem, user: current_user, title: title, body: body)
+    unless issue.save
+      flash[:error] = issue.errors.full_messages.join(', ')
     end
 
     redirect_to app_problem_path(app, problem)

app/models/issue.rb

@@ -0,0 +1,31 @@
+class Issue
+  include ActiveModel::Model
+  attr_accessor :problem, :user, :title, :body
+
+  def intialize(problem: nil, user: nil, title: nil, body: nil)
+    @problem, @user, @title, @body = problem, user, title, body
+  end
+
+  def issue_tracker
+    problem.app.issue_tracker
+  end
+
+  def save
+    if issue_tracker
+      issue_tracker.tracker.errors.each do |k, err|
+        errors.add k, err
+      end
+      return false if errors.present?
+
+      url = issue_tracker.create_issue(title, body, user: user.as_document)
+      problem.update_attributes(issue_link: url, issue_type: issue_tracker.tracker.class.label)
+    else
+      errors.add :base, "This app has no issue tracker setup."
+    end
+
+    errors.empty?
+  rescue => ex
+    errors.add :base, "There was an error during issue creation: #{ex.message}"
+    false
+  end
+end

app/models/issue_tracker.rb

@@ -2,34 +2,19 @@ class IssueTracker
   include Mongoid::Document
   include Mongoid::Timestamps
 
-  include Rails.application.routes.url_helpers
-
-  default_url_options[:host] = ActionMailer::Base.default_url_options[:host]
-  default_url_options[:port] = ActionMailer::Base.default_url_options[:port]
-
   embedded_in :app, :inverse_of => :issue_tracker
 
   field :type_tracker, :type => String
   field :options, :type => Hash, :default => {}
 
   validate :validate_tracker
 
-  ##
-  # Update default_url_option with valid data from the request information
-  #
-  # @param [ Request ] a request with host, port and protocol
-  #
-  def self.update_url_options(request)
-    IssueTracker.default_url_options[:host] = request.host
-    IssueTracker.default_url_options[:port] = request.port
-    IssueTracker.default_url_options[:protocol] = request.scheme
-  end
-
   def tracker
-    klass = ErrbitPlugin::Registry.issue_trackers[self.type_tracker]
-    klass = ErrbitPlugin::NoneIssueTracker unless klass
-
-    @tracker = klass.new(app, self.options)
+    @tracker ||=
+      begin
+        klass = ErrbitPlugin::Registry.issue_trackers[self.type_tracker] || ErrbitPlugin::NoneIssueTracker
+        klass.new(options.merge(github_repo: app.github_repo, bitbucket_repo: app.bitbucket_repo))
+      end
   end
 
   # Allow the tracker to validate its own params

app/views/issue_trackers/issue.txt.erb

@@ -0,0 +1,45 @@
+[See this exception on Errbit](<%= app_problem_url problem.app, problem %> "See this exception on Errbit")
+<% if notice = problem.notices.first %>
+# <%= notice.message %> #
+## Summary ##
+<% if notice.request['url'].present? %>
+### URL ###
+[<%= notice.request['url'] %>](<%= notice.request['url'] %>)"
+<% end %>
+### Where ###
+<%= notice.where %>
+
+### Occured ###
+<%= notice.created_at.to_s(:micro) %>
+
+### Similar ###
+<%= (notice.problem.notices_count - 1).to_s %>
+
+## Params ##
+```
+<%= pretty_hash(notice.params) %>
+```
+
+## Session ##
+```
+<%= pretty_hash(notice.session) %>
+```
+
+## Backtrace ##
+```
+<% notice.backtrace_lines.each do |line| %><%= line.number %>:  <%= line.file_relative %> -> **<%= line.method %>**
+<% end %>
+```
+
+## Environment ##
+
+<table>
+<% for key, val in notice.env_vars %>
+  <tr>
+    <td><%= key %>:</td>
+    <td><%= val %></td>
+  </tr>
+<% end %>
+</table>
+<% end %>
+

app/views/problems/_issue_tracker_links.html.haml

@@ -1,15 +1,9 @@
-- if app.issue_tracker_configured? || current_user.github_account?
+- if app.issue_tracker_configured?
   - if problem.issue_link.present?
     %span= link_to 'go to issue', problem.issue_link, :class => "#{problem.issue_type}_goto goto-issue"
     = link_to 'unlink issue', unlink_issue_app_problem_path(app, problem), :method => :delete, :data => { :confirm => "Unlink err issues?" }, :class => "unlink-issue"
   - elsif problem.issue_link == "pending"
     %span.disabled= link_to 'creating...', '#', :class => "#{problem.issue_type}_inactive create-issue"
     = link_to 'retry', create_issue_app_problem_path(app, problem), :method => :post
   - else
-    - if app.issue_tracker_configured? && !app.issue_tracker.type_tracker.eql?('github')
-      %span= link_to 'create issue', create_issue_app_problem_path(app, problem), :method => :post, :class => "#{app.issue_tracker.type_tracker}_create create-issue"
-    - elsif app.github_repo?
-      - if current_user.can_create_github_issues?
-        %span= link_to 'create issue', create_issue_app_problem_path(app, problem, :tracker => 'user_github'), :method => :post, :class => "github_create create-issue"
-      - elsif app.issue_tracker_configured? && app.issue_tracker.type_tracker.eql?('github')
-        %span= link_to 'create issue', create_issue_app_problem_path(app, problem), :method => :post, :class => "github_create create-issue"
+    %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 @@
     click_on I18n.t('apps.index.new_app')
     fill_in 'app_name', :with => 'My new app'
     find('.label_radio.github').click
+
+    fill_in 'app_github_repo', with: 'foo/bar'
     within ".github.tracker_params" do
       fill_in 'app_issue_tracker_attributes_options_username', :with => 'token'
       fill_in 'app_issue_tracker_attributes_options_password', :with => 'pass'

spec/controllers/apps_controller_spec.rb

@@ -308,42 +308,6 @@
             expect(@app.issue_tracker_configured?).to eq false
           end
         end
-
-        ErrbitPlugin::Registry.issue_trackers.each do |key, klass|
-          context key do
-            it "should save tracker params" do
-              params = {
-                :options => klass.fields.inject({}){|hash,f| hash[f[0]] = "test_value"; hash },
-                :type_tracker => key.dup.to_s
-              }
-              put :update, :id => @app.id, :app => {:issue_tracker_attributes => params}
-
-              @app.reload
-
-              tracker = @app.issue_tracker
-              expect(tracker.tracker).to be_a(ErrbitPlugin::Registry.issue_trackers[key])
-              klass.fields.each do |field, field_info|
-                case field
-                when :ticket_properties; tracker.send(field.to_sym).should == 'card_type = defect'
-                else tracker.options[field.to_s].should == 'test_value'
-                end
-              end
-            end
-
-            it "should show validation notice when sufficient params are not present" do
-              # Leave out one required param
-              # TODO. previous test was not relevant because one params can be enough. So put noone
-              put :update, :id => @app.id, :app => {
-                :issue_tracker_attributes => {
-                  :type_tracker => key.dup.to_s
-                }
-              }
-
-              @app.reload
-              expect(@app.issue_tracker_configured?).to eq false
-            end
-          end
-        end
       end
     end