Commit f835862a9b231c6b9b9a1034aa82ec6ed7fc6afe
1 parent
9a1bc7ba
Exists in
master
and in
29 other branches
Avoid crash when try to remove unexistent profile
(ActionItem3021)
Showing
3 changed files
with
21 additions
and
5 deletions
Show diff stats
app/controllers/admin/users_controller.rb
... | ... | @@ -48,11 +48,11 @@ class UsersController < AdminController |
48 | 48 | |
49 | 49 | def destroy_user |
50 | 50 | if request.post? |
51 | - person = environment.people.find(params[:id]) | |
52 | - if person.destroy | |
51 | + person = environment.people.find_by_id(params[:id]) | |
52 | + if person && person.destroy | |
53 | 53 | session[:notice] = _('The profile was deleted.') |
54 | 54 | else |
55 | - session[:notice] = _('Could not delete profile') | |
55 | + session[:notice] = _('Could not remove profile') | |
56 | 56 | end |
57 | 57 | end |
58 | 58 | redirect_to :action => :index, :q => params[:q], :filter => params[:filter] | ... | ... |
features/manage_users.feature
... | ... | @@ -26,7 +26,7 @@ Background: |
26 | 26 | Then I should see "Deactivate user" within "tr[title='Paulo Santos']" |
27 | 27 | |
28 | 28 | @selenium |
29 | - Scenario: ban user | |
29 | + Scenario: remove user | |
30 | 30 | When I follow "Remove" within "tr[title='Joao Silva']" |
31 | 31 | And I confirm the "Do you want to remove this user?" dialog |
32 | 32 | And I go to /admin/users |
... | ... | @@ -44,4 +44,4 @@ Background: |
44 | 44 | And I confirm the "Do you want to set this user as administrator?" dialog |
45 | 45 | When I follow "Reset admin role" within "tr[title='Paulo Santos']" |
46 | 46 | And I confirm the "Do you want to reset this user as administrator?" dialog |
47 | - Then I should see "Set admin role" within "tr[title='Paulo Santos']" | |
48 | 47 | \ No newline at end of file |
48 | + Then I should see "Set admin role" within "tr[title='Paulo Santos']" | ... | ... |
test/functional/users_controller_test.rb
... | ... | @@ -133,4 +133,20 @@ class UsersControllerTest < ActionController::TestCase |
133 | 133 | assert_equal 'name;email', @response.body.split("\n")[0] |
134 | 134 | end |
135 | 135 | |
136 | + should 'be able to remove a person' do | |
137 | + person = fast_create(Person, :environment_id => environment.id) | |
138 | + assert_difference Person, :count, -1 do | |
139 | + post :destroy_user, :id => person.id | |
140 | + end | |
141 | + end | |
142 | + | |
143 | + should 'not crash if user does not exist' do | |
144 | + person = fast_create(Person) | |
145 | + | |
146 | + assert_no_difference Person, :count do | |
147 | + post :destroy_user, :id => 99999 | |
148 | + end | |
149 | + assert_redirected_to :action => 'index' | |
150 | + end | |
151 | + | |
136 | 152 | end | ... | ... |