Fix/improve issue tracker app edit

Gemfile

@@ -26,6 +26,7 @@ gem 'rails_autolink'
 # Please don't update hoptoad_notifier to airbrake.
 # It's for internal use only, and we monkeypatch certain methods
 gem 'hoptoad_notifier', "~> 2.4"
+gem 'draper', :require => false
 
 
 # Remove / comment out any of the gems below if you want to disable
@@ -103,9 +104,10 @@ group :development do
 end
 
 group :test do
-  gem 'capybara'
-  gem 'launchy'
-  gem 'database_cleaner'
+  gem 'capybara', :require => false
+  gem 'poltergeist', :require => false
+  gem 'launchy', :require => false
+  gem 'database_cleaner', :require => false
   gem 'email_spec'
   gem 'timecop'
   gem 'coveralls', :require => false

Gemfile.lock

@@ -1,10 +1,3 @@
-GIT
-  remote: https://github.com/NARKOZ/gitlab.git
-  revision: 7a00d38c53335010d2fb8a233247fe2c97338903
-  specs:
-    gitlab (2.2.0)
-      httparty
-
 GEM
   remote: https://rubygems.org/
   specs:
@@ -73,6 +66,7 @@ GEM
       rack (>= 1.0.0)
       rack-test (>= 0.5.4)
       xpath (~> 2.0)
+    cliver (0.2.2)
     coderay (1.0.9)
     columnize (0.3.6)
     coveralls (0.7.0)
@@ -102,6 +96,10 @@ GEM
       warden (~> 1.2.3)
     diff-lcs (1.2.4)
     dotenv (0.9.0)
+    draper (1.2.1)
+      actionpack (>= 3.0)
+      activesupport (>= 3.0)
+      request_store (~> 1.0.3)
     email_spec (1.5.0)
       launchy (~> 2.1)
       mail (~> 2.2)
@@ -118,6 +116,8 @@ GEM
     foreman (0.63.0)
       dotenv (>= 0.7)
       thor (>= 0.13.6)
+    gitlab (3.0.0)
+      httparty
     haml (4.0.3)
       tilt
     happymapper (0.4.1)
@@ -240,6 +240,11 @@ GEM
       rest-client (~> 1.6.0)
     pjax_rails (0.3.4)
       jquery-rails
+    poltergeist (1.4.1)
+      capybara (~> 2.1.0)
+      cliver (~> 0.2.1)
+      multi_json (~> 1.0)
+      websocket-driver (>= 0.2.0)
     polyglot (0.3.3)
     premailer (1.7.3)
       css_parser (>= 1.1.9)
@@ -287,6 +292,7 @@ GEM
     rdoc (3.12.2)
       json (~> 1.4)
     ref (1.0.5)
+    request_store (1.0.5)
     rest-client (1.6.7)
       mime-types (>= 1.16)
     ri_cal (0.8.8)
@@ -363,6 +369,7 @@ GEM
     webmock (1.15.0)
       addressable (>= 2.2.7)
       crack (>= 0.3.2)
+    websocket-driver (0.3.0)
     xmpp4r (0.5.5)
     xpath (2.0.0)
       nokogiri (~> 1.3)
@@ -387,12 +394,13 @@ DEPENDENCIES
   debugger
   decent_exposure
   devise
+  draper
   email_spec
   execjs
   fabrication
   flowdock
   foreman
-  gitlab!
+  gitlab (~> 3.0.0)
   haml
   hipchat
   hoi
@@ -413,6 +421,7 @@ DEPENDENCIES
   oruen_redmine_client
   pivotal-tracker
   pjax_rails
+  poltergeist
   pry-rails
   puma
   quiet_assets

app/controllers/apps_controller.rb

@@ -16,6 +16,9 @@ class AppsController < ApplicationController
   }
 
   expose(:app, :ancestor => :app_scope)
