Download as
Browse Code »

Commit 6b16f8a7a84a91bafac97e1e7e2390e32877eabb

Authored by Dmitriy Zaporozhets
2 parents 2358489f eb4272e2
Exists in master and in 4 other branches 6-8-stable, 7-0-stable, 7-0-stable-spb, spb-stable

Merge branch 'improve/user_removal' of /home/git/repositories/gitlab/gitlabhq

Showing 10 changed files with 77 additions and 40 deletions   Show diff stats
app/assets/javascripts/admin.js.coffee
  Diff comments   View file @6b16f8a
... ... @@ -23,10 +23,16 @@ class Admin
23 23 e.preventDefault()
24 24 $(this).hide()
25 25 modal.show()
26   -
  26 +
27 27 $('.change-owner-cancel-link').bind "click", (e) ->
28 28 e.preventDefault()
29 29 modal.hide()
30 30 $('.change-owner-link').show()
31 31  
  32 + $('li.users_project').bind 'ajax:success', ->
  33 + Turbolinks.visit(location.href)
  34 +
  35 + $('li.users_group').bind 'ajax:success', ->
  36 + Turbolinks.visit(location.href)
  37 +
32 38 @Admin = Admin
... ...
app/controllers/admin/members_controller.rb
View file @2358489
... ... @@ -1,9 +0,0 @@
1   -class Admin::MembersController < Admin::ApplicationController
2   - def destroy
3   - user = User.find_by_username(params[:id])
4   - project = Project.find_with_namespace(params[:project_id])
5   - project.users_projects.where(user_id: user).first.destroy
6   -
7   - redirect_to :back
8   - end
9   -end
app/controllers/admin/users_controller.rb
  Diff comments   View file @6b16f8a
... ... @@ -83,9 +83,10 @@ class Admin::UsersController &lt; Admin::ApplicationController
83 83 end
84 84  
85 85 def destroy
86   - if user.personal_projects.count > 0
87   - redirect_to admin_users_path, alert: "User is a project owner and can't be removed." and return
88   - end
  86 + # 1. Remove groups where user is the only owner
  87 + user.solo_owned_groups.map(&:destroy)
  88 +
  89 + # 2. Remove user with all authored content including personal projects
89 90 user.destroy
90 91  
91 92 respond_to do |format|
... ...
app/models/group.rb
  Diff comments   View file @6b16f8a
... ... @@ -23,7 +23,7 @@ class Group &lt; Namespace
23 23 end
24 24  
25 25 def owners
26   - @owners ||= (users_groups.owners.map(&:user) << owner)
  26 + @owners ||= (users_groups.owners.map(&:user) << owner).uniq
27 27 end
28 28  
29 29 def add_users(user_ids, group_access)
... ...
app/models/project_team.rb
  Diff comments   View file @6b16f8a
... ... @@ -32,7 +32,15 @@ class ProjectTeam
32 32 end
33 33  
34 34 def find_tm(user_id)
35   - project.users_projects.find_by_user_id(user_id)
  35 + tm = project.users_projects.find_by_user_id(user_id)
  36 +
  37 + # If user is not in project members
  38 + # we should check for group membership
  39 + if group && !tm
  40 + tm = group.users_groups.find_by_user_id(user_id)
  41 + end
  42 +
  43 + tm
36 44 end
37 45  
38 46 def add_user(user, access)
... ...
app/models/user.rb
  Diff comments   View file @6b16f8a
... ... @@ -135,7 +135,7 @@ class User &lt; ActiveRecord::Base
135 135 # Remove user from all groups
136 136 user.users_groups.find_each do |membership|
137 137 # skip owned resources
138   - next if membership.group.owner == user
  138 + next if membership.group.owners.include?(user)
139 139  
140 140 return false unless membership.destroy
141 141 end
... ... @@ -376,4 +376,10 @@ class User &lt; ActiveRecord::Base
376 376 self.send("#{attr}=", Sanitize.clean(value)) if value.present?
377 377 end
378 378 end
  379 +
  380 + def solo_owned_groups
  381 + @solo_owned_groups ||= owned_groups.select do |group|
  382 + group.owners == [self]
  383 + end
  384 + end
379 385 end
... ...
app/views/admin/users/show.html.haml
  Diff comments   View file @6b16f8a
... ... @@ -7,15 +7,11 @@
7 7 %span.cred (Admin)
8 8  
9 9 .pull-right
10   - = link_to edit_admin_user_path(@user), class: "btn grouped btn-small" do
  10 + = link_to edit_admin_user_path(@user), class: "btn grouped" do
11 11 %i.icon-edit
12 12 Edit
13   - - unless @user == current_user
14   - - if @user.blocked?
15   - = link_to 'Unblock', unblock_admin_user_path(@user), method: :put, class: "btn grouped btn-small success"
16   - - else
17   - = link_to 'Block', block_admin_user_path(@user), confirm: 'USER WILL BE BLOCKED! Are you sure?', method: :put, class: "btn grouped btn-small btn-remove"
18   - = link_to 'Destroy', [:admin, @user], confirm: "USER #{@user.name} WILL BE REMOVED! Are you sure?", method: :delete, class: "btn grouped btn-small btn-remove"
  13 + - if @user.blocked?
  14 + = link_to 'Unblock', unblock_admin_user_path(@user), method: :put, class: "btn grouped success"
