Download as
Browse Code »

Commit a3dbf209ad7779d5b1d121b48a392a9cc8c82aaa

Authored by Cyril Mougel
1 parent 889af62f
Exists in master and in 1 other branch production

Simplify AppsController 

 * avoid a filter to parse IssueTracker params
 * Add acceptance test with phantomjs to be sure the Apps editing works
   fine with IssueTracker
 * Add IssueTrackers::None the IssueTracker configure doing nothing
Showing 13 changed files with 110 additions and 31 deletions   Show diff stats
@@ -103,9 +103,10 @@ group :development do @@ -103,9 +103,10 @@ group :development do
103 end 103 end
104 104
105 group :test do 105 group :test do
106 - gem 'capybara'  
107 - gem 'launchy'  
108 - gem 'database_cleaner' 106 + gem 'capybara', :require => false
  107 + gem 'poltergeist', :require => false
  108 + gem 'launchy', :require => false
  109 + gem 'database_cleaner', :require => false
109 gem 'email_spec' 110 gem 'email_spec'
110 gem 'timecop' 111 gem 'timecop'
111 gem 'coveralls', :require => false 112 gem 'coveralls', :require => false
@@ -66,6 +66,7 @@ GEM @@ -66,6 +66,7 @@ GEM
66 rack (>= 1.0.0) 66 rack (>= 1.0.0)
67 rack-test (>= 0.5.4) 67 rack-test (>= 0.5.4)
68 xpath (~> 2.0) 68 xpath (~> 2.0)
  69 + cliver (0.2.2)
69 coderay (1.0.9) 70 coderay (1.0.9)
70 columnize (0.3.6) 71 columnize (0.3.6)
71 coveralls (0.7.0) 72 coveralls (0.7.0)
@@ -239,6 +240,11 @@ GEM @@ -239,6 +240,11 @@ GEM
239 rest-client (~> 1.6.0) 240 rest-client (~> 1.6.0)
240 pjax_rails (0.3.4) 241 pjax_rails (0.3.4)
241 jquery-rails 242 jquery-rails
  243 + poltergeist (1.4.1)
  244 + capybara (~> 2.1.0)
  245 + cliver (~> 0.2.1)
  246 + multi_json (~> 1.0)
  247 + websocket-driver (>= 0.2.0)
242 polyglot (0.3.3) 248 polyglot (0.3.3)
243 premailer (1.7.3) 249 premailer (1.7.3)
244 css_parser (>= 1.1.9) 250 css_parser (>= 1.1.9)
@@ -363,6 +369,7 @@ GEM @@ -363,6 +369,7 @@ GEM
363 webmock (1.15.0) 369 webmock (1.15.0)
364 addressable (>= 2.2.7) 370 addressable (>= 2.2.7)
365 crack (>= 0.3.2) 371 crack (>= 0.3.2)
  372 + websocket-driver (0.3.0)
366 xmpp4r (0.5.5) 373 xmpp4r (0.5.5)
367 xpath (2.0.0) 374 xpath (2.0.0)
368 nokogiri (~> 1.3) 375 nokogiri (~> 1.3)
@@ -414,6 +421,7 @@ DEPENDENCIES @@ -414,6 +421,7 @@ DEPENDENCIES
414 oruen_redmine_client 421 oruen_redmine_client
415 pivotal-tracker 422 pivotal-tracker
416 pjax_rails 423 pjax_rails
  424 + poltergeist
417 pry-rails 425 pry-rails
418 puma 426 puma
419 quiet_assets 427 quiet_assets
app/controllers/apps_controller.rb
  Diff comments   View file @a3dbf20