+  expose(:app_decorate) do
+    AppDecorator.new(app)
+  end
 
   expose(:all_errs) {
     !!params[:all_errs]
@@ -46,7 +49,6 @@ def new
   end
 
   def create
-    initialize_subclassed_issue_tracker
     initialize_subclassed_notification_service
     if app.save
       redirect_to app_url(app), :flash => { :success => I18n.t('controllers.apps.flash.create.success') }
@@ -57,7 +59,6 @@ def create
   end
 
   def update
-    initialize_subclassed_issue_tracker
     initialize_subclassed_notification_service
     if app.save
       redirect_to app_url(app), :flash => { :success => I18n.t('controllers.apps.flash.update.success') }
@@ -87,15 +88,6 @@ def regenerate_api_key
 
   protected
 
-    def initialize_subclassed_issue_tracker
-      # set the app's issue tracker
-      if params[:app][:issue_tracker_attributes] && tracker_type = params[:app][:issue_tracker_attributes][:type]
-        if IssueTracker.subclasses.map(&:name).concat(["IssueTracker"]).include?(tracker_type)
-          app.issue_tracker = tracker_type.constantize.new(params[:app][:issue_tracker_attributes])
-        end
-      end
-    end
-
     def initialize_subclassed_notification_service
       # set the app's notification service
       if params[:app][:notification_service_attributes] && notification_type = params[:app][:notification_service_attributes][:type]

app/decorators/app_decorator.rb

@@ -0,0 +1,19 @@
+class AppDecorator < Draper::Decorator
+
+  decorates_association :watchers
+  decorates_association :issue_tracker, :with => IssueTrackerDecorator
+  delegate_all
+
+  def email_at_notices
+    object.email_at_notices.join(', ')
+  end
+
+  def notify_user_display
+    object.notify_all_users ? 'display: none;' : ''
+  end
+
+  def notify_err_display
+    object.notify_on_errs ? '' : 'display: none;'
+  end
+
+end

app/decorators/issue_tracker_decorator.rb

@@ -0,0 +1,35 @@
+class IssueTrackerDecorator < Draper::Decorator
+
+  delegate_all
+
+  def issue_trackers
+    @issue_trackers ||= [
+      IssueTracker::None,
+      IssueTracker.subclasses.select{|klass| klass != IssueTracker::None }
+    ].flatten
+    @issue_trackers.each do |it|
+      yield IssueTrackerDecorator.new(it)
+    end
+  end
+
+  def note
+    object::Note.html_safe
+  end
+
+  def fields
+    object::Fields.each do |field, field_info|
+      yield IssueTrackerFieldDecorator.new(field, field_info)
+    end
+  end
+
+  def params_class(tracker)
+    [choosen?(tracker), label].join(" ").strip
+  end
+
+  private
+
+  def choosen?(issue_tracker)
+    object.to_s == issue_tracker._type ? 'chosen' : ''
+  end
+
+end

app/decorators/issue_tracker_field_decorator.rb

@@ -0,0 +1,27 @@
+class IssueTrackerFieldDecorator < Draper::Decorator
+
+  def initialize(field, field_info)
+    @object = field
+    @field_info = field_info
+  end
+  attr_reader :object, :field_info
+
+  alias :key :object
+
+  def label
+    field_info[:label] || object.to_s.titleize
+  end
+
+
+  def input(form)
+    form.send(input_field, object,
+              :placeholder => field_info[:placeholder],
+              :value => form.object.send(object))
+  end
+
+  private
+
+  def input_field
+    object == :password ? :password_field : :text_field
+  end
+end

app/decorators/watcher_decorator.rb

@@ -0,0 +1,9 @@
+class WatcherDecorator < Draper::Decorator
+
+  delegate_all
+
+  def email_choosen
+    object.email.blank? ? 'chosen' : ''
+  end
+
+end

app/models/fingerprint.rb

@@ -29,7 +29,13 @@ def fingerprint_source
   end
 
   def file_or_message
-    @file_or_message ||= notice.message + notice.backtrace.fingerprint
+    @file_or_message ||= unified_message + notice.backtrace.fingerprint
+  end
+
+  # filter memory addresses out of object strings
+  # example: "#<Object:0x007fa2b33d9458>" becomes "#<Object>"
+  def unified_message
+    notice.message.gsub(/(#<.+?):[0-9a-f]x[0-9a-f]+(>)/, '\1\2')
   end
 
 end

app/models/issue_tracker.rb

@@ -3,6 +3,9 @@ class IssueTracker
   include Mongoid::Timestamps
   include HashHelper
   include Rails.application.routes.url_helpers
+
+  Note = ''
+
   default_url_options[:host] = ActionMailer::Base.default_url_options[:host]
 
   embedded_in :app, :inverse_of => :issue_tracker
@@ -41,7 +44,6 @@ def type=(t); self._type=t; end
   def url; nil; end
 
   # Retrieve tracker label from either class or instance.
-  Label = ''
   def self.label; self::Label; end
   def label; self.class.label; end
 

app/models/issue_trackers/none.rb

@@ -0,0 +1,5 @@
+class IssueTrackers::None < IssueTracker
+  Fields = []
+  Note = 'When no issue tracker has been configured, you will be able to leave comments on errors.'
+  Label = 'none'
+end

app/views/apps/_fields.html.haml

@@ -29,9 +29,9 @@
     = f.check_box :notify_on_errs, 'data-show-when-checked' => '.email_at_notices_nested'
     = f.label :notify_on_errs, 'Notify on errors'
   - if Errbit::Config.per_app_email_at_notices
-    %div.email_at_notices_nested{:style => f.object.notify_on_errs ? '' : 'display: none;'}
+    %div.email_at_notices_nested{:style => app_decorate.notify_err_display}
       .field-helpertext Send a notification every
-      = f.text_field :email_at_notices, :value => f.object.email_at_notices.join(", ")
+      = f.text_field :email_at_notices, :value => app_decorate.email_at_notices
       .field-helpertext times an error occurs (comma separated).
   %div.checkbox
     = f.check_box :notify_on_deploys
@@ -42,16 +42,16 @@
   = f.label :notify_all_users, 'Send notifications to all users'
 
 
-%fieldset.watchers.nested-wrapper{:style => f.object.notify_all_users ? 'display: none;' : ''}
+%fieldset.watchers.nested-wrapper{:style => app_decorate.notify_user_display}
   %legend Watchers
   = f.fields_for :watchers do |w|
     %div.watcher.nested
       %div.choose
         = w.radio_button :watcher_type, :user
-        = label_tag :watcher_type_user, 'User', :for => label_for_attr(w, 'watcher_type_user')
+        = w.label :watcher_type_user, 'User'
         = w.radio_button :watcher_type, :email
-        = label_tag :watcher_type_email, 'Email Address', :for => label_for_attr(w, 'watcher_type_email')
-      %div.watcher_params.user{:class => w.object.email.blank? ? 'chosen' : nil}
+        = w.label :watcher_type_email, 'Email Address'
+      %div.watcher_params.user{:class => w.object.email_choosen}
         = w.select :user_id, User.all.map{|u| [u.name,u.id.to_s]}, :include_blank => '-- Select a User --'
       %div.watcher_params.email{:class => w.object.email.present? ? 'chosen' : nil}
         = w.text_field :email

app/views/apps/_issue_tracker_fields.html.haml

@@ -1,29 +1,22 @@
 %fieldset
-  %legend Issue tracker
+  %legend= t('.legend')
   = f.fields_for :issue_tracker do |w|
     %div.issue_tracker.nested
       %div.choose
-        = label_tag :type_none, :for => label_for_attr(w, 'type_issuetracker'), :class => "label_radio none" do
-          = w.radio_button :type, "IssueTracker", 'data-section' => 'none'
-          (None)
-        - IssueTracker.subclasses.each do |tracker|
-          = label_tag "type_#{tracker.label}:", :for => label_for_attr(w, "type_#{tracker.name.downcase.gsub(':','')}"), :class => "label_radio #{tracker.label}" do
+        - w.object.issue_trackers do |tracker|
+          = w.label :type, :class => "label_radio #{tracker.label}", :value => tracker.name do
             = w.radio_button :type, tracker.name, 'data-section' => tracker.label
-            = tracker.name[/::(.*)Tracker/,1].titleize
+            = tracker.label
 
-      %div.tracker_params.none{:class => (w.object && !(w.object.class < IssueTracker)) ? 'chosen' : nil}
-        %p When no issue tracker has been configured, you will be able to leave comments on errors.
-      - IssueTracker.subclasses.each do |tracker|
-        %div.tracker_params{:class => (w.object.is_a?(tracker) ? 'chosen ' : '') << tracker.label}
-          - if defined?(tracker::Note)
-            %p= tracker::Note.html_safe
-          - tracker::Fields.each do |field, field_info|
-            = w.label field, field_info[:label] || field.to_s.titleize
-            - field_type = field == :password ? :password_field : :text_field
-            = w.send field_type, field, :placeholder => field_info[:placeholder], :value => w.object.send(field)
+      - w.object.issue_trackers do |tracker|
+        %div.tracker_params{:class => tracker.params_class(w.object)}
+          %p= tracker.note
+          - tracker.fields do |field|
+            = w.label field.key, field.label
+            = field.input(w)
 
       .image_preloader
-        - (IssueTracker.subclasses.map{|t| t.label } << 'none').each do |tracker|
-          = image_tag "#{tracker}_inactive.png"
-          = image_tag "#{tracker}_create.png"
+        - w.object.issue_trackers do |tracker|
+          = image_tag "#{tracker.label}_inactive.png"
+          = image_tag "#{tracker.label}_create.png"
 

app/views/apps/edit.html.haml

@@ -1,12 +1,12 @@
-- content_for :title, 'Edit App'
+- content_for :title, t('.title')
 - content_for :action_bar do
   = link_to_copy_attributes_from_other_app
-  = link_to 'destroy application', app_path(app), :method => :delete, :data => { :confirm => 'Seriously?' }, :class => 'button'
-  = link_to('cancel', app_path(app), :class => 'button')
+  = link_to t('.destroy'), app_path(app), :method => :delete, :data => { :confirm => t('.seriously') }, :class => 'button'
+  = link_to(t('.cancel'), app_path(app), :class => 'button')
 
-= form_for app do |f|
+= form_for app_decorate do |f|
 
   = render 'fields', :f => f
 
-  %div.buttons= f.submit 'Update App'
+  %div.buttons= f.submit t('.update')
 

app/views/apps/new.html.haml

@@ -1,11 +1,11 @@
-- content_for :title, 'Add App'
+- content_for :title, t('.title')
 - content_for :action_bar do
   = link_to_copy_attributes_from_other_app
-  = link_to('cancel', apps_path, :class => 'button')
+  = link_to(t('.cancel'), apps_path, :class => 'button')
 
-= form_for app do |f|
+= form_for app_decorate do |f|
 
   = render 'fields', :f => f
 
-  %div.buttons= f.submit 'Add App'
+  %div.buttons= f.submit t('.add_app')