Download as
Browse Code »

Commit 44e8ad2324d14b41f19d5b03ad4d2f6c32887a97

Authored by Cyril Mougel
2 parents ea727cfc f2409444
Exists in master and in 1 other branch production

Merge pull request #497 from shingara/features/#456 

Improve user management
Showing 21 changed files with 317 additions and 181 deletions   Show diff stats
@@ -5,6 +5,8 @@ @@ -5,6 +5,8 @@
5 - Update some gems ([@shingara][]) 5 - Update some gems ([@shingara][])
6 - [#492][] Improve some Pjax call ([@nfedyashev][]) 6 - [#492][] Improve some Pjax call ([@nfedyashev][])
7 - [#428][] Add the support of Unfuddle Tracker ([@parallel588][]) 7 - [#428][] Add the support of Unfuddle Tracker ([@parallel588][])
  8 +- Avoid to delete his own user ([@shingara][])
  9 +- [#456] Avoid to delete admin access of current user logged ([@shingara][])
8 10
9 ### Bug Fixes 11 ### Bug Fixes
10 12
@@ -48,6 +50,7 @@ @@ -48,6 +50,7 @@
48 [#428]: https://github.com/errbit/errbit/issues/428 50 [#428]: https://github.com/errbit/errbit/issues/428
49 [#453]: https://github.com/errbit/errbit/issues/453 51 [#453]: https://github.com/errbit/errbit/issues/453
50 [#455]: https://github.com/errbit/errbit/issues/455 52 [#455]: https://github.com/errbit/errbit/issues/455
  53 +[#456]: https://github.com/errbit/errbit/issues/456
51 [#457]: https://github.com/errbit/errbit/issues/457 54 [#457]: https://github.com/errbit/errbit/issues/457
52 [#460]: https://github.com/errbit/errbit/issues/460 55 [#460]: https://github.com/errbit/errbit/issues/460
53 [#466]: https://github.com/errbit/errbit/issues/466 56 [#466]: https://github.com/errbit/errbit/issues/466
@@ -2,7 +2,9 @@ source 'http://rubygems.org' @@ -2,7 +2,9 @@ source 'http://rubygems.org'
2 2
3 gem 'rails', '3.2.13' 3 gem 'rails', '3.2.13'
4 gem 'mongoid', '~> 2.7.1' 4 gem 'mongoid', '~> 2.7.1'
5 -gem 'mongoid_rails_migrations' 5 +
  6 +# Mongoid rails migration > 0.0.14 is not compatible to Mongoid 2.x
  7 +gem 'mongoid_rails_migrations', '~> 0.0.14'
6 gem 'devise', '~> 1.5.4' 8 gem 'devise', '~> 1.5.4'
7 gem 'haml' 9 gem 'haml'
8 gem 'htmlentities' 10 gem 'htmlentities'
@@ -10,6 +12,8 @@ gem 'rack-ssl', :require => 'rack/ssl' # force SSL @@ -10,6 +12,8 @@ gem 'rack-ssl', :require => 'rack/ssl' # force SSL
10 12
11 gem 'useragent' 13 gem 'useragent'
12 gem 'inherited_resources' 14 gem 'inherited_resources'
  15 +gem 'decent_exposure'
  16 +gem 'strong_parameters'
13 gem 'SystemTimer', :platform => :ruby_18 17 gem 'SystemTimer', :platform => :ruby_18
14 gem 'actionmailer_inline_css', "~> 1.3.0" 18 gem 'actionmailer_inline_css', "~> 1.3.0"
15 gem 'kaminari', '>= 0.14.1' 19 gem 'kaminari', '>= 0.14.1'
1 GIT 1 GIT
2 remote: https://github.com/NARKOZ/gitlab.git 2 remote: https://github.com/NARKOZ/gitlab.git
3 - revision: f2ba111dba70eca5346a880c541dafaf35d3332a 3 + revision: 53d7a8a86dfed63e56eeb16ea496bb7a82de337e
4 specs: 4 specs:
5 gitlab (2.2.0) 5 gitlab (2.2.0)
6 httparty 6 httparty
@@ -91,9 +91,8 @@ GEM @@ -91,9 +91,8 @@ GEM
91 simplecov (>= 0.7) 91 simplecov (>= 0.7)
92 thor 92 thor
93 crack (0.3.2) 93 crack (0.3.2)
94 - css_parser (1.2.6) 94 + css_parser (1.3.4)
95 addressable 95 addressable
96 - rdoc  
97 daemons (1.1.9) 96 daemons (1.1.9)
98 database_cleaner (0.9.1) 97 database_cleaner (0.9.1)
99 debug_inspector (0.0.2) 98 debug_inspector (0.0.2)
@@ -103,6 +102,7 @@ GEM @@ -103,6 +102,7 @@ GEM
103 debugger-ruby_core_source (~> 1.2.1) 102 debugger-ruby_core_source (~> 1.2.1)
104 debugger-linecache (1.2.0) 103 debugger-linecache (1.2.0)
105 debugger-ruby_core_source (1.2.2) 104 debugger-ruby_core_source (1.2.2)
  105 + decent_exposure (2.2.0)
106 devise (1.5.4) 106 devise (1.5.4)
107 bcrypt-ruby (~> 3.0) 107 bcrypt-ruby (~> 3.0)
108 orm_adapter (~> 0.0.3) 108 orm_adapter (~> 0.0.3)
@@ -135,6 +135,7 @@ GEM @@ -135,6 +135,7 @@ GEM
135 hike (1.2.2) 135 hike (1.2.2)
136 hipchat (0.9.0) 136 hipchat (0.9.0)
137 httparty 137 httparty
  138 + httparty
138 hoi (0.0.6) 139 hoi (0.0.6)
139 httparty (> 0.6.0) 140 httparty (> 0.6.0)
140 json (> 1.4.0) 141 json (> 1.4.0)
@@ -151,9 +152,9 @@ GEM @@ -151,9 +152,9 @@ GEM
151 has_scope (~> 0.5.0) 152 has_scope (~> 0.5.0)
152 responders (~> 0.9) 153 responders (~> 0.9)
153 journey (1.0.4) 154 journey (1.0.4)
154 - jquery-rails (2.1.3)  
155 - railties (>= 3.1.0, < 5.0)  
156 - thor (~> 0.14) 155 + jquery-rails (3.0.0)
  156 + railties (>= 3.0, < 5.0)
  157 + thor (>= 0.14, < 2.0)
157 json (1.8.0) 158 json (1.8.0)
158 jwt (0.1.8) 159 jwt (0.1.8)
159 multi_json (>= 1.5) 160 multi_json (>= 1.5)
@@ -229,12 +230,14 @@ GEM @@ -229,12 +230,14 @@ GEM
229 activeresource (>= 2.3.0) 230 activeresource (>= 2.3.0)
230 pivotal-tracker (0.5.10) 231 pivotal-tracker (0.5.10)
231 builder 232 builder
  233 + builder
232 crack 234 crack
233 happymapper (>= 0.3.2) 235 happymapper (>= 0.3.2)
234 nokogiri (>= 1.4.3) 236 nokogiri (>= 1.4.3)
235 nokogiri (>= 1.5.5) 237 nokogiri (>= 1.5.5)
236 nokogiri-happymapper (>= 0.5.4) 238 nokogiri-happymapper (>= 0.5.4)
237 rest-client (~> 1.6.0) 239 rest-client (~> 1.6.0)
  240 + rest-client (~> 1.6.0)
238 pjax_rails (0.3.4) 241 pjax_rails (0.3.4)
239 jquery-rails 242 jquery-rails
240 polyglot (0.3.3) 243 polyglot (0.3.3)
@@ -279,7 +282,7 @@ GEM @@ -279,7 +282,7 @@ GEM
279 rbx-require-relative (0.0.9) 282 rbx-require-relative (0.0.9)
280 rdoc (3.12.2) 283 rdoc (3.12.2)
281 json (~> 1.4) 284 json (~> 1.4)
282 - ref (1.0.4) 285 + ref (1.0.5)
283 responders (0.9.3) 286 responders (0.9.3)
284 railties (~> 3.1) 287 railties (~> 3.1)
285 rest-client (1.6.7) 288 rest-client (1.6.7)
@@ -323,6 +326,10 @@ GEM @@ -323,6 +326,10 @@ GEM
323 multi_json (~> 1.0) 326 multi_json (~> 1.0)
324 rack (~> 1.0) 327 rack (~> 1.0)
325 tilt (~> 1.1, != 1.3.0) 328 tilt (~> 1.1, != 1.3.0)
  329 + strong_parameters (0.2.1)
  330 + actionpack (~> 3.0)
  331 + activemodel (~> 3.0)
  332 + railties (~> 3.0)
326 taskmapper (0.8.0) 333 taskmapper (0.8.0)
327 activeresource (~> 3.0) 334 activeresource (~> 3.0)
328 activesupport (~> 3.0) 335 activesupport (~> 3.0)
@@ -385,6 +392,7 @@ DEPENDENCIES @@ -385,6 +392,7 @@ DEPENDENCIES
385 coveralls 392 coveralls
386 database_cleaner (~> 0.9.0) 393 database_cleaner (~> 0.9.0)
387 debugger 394 debugger
  395 + decent_exposure
388 devise (~> 1.5.4) 396 devise (~> 1.5.4)
389 email_spec 397 email_spec
390 execjs 398 execjs
@@ -405,7 +413,7 @@ DEPENDENCIES @@ -405,7 +413,7 @@ DEPENDENCIES
405 meta_request 413 meta_request
406 mongo 414 mongo
407 mongoid (~> 2.7.1) 415 mongoid (~> 2.7.1)
408 - mongoid_rails_migrations 416 + mongoid_rails_migrations (~> 0.0.14)
409 octokit 417 octokit
410 omniauth-github 418 omniauth-github
411 oruen_redmine_client 419 oruen_redmine_client
@@ -421,6 +429,7 @@ DEPENDENCIES @@ -421,6 +429,7 @@ DEPENDENCIES
421 ruby-debug 429 ruby-debug
422 ruby-fogbugz 430 ruby-fogbugz
423 rushover 431 rushover
  432 + strong_parameters
424 taskmapper (~> 0.8.0) 433 taskmapper (~> 0.8.0)
425 taskmapper-unfuddle (~> 0.7.0) 434 taskmapper-unfuddle (~> 0.7.0)
426 therubyracer 435 therubyracer
app/controllers/application_controller.rb
  Diff comments   View file @44e8ad2
@@ -17,6 +17,9 @@ class ApplicationController &lt; ActionController::Base @@ -17,6 +17,9 @@ class ApplicationController &lt; ActionController::Base
17 protected 17 protected
18 18
19 19
  20 + ##
  21 + # Check if the current_user is admin or not and redirect to root url if not
  22 + #
20 def require_admin! 23 def require_admin!
21 unless user_signed_in? && current_user.admin? 24 unless user_signed_in? && current_user.admin?
22 flash[:error] = "Sorry, you don't have permission to do that" 25 flash[:error] = "Sorry, you don't have permission to do that"
app/controllers/users_controller.rb
  Diff comments   View file @44e8ad2
@@ -2,71 +2,78 @@ class UsersController &lt; ApplicationController @@ -2,71 +2,78 @@ class UsersController &lt; ApplicationController
2 respond_to :html 2 respond_to :html
3 3
4 before_filter :require_admin!, :except => [:edit, :update] 4 before_filter :require_admin!, :except => [:edit, :update]
5 - before_filter :find_user, :only => [:show, :edit, :update, :destroy, :unlink_github]  
6 before_filter :require_user_edit_priviledges, :only => [:edit, :update] 5 before_filter :require_user_edit_priviledges, :only => [:edit, :update]
7 6
8 - def index  
9 - @users = User.all.page(params[:page]).per(current_user.per_page)  
10 - end 7 + expose(:user) {
  8 + params[:id] ? User.find(params[:id]) : User.new(user_params)
  9 + }
  10 + expose(:users) {
  11 + User.all.page(params[:page]).per(current_user.per_page)
  12 + }
11 13
12 - def new  
13 - @user = User.new  
14 - end 14 + def index; end
  15 + def new; end
  16 + def show; end
15 17
16 def create 18 def create
17 - @user = User.new(params[:user])  
18 -  
19 - # Set protected attributes  
20 - @user.admin = params[:user].try(:[], :admin) if current_user.admin?  
21 -  
22 - if @user.save  
23 - flash[:success] = "#{@user.name} is now part of the team. Be sure to add them as a project watcher."  
24 - redirect_to user_path(@user) 19 + if user.save
  20 + flash[:success] = "#{user.name} is now part of the team. Be sure to add them as a project watcher."
  21 + redirect_to user_path(user)
25 else 22 else
26 render :new 23 render :new
27 end 24 end
28 end 25 end
29 26
30 def update 27 def update
31 - # Devise Hack  
32 - if params[:user][:password].blank? && params[:user][:password_confirmation].blank?  
33 - params[:user].delete(:password)  
34 - params[:user].delete(:password_confirmation)  
35 - end  
36 -  
37 - # Set protected attributes  
38 - @user.admin = params[:user][:admin] if current_user.admin?  
39 -  
40 - if @user.update_attributes(params[:user])  
41 - flash[:success] = "#{@user.name}'s information was successfully updated"  
42 - redirect_to user_path(@user) 28 + if user.update_attributes(user_params)
  29 + flash[:success] = I18n.t('controllers.users.flash.update.success', :name => user.name)
  30 + redirect_to user_path(user)
43 else 31 else
44 render :edit 32 render :edit
45 end 33 end
46 end 34 end
47 35
  36 + ##
  37 + # Destroy the user pass in args
  38 + #
  39 + # @param [ String ] id the id of user we want delete
  40 + #
48 def destroy 41 def destroy
49 - @user.destroy  
50 -  
51 - flash[:success] = "That's sad. #{@user.name} is no longer part of your team." 42 + if user == current_user
  43 + flash[:error] = I18n.t('controllers.users.flash.destroy.error')
  44 + else
  45 + UserDestroy.new(user).destroy
  46 + flash[:success] = I18n.t('controllers.users.flash.destroy.success', :name => user.name)
  47 + end
52 redirect_to users_path 48 redirect_to users_path
53 end 49 end
54 50
55 def unlink_github 51 def unlink_github
56 - @user.update_attributes :github_login => nil, :github_oauth_token => nil  
57 - redirect_to user_path(@user) 52 + user.update_attributes :github_login => nil, :github_oauth_token => nil
  53 + redirect_to user_path(user)
58 end 54 end
59 55
60 protected 56 protected
61 57
62 - def find_user  
63 - @user = User.find(params[:id])  
64 - end  
65 -  
66 def require_user_edit_priviledges 58 def require_user_edit_priviledges
67 - can_edit = current_user == @user || current_user.admin? 59 + can_edit = current_user == user || current_user.admin?
68 redirect_to(root_path) and return(false) unless can_edit 60 redirect_to(root_path) and return(false) unless can_edit
69 end 61 end
70 62
  63 + def user_params
  64 + @user_params ||= params[:user] ? params.require(:user).permit(*user_permit_params) : {}
  65 + end
  66 +
  67 + def user_permit_params
  68 + @user_permit_params ||= [:name,:username, :email, :github_login, :per_page, :time_zone]
  69 + @user_permit_params << :admin if current_user.admin? && current_user.id != params[:id]
  70 + @user_permit_params |= [:password, :password_confirmation] if user_password_params.values.all?{|pa| !pa.blank? }
  71 + @user_permit_params
  72 + end
  73 +
  74 + def user_password_params
  75 + @user_password_params ||= params[:user] ? params.require(:user).permit(:password, :password_confirmation) : {}
  76 + end
  77 +
71 end 78 end
72 79
app/interactors/user_destroy.rb 0 → 100644
  Diff comments   View file @44e8ad2
@@ -0,0 +1,11 @@ @@ -0,0 +1,11 @@
  1 +class UserDestroy
  2 + def initialize(user)
  3 + @user = user
  4 + end
  5 +
  6 + def destroy
  7 + @user.destroy
  8 + @user.watchers.each(&:destroy)
  9 + end
  10 +
  11 +end
app/models/user.rb
  Diff comments   View file @44e8ad2
@@ -13,14 +13,11 @@ class User @@ -13,14 +13,11 @@ class User
13 field :per_page, :type => Fixnum, :default => PER_PAGE 13 field :per_page, :type => Fixnum, :default => PER_PAGE
14 field :time_zone, :default => "UTC" 14 field :time_zone, :default => "UTC"
15 15
16 - after_destroy :destroy_watchers  
17 before_save :ensure_authentication_token 16 before_save :ensure_authentication_token
18 17
19 validates_presence_of :name 18 validates_presence_of :name
20 validates_uniqueness_of :github_login, :allow_nil => true 19 validates_uniqueness_of :github_login, :allow_nil => true
21 20
22 - attr_protected :admin  
23 -  
24 has_many :apps, :foreign_key => 'watchers.user_id' 21 has_many :apps, :foreign_key => 'watchers.user_id'
25 22
26 if Errbit::Config.user_has_username 23 if Errbit::Config.user_has_username
@@ -59,10 +56,5 @@ class User @@ -59,10 +56,5 @@ class User
59 self[:github_login] = login 56 self[:github_login] = login
60 end 57 end
61 58
62 - protected  
63 -  
64 - def destroy_watchers  
65 - watchers.each(&:destroy)  
66 - end  
67 end 59 end
68 60
app/views/users/_fields.html.haml
  Diff comments   View file @44e8ad2
1 -= errors_for @user 1 += errors_for user
2 2
3 .required 3 .required
4 = f.label :name 4 = f.label :name
app/views/users/edit.html.haml
  Diff comments   View file @44e8ad2
1 -- content_for :title, "Edit #{@user.name}" 1 +- content_for :title, "Edit #{user.name}"
2 - content_for :action_bar do 2 - content_for :action_bar do
3 - = render 'shared/link_github_account', :user => @user  
4 - = link_to('cancel', user_path(@user), :class => 'button') 3 + = render 'shared/link_github_account', :user => user
  4 + = link_to('cancel', user_path(user), :class => 'button')
5 5
6 -= form_for @user, :html => {:autocomplete => "off"} do |f|  
7 - = @user.errors.full_messages.to_sentence 6 += form_for user, :html => {:autocomplete => "off"} do |f|
  7 + = user.errors.full_messages.to_sentence
8 = render 'fields', :f => f 8 = render 'fields', :f => f
9 9
10 %div.buttons= f.submit 'Update User' 10 %div.buttons= f.submit 'Update User'
app/views/users/index.html.haml
  Diff comments   View file @44e8ad2
@@ -13,8 +13,8 @@ @@ -13,8 +13,8 @@
13 %th.main Email 13 %th.main Email
14 %th Admin? 14 %th Admin?
15 %tbody 15 %tbody
16 - - @users.each do |user|  
17 - %tr 16 + - users.each do |user|
  17 + %tr.user_list
18 - if Errbit::Config.use_gravatar 18 - if Errbit::Config.use_gravatar
19 %td= gravatar_tag user.email, :s => 24 19 %td= gravatar_tag user.email, :s => 24
20 %td.nowrap= link_to user.name, user_path(user) 20 %td.nowrap= link_to user.name, user_path(user)
@@ -22,5 +22,5 @@ @@ -22,5 +22,5 @@
22 %td= user.username 22 %td= user.username
23 %td= user.email 23 %td= user.email
24 %td= user.admin? ? 'Y' : 'N' 24 %td= user.admin? ? 'Y' : 'N'
25 -= paginate @users 25 += paginate users
26 26
app/views/users/new.html.haml
  Diff comments   View file @44e8ad2
1 - content_for :title, 'New User' 1 - content_for :title, 'New User'
2 - content_for :action_bar, link_to('cancel', users_path, :class => 'button') 2 - content_for :action_bar, link_to('cancel', users_path, :class => 'button')
3 3
4 -= form_for @user do |f|  
5 - 4 += form_for user do |f|
  5 +
6 = render 'fields', :f => f 6 = render 'fields', :f => f
7 -  
8 - %div.buttons= f.submit 'Add User'  
9 \ No newline at end of file 7 \ No newline at end of file
  8 +
  9 + %div.buttons= f.submit 'Add User'
app/views/users/show.html.haml
  Diff comments   View file @44e8ad2
1 -- content_for :title, @user.name  
2 -- if Errbit::Config.use_gravatar && gravatar = gravatar_url(@user.email, :s => 86) 1 +- content_for :title, user.name
  2 +
  3 +- if Errbit::Config.use_gravatar && gravatar = gravatar_url(user.email, :s => 86)
3 - content_for :title_style do 4 - content_for :title_style do
4 background: url('#{gravatar}') no-repeat; 5 background: url('#{gravatar}') no-repeat;
5 padding-left: 106px; 6 padding-left: 106px;
6 7
7 - content_for :action_bar do 8 - content_for :action_bar do
8 - = render 'shared/link_github_account', :user => @user 9 + = render 'shared/link_github_account'
9 %span= link_to('Add a New User', new_user_path, :class => 'add') 10 %span= link_to('Add a New User', new_user_path, :class => 'add')
10 - = link_to 'edit', edit_user_path(@user), :class => 'button'  
11 - = link_to 'destroy', user_path(@user), :method => :delete, :data => { :confirm => 'Seriously?' }, :class => 'button' 11 + = link_to 'edit', edit_user_path(user), :class => 'button'
  12 + = link_to 'destroy', user_path(user), :method => :delete, :data => { :confirm => 'Seriously?' }, :class => 'button'
12 13
13 %table.single_user 14 %table.single_user
14 %tr 15 %tr
15 %th Email 16 %th Email
16 - %td.main= @user.email 17 + %td.main= user.email
17 - if Errbit::Config.user_has_username 18 - if Errbit::Config.user_has_username
18 %tr 19 %tr
19 %th Username 20 %th Username
20 - %td.main= @user.username  
21 - - if Errbit::Config.github_authentication && @user.github_login.present? 21 + %td.main= user.username
  22 + - if Errbit::Config.github_authentication && user.github_login.present?
22 %tr 23 %tr
23 %th GitHub Login 24 %th GitHub Login
24 - %td.main= link_to @user.github_login, "https://github.com/#{@user.github_login}" 25 + %td.main= link_to user.github_login, "https://github.com/#{user.github_login}"
25 %tr 26 %tr
26 %th Admin? 27 %th Admin?
27 - %td= @user.admin? ? 'Y' : 'N' 28 + %td= user.admin? ? 'Y' : 'N'
28 %tr 29 %tr
29 %th Created 30 %th Created
30 - %td= @user.created_at.to_s(:micro) 31 + %td= user.created_at.to_s(:micro)
31 32
config/locales/en.yml
  Diff comments   View file @44e8ad2
@@ -14,3 +14,13 @@ en: @@ -14,3 +14,13 @@ en:
14 n_errs_have: 14 n_errs_have:
15 one: "%{count} err has" 15 one: "%{count} err has"
16 other: "%{count} errs have" 16 other: "%{count} errs have"
  17 +
  18 +
  19 + controllers:
  20 + users:
  21 + flash:
  22 + destroy:
  23 + success: "That's sad. %{name} is no longer part of your team."
  24 + error: "You can't delete yourself"
  25 + update:
  26 + success: "%{name}'s information was successfully updated."
spec/controllers/users_controller_spec.rb
  Diff comments   View file @44e8ad2
1 require 'spec_helper' 1 require 'spec_helper'
2 2
3 describe UsersController do 3 describe UsersController do
4 - render_views  
5 4
6 it_requires_authentication 5 it_requires_authentication
7 it_requires_admin_privileges :for => { 6 it_requires_admin_privileges :for => {
@@ -12,42 +11,39 @@ describe UsersController do @@ -12,42 +11,39 @@ describe UsersController do
12 :destroy => :delete 11 :destroy => :delete
13 } 12 }
14 13
  14 + let(:admin) { Fabricate(:admin) }
  15 + let(:user) { Fabricate(:user) }
  16 + let(:other_user) { Fabricate(:user) }
  17 +
15 context 'Signed in as a regular user' do 18 context 'Signed in as a regular user' do
  19 +
16 before do 20 before do
17 - sign_in @user = Fabricate(:user) 21 + sign_in user
18 end 22 end
19 23
20 it "should set a time zone" do 24 it "should set a time zone" do
21 - Time.zone.should.to_s == @user.time_zone 25 + Time.zone.should.to_s == user.time_zone
22 end 26 end
23 27
24 context "GET /users/:other_id/edit" do 28 context "GET /users/:other_id/edit" do
25 it "redirects to the home page" do 29 it "redirects to the home page" do
26 - get :edit, :id => Fabricate(:user).id 30 + get :edit, :id => other_user.id
27 response.should redirect_to(root_path) 31 response.should redirect_to(root_path)
28 end 32 end
29 end 33 end
30 34
31 context "GET /users/:my_id/edit" do 35 context "GET /users/:my_id/edit" do
32 it 'finds the user' do 36 it 'finds the user' do
33 - get :edit, :id => @user.id  
34 - assigns(:user).should == @user  
35 - end  
36 -  
37 - it "should have per_page option" do  
38 - get :edit, :id => @user.id  
39 - response.body.should match(/id="user_per_page"/) 37 + get :edit, :id => user.id
  38 + controller.user.should == user
  39 + expect(response).to render_template 'edit'
40 end 40 end
41 41
42 - it "should have time_zone option" do  
43 - get :edit, :id => @user.id  
44 - response.body.should match(/id="user_time_zone"/)  
45 - end  
46 end 42 end
47 43
48 context "PUT /users/:other_id" do 44 context "PUT /users/:other_id" do
49 it "redirects to the home page" do 45 it "redirects to the home page" do
50 - put :update, :id => Fabricate(:user).id 46 + put :update, :id => other_user.id
51 response.should redirect_to(root_path) 47 response.should redirect_to(root_path)
52 end 48 end
53 end 49 end
@@ -55,44 +51,47 @@ describe UsersController do @@ -55,44 +51,47 @@ describe UsersController do
55 context "PUT /users/:my_id/id" do 51 context "PUT /users/:my_id/id" do
56 context "when the update is successful" do 52 context "when the update is successful" do
57 it "sets a message to display" do 53 it "sets a message to display" do
58 - put :update, :id => @user.to_param, :user => {:name => 'Kermit'} 54 + put :update, :id => user.to_param, :user => {:name => 'Kermit'}
59 request.flash[:success].should include('updated') 55 request.flash[:success].should include('updated')
60 end 56 end
61 57
62 it "redirects to the user's page" do 58 it "redirects to the user's page" do
63 - put :update, :id => @user.to_param, :user => {:name => 'Kermit'}  
64 - response.should redirect_to(user_path(@user)) 59 + put :update, :id => user.to_param, :user => {:name => 'Kermit'}
  60 + response.should redirect_to(user_path(user))
65 end 61 end
66 62
67 it "should not be able to become an admin" do 63 it "should not be able to become an admin" do
68 - put :update, :id => @user.to_param, :user => {:admin => true}  
69 - @user.reload.admin.should be_false 64 + expect {
  65 + put :update, :id => user.to_param, :user => {:admin => true}
  66 + }.to_not change {
  67 + user.reload.admin
  68 + }.from(false)
70 end 69 end
71 70
72 it "should be able to set per_page option" do 71 it "should be able to set per_page option" do
73 - put :update, :id => @user.to_param, :user => {:per_page => 555}  
74 - @user.reload.per_page.should == 555 72 + put :update, :id => user.to_param, :user => {:per_page => 555}
  73 + user.reload.per_page.should == 555
75 end 74 end
76 75
77 it "should be able to set time_zone option" do 76 it "should be able to set time_zone option" do
78 - put :update, :id => @user.to_param, :user => {:time_zone => "Warsaw"}  
79 - @user.reload.time_zone.should == "Warsaw" 77 + put :update, :id => user.to_param, :user => {:time_zone => "Warsaw"}
  78 + user.reload.time_zone.should == "Warsaw"
80 end 79 end
81 80
82 it "should be able to not set github_login option" do 81 it "should be able to not set github_login option" do
83 - put :update, :id => @user.to_param, :user => {:github_login => " "}  
84 - @user.reload.github_login.should == nil 82 + put :update, :id => user.to_param, :user => {:github_login => " "}
  83 + user.reload.github_login.should == nil
85 end 84 end
86 85
87 it "should be able to set github_login option" do 86 it "should be able to set github_login option" do
88 - put :update, :id => @user.to_param, :user => {:github_login => "awesome_name"}  
89 - @user.reload.github_login.should == "awesome_name" 87 + put :update, :id => user.to_param, :user => {:github_login => "awesome_name"}
  88 + user.reload.github_login.should == "awesome_name"
90 end 89 end
91 end 90 end
92 91
93 context "when the update is unsuccessful" do 92 context "when the update is unsuccessful" do
94 it "renders the edit page" do 93 it "renders the edit page" do
95 - put :update, :id => @user.to_param, :user => {:name => nil} 94 + put :update, :id => user.to_param, :user => {:name => nil}
96 response.should render_template(:edit) 95 response.should render_template(:edit)
97 end 96 end
98 end 97 end
@@ -101,81 +100,82 @@ describe UsersController do @@ -101,81 +100,82 @@ describe UsersController do
101 100
102 context 'Signed in as an admin' do 101 context 'Signed in as an admin' do
103 before do 102 before do
104 - @user = Fabricate(:admin)  
105 - sign_in @user 103 + sign_in admin
106 end 104 end
107 105
108 context "GET /users" do 106 context "GET /users" do
  107 +
109 it 'paginates all users' do 108 it 'paginates all users' do
110 - @user.update_attribute :per_page, 2  
111 - users = 3.times { Fabricate(:user) } 109 + admin.update_attribute :per_page, 2
  110 + users = 3.times {
  111 + Fabricate(:user)
  112 + }
112 get :index 113 get :index
113 - assigns(:users).to_a.size.should == 2 114 + controller.users.to_a.size.should == 2
114 end 115 end
  116 +
115 end 117 end
116 118
117 context "GET /users/:id" do 119 context "GET /users/:id" do
118 it 'finds the user' do 120 it 'finds the user' do
119 - user = Fabricate(:user)  
120 get :show, :id => user.id 121 get :show, :id => user.id
121 - assigns(:user).should == user 122 + controller.user.should == user
122 end 123 end
123 end 124 end
124 125
125 context "GET /users/new" do 126 context "GET /users/new" do
126 it 'assigns a new user' do 127 it 'assigns a new user' do
127 get :new 128 get :new
128 - assigns(:user).should be_a(User)  
129 - assigns(:user).should be_new_record 129 + controller.user.should be_a(User)
  130 + controller.user.should be_new_record
130 end 131 end
131 end 132 end
132 133
133 context "GET /users/:id/edit" do 134 context "GET /users/:id/edit" do
134 it 'finds the user' do 135 it 'finds the user' do
135 - user = Fabricate(:user)  
136 get :edit, :id => user.id 136 get :edit, :id => user.id
137 - assigns(:user).should == user 137 + controller.user.should == user
138 end 138 end
139 end 139 end
140 140
141 context "POST /users" do 141 context "POST /users" do
142 context "when the create is successful" do 142 context "when the create is successful" do
143 - before do  
144 - @attrs = {:user => Fabricate.attributes_for(:user)}  
145 - end 143 + let(:attrs) { {:user => Fabricate.attributes_for(:user)} }
146 144
147 it "sets a message to display" do 145 it "sets a message to display" do
148 - post :create, @attrs 146 + post :create, attrs
149 request.flash[:success].should include('part of the team') 147 request.flash[:success].should include('part of the team')
150 end 148 end
151 149
152 it "redirects to the user's page" do 150 it "redirects to the user's page" do
153 - post :create, @attrs  
154 - response.should redirect_to(user_path(assigns(:user))) 151 + post :create, attrs
  152 + response.should redirect_to(user_path(controller.user))
155 end 153 end
156 154
157 it "should be able to create admin" do 155 it "should be able to create admin" do
158 - @attrs[:user][:admin] = true  
159 - post :create, @attrs 156 + attrs[:user][:admin] = true
  157 + post :create, attrs
160 response.should be_redirect 158 response.should be_redirect
161 - User.find(assigns(:user).to_param).admin.should be_true 159 + User.find(controller.user.to_param).admin.should be_true
162 end 160 end
163 161
164 it "should has auth token" do 162 it "should has auth token" do
165 - post :create, @attrs 163 + post :create, attrs
166 User.last.authentication_token.should_not be_blank 164 User.last.authentication_token.should_not be_blank
167 end 165 end
168 end 166 end
169 167
170 context "when the create is unsuccessful" do 168 context "when the create is unsuccessful" do
  169 + let(:user) {
  170 + Struct.new(:admin, :attributes).new(true, {})
  171 + }
171 before do 172 before do
172 - @user = Fabricate(:user)  
173 - User.should_receive(:new).and_return(@user)  
174 - @user.should_receive(:save).and_return(false) 173 + User.should_receive(:new).and_return(user)
  174 + user.should_receive(:save).and_return(false)
175 end 175 end
176 176
177 it "renders the new page" do 177 it "renders the new page" do
178 - post :create 178 + post :create, :user => { :username => 'foo' }
179 response.should render_template(:new) 179 response.should render_template(:new)
180 end 180 end
181 end 181 end
@@ -183,59 +183,85 @@ describe UsersController do @@ -183,59 +183,85 @@ describe UsersController do
183 183
184 context "PUT /users/:id" do 184 context "PUT /users/:id" do
185 context "when the update is successful" do 185 context "when the update is successful" do
186 - before do  
187 - @user = Fabricate(:user)  
188 - end  
189 -  
190 - it "sets a message to display" do  
191 - put :update, :id => @user.to_param, :user => {:name => 'Kermit'}  
192 - request.flash[:success].should include('updated')  
193 - end  
194 -  
195 - it "redirects to the user's page" do  
196 - put :update, :id => @user.to_param, :user => {:name => 'Kermit'}  
197 - response.should redirect_to(user_path(@user))  
198 - end  
199 -  
200 - it "should be able to make user an admin" do  
201 - put :update, :id => @user.to_param, :user => {:admin => true}  
202 - response.should be_redirect  
203 - User.find(assigns(:user).to_param).admin.should be_true 186 + before {
  187 + put :update, :id => user.to_param, :user => user_params
  188 + }
  189 +
  190 + context "with normal params" do
  191 + let(:user_params) { {:name => 'Kermit'} }
  192 + it "sets a message to display" do
  193 + expect(request.flash[:success]).to eq I18n.t('controllers.users.flash.update.success', :name => user.name)
  194 + expect(response).to redirect_to(user_path(user))
  195 + end
204 end 196 end
205 end 197 end
206 -  
207 context "when the update is unsuccessful" do 198 context "when the update is unsuccessful" do
208 - before do  
209 - @user = Fabricate(:user)  
210 - end  
211 199
212 it "renders the edit page" do 200 it "renders the edit page" do
213 - put :update, :id => @user.to_param, :user => {:name => nil} 201 + put :update, :id => user.to_param, :user => {:name => nil}
214 response.should render_template(:edit) 202 response.should render_template(:edit)
215 end 203 end
216 end 204 end
217 end 205 end
218 206
219 context "DELETE /users/:id" do 207 context "DELETE /users/:id" do
220 - before do  
221 - @user = Fabricate(:user) 208 +
  209 + context "with a destroy success" do
  210 + let(:user_destroy) { mock(:destroy => true) }
  211 +
  212 + before {
  213 + UserDestroy.should_receive(:new).with(user).and_return(user_destroy)
  214 + delete :destroy, :id => user.id
  215 + }
  216 +
  217 + it 'should destroy user' do
  218 + expect(request.flash[:success]).to eq I18n.t('controllers.users.flash.destroy.success', :name => user.name)
  219 + response.should redirect_to(users_path)
  220 + end
222 end 221 end
223 222
224 - it "destroys the user" do  
225 - delete :destroy, :id => @user.id  
226 - User.where(:id => @user.id).first.should be_nil 223 + context "with trying destroy himself" do
  224 + before {
  225 + UserDestroy.should_not_receive(:new)
  226 + delete :destroy, :id => admin.id
  227 + }
  228 +
  229 + it 'should not destroy user' do
  230 + response.should redirect_to(users_path)
  231 + expect(request.flash[:error]).to eq I18n.t('controllers.users.flash.destroy.error')
  232 + end
227 end 233 end
  234 + end
228 235
229 - it "redirects to the users index page" do  
230 - delete :destroy, :id => @user.id  
231 - response.should redirect_to(users_path) 236 + describe "#user_params" do
  237 + context "with current user not admin" do
  238 + before {
  239 + controller.stub(:current_user).and_return(user)
  240 + controller.stub(:params).and_return(ActionController::Parameters.new(user_param))
  241 + }
  242 + let(:user_param) { {'user' => { :name => 'foo', :admin => true }} }
  243 + it 'not have admin field' do
  244 + expect(controller.send(:user_params)).to eq ({'name' => 'foo'})
  245 + end
  246 + context "with password and password_confirmation empty?" do
  247 + let(:user_param) { {'user' => { :name => 'foo', 'password' => '', 'password_confirmation' => '' }} }
  248 + it 'not have password and password_confirmation field' do
  249 + expect(controller.send(:user_params)).to eq ({'name' => 'foo'})
  250 + end
  251 + end
232 end 252 end
233 253
234 - it "sets a message to display" do  
235 - delete :destroy, :id => @user.id  
236 - request.flash[:success].should include('no longer part of your team') 254 + context "with current user admin" do
  255 + it 'have admin field'
  256 + context "with password and password_confirmation empty?" do
  257 + it 'not have password and password_confirmation field'
  258 + end
  259 + context "on his own user" do
  260 + it 'not have admin field'
  261 + end
237 end 262 end
238 end 263 end
  264 +
239 end 265 end
240 266
241 end 267 end
spec/fabricators/app_fabricator.rb
  Diff comments   View file @44e8ad2
@@ -4,7 +4,9 @@ Fabricator(:app) do @@ -4,7 +4,9 @@ Fabricator(:app) do
4 end 4 end
5 5
6 Fabricator(:app_with_watcher, :from => :app) do 6 Fabricator(:app_with_watcher, :from => :app) do
7 - watchers(:count => 1) { |parent, i| Fabricate.build(:watcher, :app => parent) } 7 + watchers(:count => 1) { |parent, i|
  8 + Fabricate.build(:watcher, :app => parent)
  9 + }
8 end 10 end
9 11
10 Fabricator(:watcher) do 12 Fabricator(:watcher) do
spec/interactors/user_destroy_spec.rb 0 → 100644
  Diff comments   View file @44e8ad2
@@ -0,0 +1,27 @@ @@ -0,0 +1,27 @@
  1 +require 'spec_helper'
  2 +
  3 +describe UserDestroy do
  4 + let(:app) { Fabricate(
  5 + :app,
  6 + :watchers => [
  7 + Fabricate.build(:user_watcher, :user => user)
  8 + ])
  9 + }
  10 +
  11 + describe "#destroy" do
  12 + let!(:user) { Fabricate(:user) }
  13 + it 'should delete user' do
  14 + expect {
  15 + UserDestroy.new(user).destroy
  16 + }.to change(User, :count)
  17 + end
  18 +
  19 + it 'should delete watcher' do
  20 + expect {
  21 + UserDestroy.new(user).destroy
  22 + }.to change{
  23 + app.reload.watchers.where(:user_id => user.id).count
  24 + }.from(1).to(0)
  25 + end
  26 + end
  27 +end
spec/models/user_spec.rb
  Diff comments   View file @44e8ad2
@@ -49,15 +49,6 @@ describe User do @@ -49,15 +49,6 @@ describe User do
49 user.watchers.should include(watcher) 49 user.watchers.should include(watcher)
50 end 50 end
51 51
52 - it "destroys any related watchers when it is destroyed" do  
53 - user = Fabricate(:user)  
54 - app = Fabricate(:app)  
55 - watcher = Fabricate(:user_watcher, :app => app, :user => user)  
56 - user.watchers.should_not be_empty  
57 - user.destroy  
58 - app.reload.watchers.should_not include(watcher)  
59 - end  
60 -  
61 it "has many apps through watchers" do 52 it "has many apps through watchers" do
62 user = Fabricate(:user) 53 user = Fabricate(:user)
63 watched_app = Fabricate(:app) 54 watched_app = Fabricate(:app)
spec/views/users/edit.html.haml_spec.rb 0 → 100644
  Diff comments   View file @44e8ad2
@@ -0,0 +1,18 @@ @@ -0,0 +1,18 @@
  1 +require 'spec_helper'
  2 +
  3 +describe 'users/edit.html.haml' do
  4 + let(:user) { stub_model(User, :name => 'shingara') }
  5 + before {
  6 + view.stub(:current_user).and_return(user)
  7 + view.stub(:user).and_return(user)
  8 + }
  9 + it 'should have per_page option' do
  10 + render
  11 + expect(rendered).to match(/id="user_per_page"/)
  12 + end
  13 +
  14 + it 'should have time_zone option' do
  15 + render
  16 + expect(rendered).to match(/id="user_time_zone"/)
  17 + end
  18 +end
spec/views/users/index.html.haml_spec.rb 0 → 100644
  Diff comments   View file @44e8ad2
@@ -0,0 +1,16 @@ @@ -0,0 +1,16 @@
  1 +require 'spec_helper'
  2 +
  3 +describe 'users/index.html.haml' do
  4 + let(:user) { stub_model(User) }
  5 + before {
  6 + view.stub(:current_user).and_return(user)
  7 + view.stub(:users).and_return(
  8 + Kaminari.paginate_array([user], :total_count => 1).page(1)
  9 + )
  10 + }
  11 + it 'should see users option' do
  12 + render
  13 + expect(rendered).to match(/class='user_list'/)
  14 + end
  15 +
  16 +end
spec/views/users/new.html.haml_spec.rb 0 → 100644
  Diff comments   View file @44e8ad2
@@ -0,0 +1,18 @@ @@ -0,0 +1,18 @@
  1 +require 'spec_helper'
  2 +
  3 +describe 'users/new.html.haml' do
  4 + let(:user) { stub_model(User) }
  5 + before {
  6 + view.stub(:current_user).and_return(user)
  7 + view.stub(:user).and_return(user)
  8 + }
  9 + it 'should have per_page option' do
  10 + render
  11 + expect(rendered).to match(/id="user_per_page"/)
  12 + end
  13 +
  14 + it 'should have time_zone option' do
  15 + render
  16 + expect(rendered).to match(/id="user_time_zone"/)
  17 + end
  18 +end
spec/views/users/show.html.haml_spec.rb
  Diff comments   View file @44e8ad2
1 require 'spec_helper' 1 require 'spec_helper'
2 2
3 describe 'users/show.html.haml' do 3 describe 'users/show.html.haml' do
  4 +
4 let(:user) do 5 let(:user) do
5 stub_model(User, :created_at => Time.now, :email => "test@example.com") 6 stub_model(User, :created_at => Time.now, :email => "test@example.com")
6 end 7 end
@@ -8,12 +9,12 @@ describe &#39;users/show.html.haml&#39; do @@ -8,12 +9,12 @@ describe &#39;users/show.html.haml&#39; do
8 before do 9 before do
9 Errbit::Config.stub(:github_authentication) { true } 10 Errbit::Config.stub(:github_authentication) { true }
10 controller.stub(:current_user) { stub_model(User) } 11 controller.stub(:current_user) { stub_model(User) }
  12 + view.stub(:user) { user }
11 end 13 end
12 14
13 context 'with GitHub authentication' do 15 context 'with GitHub authentication' do
14 it 'shows github login' do 16 it 'shows github login' do
15 user.github_login = 'test_user' 17 user.github_login = 'test_user'
16 - assign :user, user  
17 render 18 render
18 rendered.should match(/GitHub/) 19 rendered.should match(/GitHub/)
19 rendered.should match(/test_user/) 20 rendered.should match(/test_user/)
@@ -21,7 +22,6 @@ describe &#39;users/show.html.haml&#39; do @@ -21,7 +22,6 @@ describe &#39;users/show.html.haml&#39; do
21 22
22 it 'does not show github if blank' do 23 it 'does not show github if blank' do
23 user.github_login = ' ' 24 user.github_login = ' '
24 - assign :user, user  
25 render 25 render
26 rendered.should_not match(/GitHub/) 26 rendered.should_not match(/GitHub/)
27 end 27 end
@@ -30,7 +30,6 @@ describe &#39;users/show.html.haml&#39; do @@ -30,7 +30,6 @@ describe &#39;users/show.html.haml&#39; do
30 context "Linking GitHub account" do 30 context "Linking GitHub account" do
31 context 'viewing another user page' do 31 context 'viewing another user page' do
32 it "doesn't show and github linking buttons if user is not current user" do 32 it "doesn't show and github linking buttons if user is not current user" do
33 - assign :user, user  
34 render 33 render
35 view.content_for(:action_bar).should_not include('Link GitHub account') 34 view.content_for(:action_bar).should_not include('Link GitHub account')
36 view.content_for(:action_bar).should_not include('Unlink GitHub account') 35 view.content_for(:action_bar).should_not include('Unlink GitHub account')
@@ -40,7 +39,6 @@ describe &#39;users/show.html.haml&#39; do @@ -40,7 +39,6 @@ describe &#39;users/show.html.haml&#39; do
40 context 'viewing own user page' do 39 context 'viewing own user page' do
41 before do 40 before do
42 controller.stub(:current_user) { user } 41 controller.stub(:current_user) { user }
43 - assign :user, user  
44 end 42 end
45 43
46 it 'shows link github button when no login or token' do 44 it 'shows link github button when no login or token' do