Refactor the Issue Tracker forms in App

Cyril Mougel
1 parent ed78ea13
Showing 21 changed files with 261 additions and 30 deletions Show diff stats
Gemfile
Gemfile.lock
app/controllers/apps_controller.rb
app/decorators/app_decorator.rb
app/decorators/issue_tracker_decorator.rb
app/decorators/issue_tracker_field_decorator.rb
app/decorators/watcher_decorator.rb
app/models/issue_tracker.rb
app/views/apps/_fields.html.haml
app/views/apps/_issue_tracker_fields.html.haml
app/views/apps/edit.html.haml
app/views/apps/new.html.haml
config/application.rb
config/locales/en.yml
spec/acceptance/app_regenerate_api_key_spec.rb
spec/decorators/app_decorator_spec.rb
spec/decorators/issue_tracker_decorator_spec.rb
spec/decorators/issue_tracker_field_decorator.rb
spec/decorators/watcher_decorator_spec.rb
spec/views/apps/edit.html.haml_spec.rb
@@ -25,6 +25,7 @@ gem &#39;rails_autolink&#39;
 # 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
@@ -95,6 +95,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)
@@ -282,6 +286,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)
@@ -382,6 +387,7 @@ DEPENDENCIES
   debugger
   decent_exposure
   devise
+  draper
   email_spec
   execjs
   fabrication
@@ -16,6 +16,9 @@ class AppsController &lt; ApplicationController
   }
   expose(:app, :ancestor => :app_scope)