@@ -49,7 +49,6 @@ class AppsController < ApplicationController @@ -49,7 +49,6 @@ class AppsController < ApplicationController
49 end 49 end
50 50
51 def create 51 def create
52 - initialize_subclassed_issue_tracker  
53 initialize_subclassed_notification_service 52 initialize_subclassed_notification_service
54 if app.save 53 if app.save
55 redirect_to app_url(app), :flash => { :success => I18n.t('controllers.apps.flash.create.success') } 54 redirect_to app_url(app), :flash => { :success => I18n.t('controllers.apps.flash.create.success') }
@@ -60,7 +59,6 @@ class AppsController < ApplicationController @@ -60,7 +59,6 @@ class AppsController < ApplicationController
60 end 59 end
61 60
62 def update 61 def update
63 - initialize_subclassed_issue_tracker  
64 initialize_subclassed_notification_service 62 initialize_subclassed_notification_service
65 if app.save 63 if app.save
66 redirect_to app_url(app), :flash => { :success => I18n.t('controllers.apps.flash.update.success') } 64 redirect_to app_url(app), :flash => { :success => I18n.t('controllers.apps.flash.update.success') }
@@ -90,15 +88,6 @@ class AppsController < ApplicationController @@ -90,15 +88,6 @@ class AppsController < ApplicationController
90 88
91 protected 89 protected
92 90
93 - def initialize_subclassed_issue_tracker  
94 - # set the app's issue tracker  
95 - if params[:app][:issue_tracker_attributes] && tracker_type = params[:app][:issue_tracker_attributes][:type]  
96 - if IssueTracker.subclasses.map(&:name).concat(["IssueTracker"]).include?(tracker_type)  
97 - app.issue_tracker = tracker_type.constantize.new(params[:app][:issue_tracker_attributes])  
98 - end  
99 - end  
100 - end  
101 -  
102 def initialize_subclassed_notification_service 91 def initialize_subclassed_notification_service
103 # set the app's notification service 92 # set the app's notification service
104 if params[:app][:notification_service_attributes] && notification_type = params[:app][:notification_service_attributes][:type] 93 if params[:app][:notification_service_attributes] && notification_type = params[:app][:notification_service_attributes][:type]
app/decorators/issue_tracker_decorator.rb
  Diff comments   View file @a3dbf20
@@ -4,8 +4,8 @@ class IssueTrackerDecorator < Draper::Decorator @@ -4,8 +4,8 @@ class IssueTrackerDecorator < Draper::Decorator
4 4
5 def issue_trackers 5 def issue_trackers
6 @issue_trackers ||= [ 6 @issue_trackers ||= [
7 - IssueTracker,  
8 - IssueTracker.subclasses 7 + IssueTracker::None,
  8 + IssueTracker.subclasses.select{|klass| klass != IssueTracker::None }
9 ].flatten 9 ].flatten
10 @issue_trackers.each do |it| 10 @issue_trackers.each do |it|
11 yield IssueTrackerDecorator.new(it) 11 yield IssueTrackerDecorator.new(it)
app/models/issue_tracker.rb
  Diff comments   View file @a3dbf20
@@ -4,9 +4,7 @@ class IssueTracker @@ -4,9 +4,7 @@ class IssueTracker
4 include HashHelper 4 include HashHelper
5 include Rails.application.routes.url_helpers 5 include Rails.application.routes.url_helpers
6 6
7 - Fields = []  
8 - Note = 'When no issue tracker has been configured, you will be able to leave comments on errors.'  
9 - Label = 'none' 7 + Note = ''
10 8
11 default_url_options[:host] = ActionMailer::Base.default_url_options[:host] 9 default_url_options[:host] = ActionMailer::Base.default_url_options[:host]
12 10
app/models/issue_trackers/none.rb 0 → 100644
  Diff comments   View file @a3dbf20
@@ -0,0 +1,5 @@ @@ -0,0 +1,5 @@
  1 +class IssueTrackers::None < IssueTracker
  2 + Fields = []
  3 + Note = 'When no issue tracker has been configured, you will be able to leave comments on errors.'
  4 + Label = 'none'
  5 +end
app/views/apps/edit.html.haml
  Diff comments   View file @a3dbf20