19 15 %hr
20 16  
21 17 .row
... ... @@ -63,24 +59,56 @@
63 59 %strong
64 60 = link_to @user.created_by.name, [:admin, @user.created_by]
65 61  
  62 + - unless @user == current_user
  63 + .alert
  64 + %h4 Block user
  65 + %br
  66 + %p Blocking user has the following effects:
  67 + %ul
  68 + %li User will not be able to login
  69 + %li User will not be able to access git repositories
  70 + %li User will be removed from joined projects and groups
  71 + %li Personal projects will be left
  72 + %li Owned groups will be left
  73 + = link_to 'Block user', block_admin_user_path(@user), confirm: 'USER WILL BE BLOCKED! Are you sure?', method: :put, class: "btn btn-remove"
  74 +
  75 + .alert.alert-error
  76 + %h4
  77 + Remove user
  78 + %br
  79 + %p Deleting a user has the following effects:
  80 + %ul
  81 + %li All user content like authored issues, snippets, comments will be removed
  82 + - rp = @user.personal_projects.count
  83 + - unless rp.zero?
  84 + %li #{pluralize rp, 'personal project'} will be removed and cannot be restored
  85 + - if @user.solo_owned_groups.present?
  86 + %li
  87 + Next groups with all content will be removed:
  88 + %strong #{@user.solo_owned_groups.map(&:name).join(', ')}
  89 + = link_to 'Remove user', [:admin, @user], confirm: "USER #{@user.name} WILL BE REMOVED! Are you sure?", method: :delete, class: "btn btn-remove"
  90 +
  91 + .span6
66 92 - if @user.users_groups.present?
67 93 .ui-box
68 94 .title Groups:
69 95 %ul.well-list
70 96 - @user.users_groups.each do |user_group|
71 97 - group = user_group.group
72   - %li
  98 + %li.users_group
73 99 %strong= link_to group.name, admin_group_path(group)
74 100 .pull-right
75 101 %span.light= user_group.human_access
  102 + - unless user_group.owner?
  103 + = link_to group_users_group_path(group, user_group), confirm: remove_user_from_group_message(group, @user), method: :delete, remote: true, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do
  104 + %i.icon-remove.icon-white
76 105  
77   - .span6
78 106 .ui-box
79 107 .title Projects (#{@projects.count})
80 108 %ul.well-list
81 109 - @projects.sort_by(&:name_with_namespace).each do |project|
82 110 - tm = project.team.find_tm(@user.id)
83   - %li
  111 + %li.users_project
84 112 = link_to admin_project_path(project), class: dom_class(project) do
85 113 - if project.namespace
86 114 = project.namespace.human_name
... ... @@ -94,5 +122,9 @@
94 122 %span.light Owner
95 123 - else
96 124 %span.light= tm.human_access
97   - = link_to admin_project_member_path(project, tm.user), confirm: remove_from_project_team_message(project, @user), method: :delete, class: "btn btn-small btn-remove" do
98   - %i.icon-remove
  125 +
  126 + - if tm.respond_to? :project
  127 + = link_to project_team_member_path(project, @user), confirm: remove_from_project_team_message(project, @user), remote: true, method: :delete, class: "btn-tiny btn btn-remove", title: 'Remove user from project' do
  128 + %i.icon-remove
  129 +
  130 +
... ...
config/routes.rb
  Diff comments   View file @6b16f8a
... ... @@ -89,11 +89,7 @@ Gitlab::Application.routes.draw do
89 89  
90 90 resource :logs, only: [:show]
91 91 resource :background_jobs, controller: 'background_jobs', only: [:show]
92   -
93   - resources :projects, constraints: { id: /[a-zA-Z.\/0-9_\-]+/ }, only: [:index, :show] do
94   - resources :members, only: [:destroy]
95   - end
96   -
  92 + resources :projects, constraints: { id: /[a-zA-Z.\/0-9_\-]+/ }, only: [:index, :show]
97 93 root to: "dashboard#index"
98 94 end
99 95  
... ...
lib/gitlab/access.rb
  Diff comments   View file @6b16f8a
... ... @@ -44,5 +44,9 @@ module Gitlab
44 44 def human_access
45 45 Gitlab::Access.options_with_owner.key(access_field)
46 46 end
  47 +
  48 + def owner?
  49 + access_field == OWNER
  50 + end
47 51 end
48 52 end
... ...
spec/routing/admin_routing_spec.rb
  Diff comments   View file @6b16f8a
... ... @@ -75,13 +75,6 @@ describe Admin::ProjectsController, &quot;routing&quot; do
75 75 end
76 76 end
77 77  
78   -# DELETE /admin/projects/:project_id/members/:id(.:format) admin/projects/members#destroy {id: /[^\/]+/, project_id: /[^\/]+/}
79   -describe Admin::MembersController, "routing" do
80   - it "to #destroy" do
81   - delete("/admin/projects/test/members/1").should route_to('admin/members#destroy', project_id: 'test', id: '1')
82   - end
83   -end
84   -
85 78 # admin_hook_test GET /admin/hooks/:hook_id/test(.:format) admin/hooks#test
86 79 # admin_hooks GET /admin/hooks(.:format) admin/hooks#index
87 80 # POST /admin/hooks(.:format) admin/hooks#create
... ...