+  expose(:app_decorate) do
+    AppDecorator.new(app)
+  end
   expose(:all_errs) {
     !!params[:all_errs]
@@ -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
@@ -0,0 +1,35 @@
+class IssueTrackerDecorator < Draper::Decorator
+
+  delegate_all
+
+  def issue_trackers
+    @issue_trackers ||= [
+        IssueTracker,
+        IssueTracker.subclasses
+    ].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
@@ -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
@@ -0,0 +1,9 @@
+class WatcherDecorator < Draper::Decorator
+
+  delegate_all
+
+  def email_choosen
+    object.email.blank? ? 'chosen' : ''
+  end
+
+end
@@ -3,6 +3,11 @@ class IssueTracker
   include Mongoid::Timestamps
   include HashHelper
   include Rails.application.routes.url_helpers
+
+  Fields = []
+  Note = 'When no issue tracker has been configured, you will be able to leave comments on errors.'
+  Label = 'none'
+
   default_url_options[:host] = ActionMailer::Base.default_url_options[:host]
   embedded_in :app, :inverse_of => :issue_tracker
@@ -41,7 +46,6 @@ class IssueTracker
   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
@@ -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
 %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"
@@ -5,7 +5,7 @@
     :data => { :confirm => t('apps.confirm_delete') }, :class => 'button'
   = link_to('cancel', app_path(app), :class => 'button')
-= form_for app do |f|
+= form_for app_decorate do |f|
   = render 'fields', :f => f
@@ -3,7 +3,7 @@
   = link_to_copy_attributes_from_other_app
   = link_to('cancel', apps_path, :class => 'button')
-= form_for app do |f|
+= form_for app_decorate do |f|
   = render 'fields', :f => f
@@ -9,6 +9,8 @@ require &quot;action_mailer/railtie&quot;
 require 'mongoid/railtie'
 require "sprockets/railtie"
+require 'draper'
+
 if defined?(Bundler)
   # If you precompile assets before deploying to production, use this line
   Bundler.require(*Rails.groups(:assets => %w(development test)))
@@ -125,4 +125,6 @@ en:
       watchers: Watchers
       when:  When
       who: Who
+    issue_tracker_fields:
+      legend: Issue Tracker
@@ -28,5 +28,4 @@ feature &quot;Regeneration api_Key&quot; do
     click_link app.name if page.current_url != app_url(app)
     expect(page).to_not have_button I18n.t('apps.show.edit')
   end
-
 end
@@ -0,0 +1,37 @@
+require 'spec_helper'
+
+describe AppDecorator do
+
+  describe "#email_at_notices" do
+
+    it 'return the list separate by comma' do
+      expect(AppDecorator.new(double(:email_at_notices => [2,3])).email_at_notices).to eql '2, 3'
+    end
+
+  end
+
+  describe "#notify_user_display" do
+
+    it 'return display:none if notify' do
+      expect(AppDecorator.new(double(:notify_all_users => true)).notify_user_display).to eql 'display: none;'
+    end
+
+    it 'return blank if no notify' do
+      expect(AppDecorator.new(double(:notify_all_users => false)).notify_user_display).to eql ''
+    end
+
+  end
+
+  describe "#notify_err_display" do
+
+    it 'return display:none if no notify' do
+      expect(AppDecorator.new(double(:notify_on_errs => false)).notify_err_display).to eql 'display: none;'
+    end
+
+    it 'return blank if no notify' do
+      expect(AppDecorator.new(double(:notify_on_errs => true)).notify_err_display).to eql ''
+    end
+
+  end
+
+end
@@ -0,0 +1,59 @@
+require 'spec_helper'
+
+describe IssueTrackerDecorator do
+
+    class Foo
+      Note = 'hello <strong></strong>'
+      Fields = [
+        [:foo, :bar]
+      ]
+      Label = 'foo'
+      def self.label; Label; end
+      def _type
+        'Foo'
+      end
+    end
+
+    class Bar
+      Label = 'bar'
+      def self.label; Label; end
+      def _type
+        'Bar'
+      end
+    end
+
+  describe "#note" do
+
+
+    it 'return the html_safe of Note' do
+      expect(IssueTrackerDecorator.new(Foo).note).to eql 'hello <strong></strong>'.html_safe
+    end
+  end
+
+  describe "#issue_trackers" do
+    it 'return an array of IssueTrackerDecorator' do
+      IssueTrackerDecorator.new(Foo).issue_trackers do |it|
+        expect(it).to be_a(IssueTrackerDecorator)
+      end
+    end
+  end
+
+  describe "#fields" do
+    it 'return all Fields define decorate' do
+      IssueTrackerDecorator.new(Foo).fields do |itf|
+        expect(itf).to be_a(IssueTrackerFieldDecorator)
+        expect(itf.object).to eql :foo
+        expect(itf.field_info).to eql :bar
+      end
+    end
+  end
+
+  describe "#params_class" do
+    it 'add the label in class' do
+      expect(IssueTrackerDecorator.new(Bar).params_class(Foo.new)).to eql 'bar'
+    end
+    it 'add chosen class if _type is same' do
+      expect(IssueTrackerDecorator.new(Foo).params_class(Foo.new)).to eql 'chosen foo'
+    end
+  end
+end
@@ -0,0 +1,14 @@
+require 'spec_helper'
+
+describe IssueTrackerFieldDecorator do
+
+  describe "#label" do
+    it 'return the label of field_info by default' do
+      expect(IssueTrackerFieldDecorator.new(:foo, {:label => 'hello'}).label).to eq 'hello'
+    end
+    it 'return the key of field if no label define' do
+      expect(IssueTrackerFieldDecorator.new(:foo, {}).label).to eq 'foo'
+    end
+  end
+
+end
@@ -0,0 +1,17 @@
+require 'spec_helper'
+
+describe WatcherDecorator do
+  describe "#email_choosen" do
+    context "with email define" do
+      it 'return blank' do
+        expect(WatcherDecorator.new(double(:email => 'foo')).email_choosen).to eql ''
+      end
+    end
+
+    context "without email define" do
+      it 'return choosen' do
+        expect(WatcherDecorator.new(double(:email => '')).email_choosen).to eql 'chosen'
+      end
+    end
+  end
+end
@@ -2,8 +2,10 @@ require &#39;spec_helper&#39;
 describe "apps/edit.html.haml" do
   let(:app) { stub_model(App) }
+  let(:app_decorate) { AppDecorator.new(app) }
   before do
     view.stub(:app).and_return(app)
+    view.stub(:app_decorate).and_return(app_decorate)
     controller.stub(:current_user) { stub_model(User) }
   end
@@ -2,8 +2,10 @@ require &#39;spec_helper&#39;
 describe "apps/new.html.haml" do
   let(:app) { stub_model(App) }
+  let(:app_decorate) { AppDecorator.new(app) }
   before do
     view.stub(:app).and_return(app)
+    view.stub(:app_decorate).and_return(app_decorate)
     controller.stub(:current_user) { stub_model(User) }
   end
	@@ -25,6 +25,7 @@ gem 'rails_autolink'		@@ -25,6 +25,7 @@ gem 'rails_autolink'
25	# Please don't update hoptoad_notifier to airbrake.	25	# Please don't update hoptoad_notifier to airbrake.
26	# It's for internal use only, and we monkeypatch certain methods	26	# It's for internal use only, and we monkeypatch certain methods
27	gem 'hoptoad_notifier', "~> 2.4"	27	gem 'hoptoad_notifier', "~> 2.4"
		28	+gem 'draper', :require => false
28		29
29		30
30	# Remove / comment out any of the gems below if you want to disable	31	# Remove / comment out any of the gems below if you want to disable
	@@ -95,6 +95,10 @@ GEM		@@ -95,6 +95,10 @@ GEM
95	warden (~> 1.2.3)	95	warden (~> 1.2.3)
96	diff-lcs (1.2.4)	96	diff-lcs (1.2.4)
97	dotenv (0.9.0)	97	dotenv (0.9.0)
		98	+ draper (1.2.1)
		99	+ actionpack (>= 3.0)
		100	+ activesupport (>= 3.0)
		101	+ request_store (~> 1.0.3)
98	email_spec (1.5.0)	102	email_spec (1.5.0)
99	launchy (~> 2.1)	103	launchy (~> 2.1)
100	mail (~> 2.2)	104	mail (~> 2.2)
	@@ -282,6 +286,7 @@ GEM		@@ -282,6 +286,7 @@ GEM
282	rdoc (3.12.2)	286	rdoc (3.12.2)
283	json (~> 1.4)	287	json (~> 1.4)
284	ref (1.0.5)	288	ref (1.0.5)
		289	+ request_store (1.0.5)
285	rest-client (1.6.7)	290	rest-client (1.6.7)
286	mime-types (>= 1.16)	291	mime-types (>= 1.16)
287	ri_cal (0.8.8)	292	ri_cal (0.8.8)
	@@ -382,6 +387,7 @@ DEPENDENCIES		@@ -382,6 +387,7 @@ DEPENDENCIES
382	debugger	387	debugger
383	decent_exposure	388	decent_exposure
384	devise	389	devise
		390	+ draper
385	email_spec	391	email_spec
386	execjs	392	execjs
387	fabrication	393	fabrication
@@ -0,0 +1,19 @@		@@ -0,0 +1,19 @@
	1	+class AppDecorator < Draper::Decorator
	2	+
	3	+ decorates_association :watchers
	4	+ decorates_association :issue_tracker, :with => IssueTrackerDecorator
	5	+ delegate_all
	6	+
	7	+ def email_at_notices
	8	+ object.email_at_notices.join(', ')
	9	+ end
	10	+
	11	+ def notify_user_display
	12	+ object.notify_all_users ? 'display: none;' : ''
	13	+ end
	14	+
	15	+ def notify_err_display
	16	+ object.notify_on_errs ? '' : 'display: none;'
	17	+ end
	18	+
	19	+end
@@ -0,0 +1,35 @@		@@ -0,0 +1,35 @@
	1	+class IssueTrackerDecorator < Draper::Decorator
	2	+
	3	+ delegate_all
	4	+
	5	+ def issue_trackers
	6	+ @issue_trackers \|\|= [
	7	+ IssueTracker,
	8	+ IssueTracker.subclasses
	9	+ ].flatten
	10	+ @issue_trackers.each do \|it\|
	11	+ yield IssueTrackerDecorator.new(it)
	12	+ end
	13	+ end
	14	+
	15	+ def note
	16	+ object::Note.html_safe
	17	+ end
	18	+
	19	+ def fields
	20	+ object::Fields.each do \|field, field_info\|
	21	+ yield IssueTrackerFieldDecorator.new(field, field_info)
	22	+ end
	23	+ end
	24	+
	25	+ def params_class(tracker)
	26	+ [choosen?(tracker), label].join(" ").strip
	27	+ end
	28	+
	29	+ private
	30	+
	31	+ def choosen?(issue_tracker)
	32	+ object.to_s == issue_tracker._type ? 'chosen' : ''
	33	+ end
	34	+
	35	+end
@@ -0,0 +1,27 @@		@@ -0,0 +1,27 @@
	1	+class IssueTrackerFieldDecorator < Draper::Decorator
	2	+
	3	+ def initialize(field, field_info)
	4	+ @object = field
	5	+ @field_info = field_info
	6	+ end
	7	+ attr_reader :object, :field_info
	8	+
	9	+ alias :key :object
	10	+
	11	+ def label
	12	+ field_info[:label] \|\| object.to_s.titleize
	13	+ end
	14	+
	15	+
	16	+ def input(form)
	17	+ form.send(input_field, object,
	18	+ :placeholder => field_info[:placeholder],
	19	+ :value => form.object.send(object))
	20	+ end
	21	+
	22	+ private
	23	+
	24	+ def input_field
	25	+ object == :password ? :password_field : :text_field
	26	+ end
	27	+end
@@ -0,0 +1,9 @@		@@ -0,0 +1,9 @@
	1	+class WatcherDecorator < Draper::Decorator
	2	+
	3	+ delegate_all
	4	+
	5	+ def email_choosen
	6	+ object.email.blank? ? 'chosen' : ''
	7	+ end
	8	+
	9	+end
	@@ -5,7 +5,7 @@		@@ -5,7 +5,7 @@
5	:data => { :confirm => t('apps.confirm_delete') }, :class => 'button'	5	:data => { :confirm => t('apps.confirm_delete') }, :class => 'button'
6	= link_to('cancel', app_path(app), :class => 'button')	6	= link_to('cancel', app_path(app), :class => 'button')
7		7
8	-= form_for app do \|f\|	8	+= form_for app_decorate do \|f\|
9		9
10	= render 'fields', :f => f	10	= render 'fields', :f => f
11		11
	@@ -3,7 +3,7 @@		@@ -3,7 +3,7 @@
3	= link_to_copy_attributes_from_other_app	3	= link_to_copy_attributes_from_other_app
4	= link_to('cancel', apps_path, :class => 'button')	4	= link_to('cancel', apps_path, :class => 'button')
5		5
6	-= form_for app do \|f\|	6	+= form_for app_decorate do \|f\|
7		7
8	= render 'fields', :f => f	8	= render 'fields', :f => f
9		9
	@@ -125,4 +125,6 @@ en:		@@ -125,4 +125,6 @@ en:
125	watchers: Watchers	125	watchers: Watchers
126	when: When	126	when: When
127	who: Who	127	who: Who
		128	+ issue_tracker_fields:
		129	+ legend: Issue Tracker
128		130
	@@ -28,5 +28,4 @@ feature "Regeneration api_Key" do		@@ -28,5 +28,4 @@ feature "Regeneration api_Key" do
28	click_link app.name if page.current_url != app_url(app)	28	click_link app.name if page.current_url != app_url(app)
29	expect(page).to_not have_button I18n.t('apps.show.edit')	29	expect(page).to_not have_button I18n.t('apps.show.edit')
30	end	30	end
31	-
32	end	31	end