1 -- content_for :title, 'Edit App' 1 +- content_for :title, t('.title')
2 - content_for :action_bar do 2 - content_for :action_bar do
3 = link_to_copy_attributes_from_other_app 3 = link_to_copy_attributes_from_other_app
4 - = link_to 'delete application', app_path(app), :method => :delete, 4 + = link_to t('.destroy'), app_path(app), :method => :delete,
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(t('.cancel'), app_path(app), :class => 'button')
7 7
8 = form_for app_decorate do |f| 8 = form_for app_decorate do |f|
9 9
10 = render 'fields', :f => f 10 = render 'fields', :f => f
11 11
12 - %div.buttons= f.submit 'Update App' 12 + %div.buttons= f.submit t('.update')
13 13
app/views/apps/new.html.haml
  Diff comments   View file @a3dbf20
1 -- content_for :title, 'Add App' 1 +- content_for :title, t('.title')
2 - content_for :action_bar do 2 - content_for :action_bar do
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(t('.cancel'), apps_path, :class => 'button')
5 5
6 = form_for app_decorate do |f| 6 = form_for app_decorate do |f|
7 7
8 = render 'fields', :f => f 8 = render 'fields', :f => f
9 9
10 - %div.buttons= f.submit 'Add App' 10 + %div.buttons= f.submit t('.add_app')
11 11
config/locales/en.yml
  Diff comments   View file @a3dbf20
@@ -127,4 +127,14 @@ en: @@ -127,4 +127,14 @@ en:
127 who: Who 127 who: Who
128 issue_tracker_fields: 128 issue_tracker_fields:
129 legend: Issue Tracker 129 legend: Issue Tracker
  130 + new:
  131 + add_app: "Add App"
  132 + cancel: "cancel"
  133 + title: "Add App"
  134 + edit:
  135 + title: 'Edit App'
  136 + destroy: 'destroy application'
  137 + cancel: 'cancel'
  138 + seriously: 'Seriously?'
  139 + update: 'Update App'
130 140
spec/acceptance/acceptance_helper.rb
  Diff comments   View file @a3dbf20
1 require 'spec_helper' 1 require 'spec_helper'
2 require 'capybara/rspec' 2 require 'capybara/rspec'
  3 +require 'capybara/poltergeist'
  4 +
  5 +Capybara.javascript_driver = :poltergeist
3 6
4 OmniAuth.config.test_mode = true 7 OmniAuth.config.test_mode = true
5 8
spec/acceptance/app_regenerate_api_key_spec.rb
  Diff comments   View file @a3dbf20
1 require 'acceptance/acceptance_helper' 1 require 'acceptance/acceptance_helper'
2 2
3 feature "Regeneration api_Key" do 3 feature "Regeneration api_Key" do
4 - let!(:app) { Fabricate(:app) }  
5 - let!(:admin) { Fabricate(:admin) } 4 + let(:app) { Fabricate(:app) }
  5 + let(:admin) { Fabricate(:admin) }
6 let(:user) { 6 let(:user) {
7 Fabricate(:user_watcher, :app => app).user 7 Fabricate(:user_watcher, :app => app).user
8 } 8 }
9 9
  10 + before do
  11 + app && admin
  12 + end
  13 +
10 scenario "an admin change api_key" do 14 scenario "an admin change api_key" do
11 visit '/' 15 visit '/'
12 log_in admin 16 log_in admin
@@ -29,3 +33,64 @@ feature &quot;Regeneration api_Key&quot; do @@ -29,3 +33,64 @@ feature &quot;Regeneration api_Key&quot; do
29 expect(page).to_not have_button I18n.t('apps.show.edit') 33 expect(page).to_not have_button I18n.t('apps.show.edit')
30 end 34 end
31 end 35 end
  36 +
  37 +feature "Create an application" do
  38 +
  39 + let(:admin) { Fabricate(:admin) }
  40 + let(:user) {
  41 + Fabricate(:user_watcher, :app => app).user
  42 + }
  43 +
  44 + before do
  45 + admin
  46 + end
  47 +
  48 + scenario "create an apps without issue tracker and edit it" do
  49 + visit '/'
  50 + log_in admin
  51 + click_on I18n.t('apps.index.new_app')
  52 + fill_in 'app_name', :with => 'My new app'
  53 + click_on I18n.t('apps.new.add_app')
  54 + page.has_content?(I18n.t('controllers.apps.flash.create.success'))
  55 + expect(App.where(:name => 'My new app').count).to eql 1
  56 + expect(App.where(:name => 'My new app 2').count).to eql 0
  57 +
  58 +
  59 + click_on I18n.t('shared.navigation.apps')
  60 + click_on 'My new app'
  61 + click_link I18n.t('apps.show.edit')
  62 + fill_in 'app_name', :with => 'My new app 2'
  63 + click_on I18n.t('apps.edit.update')
  64 + page.has_content?(I18n.t('controllers.apps.flash.update.success'))
  65 + expect(App.where(:name => 'My new app').count).to eql 0
  66 + expect(App.where(:name => 'My new app 2').count).to eql 1
  67 +
  68 + end
  69 +
  70 + scenario "create an apps with issue tracker and edit it", :js => true do
  71 + visit '/'
  72 + log_in admin
  73 + click_on I18n.t('apps.index.new_app')
  74 + fill_in 'app_name', :with => 'My new app'
  75 + find('.label_radio.bitbucket').click
  76 + within ".bitbucket.tracker_params" do
  77 + fill_in 'app_issue_tracker_attributes_api_token', :with => 'token'
  78 + fill_in 'app_issue_tracker_attributes_project_id', :with => 'pass'
  79 + end
  80 + click_on I18n.t('apps.new.add_app')
  81 + expect(page.has_content?(I18n.t('controllers.apps.flash.create.success'))).to eql true
  82 + app = App.where(:name => 'My new app').first
  83 + expect(app.issue_tracker).to be_a IssueTracker::BitbucketIssuesTracker
  84 + expect(app.issue_tracker.api_token).to eql 'token'
  85 + expect(app.issue_tracker.project_id).to eql 'pass'
  86 +
  87 + click_on I18n.t('shared.navigation.apps')
  88 + click_on 'My new app'
  89 + click_link I18n.t('apps.show.edit')
  90 + find('.issue_tracker .label_radio.none').click
  91 + click_on I18n.t('apps.edit.update')
  92 + expect(page.has_content?(I18n.t('controllers.apps.flash.update.success'))).to eql true
  93 + app = App.where(:name => 'My new app').first
  94 + expect(app.issue_tracker).to be_a IssueTracker::None
  95 + end
  96 +end
spec/controllers/apps_controller_spec.rb
  Diff comments   View file @a3dbf20
@@ -323,8 +323,8 @@ describe AppsController do @@ -323,8 +323,8 @@ describe AppsController do
323 323
324 it "should show validation notice when sufficient params are not present" do 324 it "should show validation notice when sufficient params are not present" do
325 # Leave out one required param 325 # Leave out one required param
326 - params = tracker_klass::Fields[1..-1].inject({}){|hash,f| hash[f[0]] = "test_value"; hash }  
327 - params[:type] = tracker_klass.to_s 326 + # TODO. previous test was not relevant because one params can be enough. So put noone
  327 + params = {:type => tracker_klass.to_s }
328 put :update, :id => @app.id, :app => {:issue_tracker_attributes => params} 328 put :update, :id => @app.id, :app => {:issue_tracker_attributes => params}
329 329
330 @app.reload 330 @app.reload
spec/spec_helper.rb
  Diff comments   View file @a3dbf20
@@ -55,7 +55,7 @@ RSpec.configure do |config| @@ -55,7 +55,7 @@ RSpec.configure do |config|
55 end 55 end
56 56
57 config.after(:all) do 57 config.after(:all) do
58 - WebMock.disable_net_connect! :allow => /coveralls\.io/ 58 + WebMock.disable_net_connect! :allow => /coveralls\.io|127\.0\.0\.1/
59 end 59 end
60 end 60 end
61 61