diff options
-rw-r--r-- | app/assets/javascripts/dispatcher.js.coffee | 2 | ||||
-rw-r--r-- | app/controllers/groups/application_controller.rb | 18 | ||||
-rw-r--r-- | app/controllers/groups/group_members_controller.rb | 23 | ||||
-rw-r--r-- | app/controllers/groups_controller.rb | 15 | ||||
-rw-r--r-- | app/views/groups/group_members/_new_group_member.html.haml (renamed from app/views/groups/_new_group_member.html.haml) | 4 | ||||
-rw-r--r-- | app/views/groups/group_members/index.html.haml (renamed from app/views/groups/members.html.haml) | 4 | ||||
-rw-r--r-- | app/views/layouts/nav/_group.html.haml | 4 | ||||
-rw-r--r-- | config/routes.rb | 5 | ||||
-rw-r--r-- | features/steps/explore/groups.rb | 2 | ||||
-rw-r--r-- | features/steps/shared/paths.rb | 4 | ||||
-rw-r--r-- | spec/features/security/group/group_access_spec.rb | 4 | ||||
-rw-r--r-- | spec/features/security/group/internal_group_access_spec.rb | 4 | ||||
-rw-r--r-- | spec/features/security/group/mixed_group_access_spec.rb | 4 | ||||
-rw-r--r-- | spec/features/security/group/public_group_access_spec.rb | 4 |
14 files changed, 60 insertions, 37 deletions
diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index 5da774cfb23..4dce6e66ce2 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -73,7 +73,7 @@ class Dispatcher new Activities() shortcut_handler = new ShortcutsNavigation() new ProjectsList() - when 'groups:members' + when 'groups:group_members:index' new GroupMembers() new UsersSelect() when 'groups:new', 'groups:edit', 'admin:groups:edit' diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index 7f27f2bb734..a73b8fa212a 100644 --- a/app/controllers/groups/application_controller.rb +++ b/app/controllers/groups/application_controller.rb @@ -2,9 +2,27 @@ class Groups::ApplicationController < ApplicationController private + def authorize_read_group! + unless @group and can?(current_user, :read_group, @group) + if current_user.nil? + return authenticate_user! + else + return render_404 + end + end + end + def authorize_admin_group! unless can?(current_user, :manage_group, group) return render_404 end end + + def determine_layout + if current_user + 'group' + else + 'public_group' + end + end end diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 132452d61c9..d3d6ce1ca2c 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -1,15 +1,30 @@ class Groups::GroupMembersController < Groups::ApplicationController + skip_before_filter :authenticate_user!, only: [:index] before_filter :group # Authorize - before_filter :authorize_admin_group! + before_filter :authorize_read_group! + before_filter :authorize_admin_group!, except: [:index, :leave] - layout 'group' + layout :determine_layout + + def index + @project = @group.projects.find(params[:project_id]) if params[:project_id] + @members = @group.group_members + + if params[:search].present? + users = @group.users.search(params[:search]).to_a + @members = @members.where(user_id: users) + end + + @members = @members.order('access_level DESC').page(params[:page]).per(50) + @group_member = GroupMember.new + end def create @group.add_users(params[:user_ids].split(','), params[:access_level]) - redirect_to members_group_path(@group), notice: 'Users were successfully added.' + redirect_to group_group_members_path(@group), notice: 'Users were successfully added.' end def update @@ -23,7 +38,7 @@ class Groups::GroupMembersController < Groups::ApplicationController if can?(current_user, :destroy_group_member, @group_member) # May fail if last owner. @group_member.destroy respond_to do |format| - format.html { redirect_to members_group_path(@group), notice: 'User was successfully removed from group.' } + format.html { redirect_to group_group_members_path(@group), notice: 'User was successfully removed from group.' } format.js { render nothing: true } end else diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 7e336803fbb..7af3c077182 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -1,5 +1,5 @@ class GroupsController < Groups::ApplicationController - skip_before_filter :authenticate_user!, only: [:show, :issues, :members, :merge_requests] + skip_before_filter :authenticate_user!, only: [:show, :issues, :merge_requests] respond_to :html before_filter :group, except: [:new, :create] @@ -67,19 +67,6 @@ class GroupsController < Groups::ApplicationController end end - def members - @project = group.projects.find(params[:project_id]) if params[:project_id] - @members = group.group_members - - if params[:search].present? - users = group.users.search(params[:search]).to_a - @members = @members.where(user_id: users) - end - - @members = @members.order('access_level DESC').page(params[:page]).per(50) - @users_group = GroupMember.new - end - def edit end diff --git a/app/views/groups/_new_group_member.html.haml b/app/views/groups/group_members/_new_group_member.html.haml index 345c0555a36..c4c29bb2e8d 100644 --- a/app/views/groups/_new_group_member.html.haml +++ b/app/views/groups/group_members/_new_group_member.html.haml @@ -1,4 +1,4 @@ -= form_for @users_group, url: group_group_members_path(@group), html: { class: 'form-horizontal users-group-form' } do |f| += form_for @group_member, url: group_group_members_path(@group), html: { class: 'form-horizontal users-group-form' } do |f| .form-group = f.label :user_ids, "People", class: 'control-label' .col-sm-10= users_select_tag(:user_ids, multiple: true, class: 'input-large') @@ -6,7 +6,7 @@ .form-group = f.label :access_level, "Group Access", class: 'control-label' .col-sm-10 - = select_tag :access_level, options_for_select(GroupMember.access_level_roles, @users_group.access_level), class: "project-access-select select2" + = select_tag :access_level, options_for_select(GroupMember.access_level_roles, @group_member.access_level), class: "project-access-select select2" .help-block Read more about role permissions %strong= link_to "here", help_page_path("permissions", "permissions"), class: "vlink" diff --git a/app/views/groups/members.html.haml b/app/views/groups/group_members/index.html.haml index 688c22e9624..0d501fe7bd3 100644 --- a/app/views/groups/members.html.haml +++ b/app/views/groups/group_members/index.html.haml @@ -1,4 +1,5 @@ - show_roles = should_user_see_group_roles?(current_user, @group) + %h3.page-title Group members - if show_roles @@ -10,7 +11,7 @@ %hr .clearfix.js-toggle-container - = form_tag members_group_path(@group), method: :get, class: 'form-inline member-search-form' do + = form_tag group_group_members_path(@group), method: :get, class: 'form-inline member-search-form' do .form-group = search_field_tag :search, params[:search], { placeholder: 'Find existing member by name', class: 'form-control search-text-input input-mn-300' } = button_tag 'Search', class: 'btn' @@ -33,6 +34,7 @@ %ul.well-list - @members.each do |member| = render 'groups/group_members/group_member', member: member, show_roles: show_roles, show_controls: true + = paginate @members, theme: 'gitlab' :coffeescript diff --git a/app/views/layouts/nav/_group.html.haml b/app/views/layouts/nav/_group.html.haml index ddd3df19eec..32fe0e37df8 100644 --- a/app/views/layouts/nav/_group.html.haml +++ b/app/views/layouts/nav/_group.html.haml @@ -24,8 +24,8 @@ Merge Requests - if current_user %span.count= MergeRequest.opened.of_group(@group).count - = nav_link(path: 'groups#members') do - = link_to members_group_path(@group), title: 'Members' do + = nav_link(controller: [:group_members]) do + = link_to group_group_members_path(@group), title: 'Members' do %i.fa.fa-users %span Members diff --git a/config/routes.rb b/config/routes.rb index 1b7ae09c773..459158dcb61 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -236,12 +236,13 @@ Gitlab::Application.routes.draw do member do get :issues get :merge_requests - get :members get :projects end scope module: :groups do - resources :group_members, only: [:create, :update, :destroy] + resources :group_members, only: [:index, :create, :update, :destroy] do + end + resource :avatar, only: [:destroy] resources :milestones, only: [:index, :show, :update] end diff --git a/features/steps/explore/groups.rb b/features/steps/explore/groups.rb index ccbf6cda07e..0c2127d4c4b 100644 --- a/features/steps/explore/groups.rb +++ b/features/steps/explore/groups.rb @@ -35,7 +35,7 @@ class Spinach::Features::ExploreGroups < Spinach::FeatureSteps end step 'I visit group "TestGroup" members page' do - visit members_group_path(Group.find_by(name: "TestGroup")) + visit group_group_members_path(Group.find_by(name: "TestGroup")) end step 'I should not see project "Enterprise" items' do diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index 77a90e80d14..e3cf1b92cda 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -32,7 +32,7 @@ module SharedPaths end step 'I visit group "Owned" members page' do - visit members_group_path(Group.find_by(name:"Owned")) + visit group_group_members_path(Group.find_by(name:"Owned")) end step 'I visit group "Owned" settings page' do @@ -52,7 +52,7 @@ module SharedPaths end step 'I visit group "Guest" members page' do - visit members_group_path(Group.find_by(name:"Guest")) + visit group_group_members_path(Group.find_by(name:"Guest")) end step 'I visit group "Guest" settings page' do diff --git a/spec/features/security/group/group_access_spec.rb b/spec/features/security/group/group_access_spec.rb index e0c5cbf4d3d..63793149459 100644 --- a/spec/features/security/group/group_access_spec.rb +++ b/spec/features/security/group/group_access_spec.rb @@ -59,8 +59,8 @@ describe "Group access", feature: true do it { is_expected.to be_denied_for :visitor } end - describe "GET /groups/:path/members" do - subject { members_group_path(group) } + describe "GET /groups/:path/group_members" do + subject { group_group_members_path(group) } it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } diff --git a/spec/features/security/group/internal_group_access_spec.rb b/spec/features/security/group/internal_group_access_spec.rb index 5279a1bc13a..d17a7412e43 100644 --- a/spec/features/security/group/internal_group_access_spec.rb +++ b/spec/features/security/group/internal_group_access_spec.rb @@ -55,8 +55,8 @@ describe "Group with internal project access", feature: true do it { is_expected.to be_denied_for :visitor } end - describe "GET /groups/:path/members" do - subject { members_group_path(group) } + describe "GET /groups/:path/group_members" do + subject { group_group_members_path(group) } it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } diff --git a/spec/features/security/group/mixed_group_access_spec.rb b/spec/features/security/group/mixed_group_access_spec.rb index efd14858b98..b3db7b5dea4 100644 --- a/spec/features/security/group/mixed_group_access_spec.rb +++ b/spec/features/security/group/mixed_group_access_spec.rb @@ -56,8 +56,8 @@ describe "Group access", feature: true do it { is_expected.to be_allowed_for :visitor } end - describe "GET /groups/:path/members" do - subject { members_group_path(group) } + describe "GET /groups/:path/group_members" do + subject { group_group_members_path(group) } it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } diff --git a/spec/features/security/group/public_group_access_spec.rb b/spec/features/security/group/public_group_access_spec.rb index c7e3d0a8a40..c16f0c0d1e1 100644 --- a/spec/features/security/group/public_group_access_spec.rb +++ b/spec/features/security/group/public_group_access_spec.rb @@ -55,8 +55,8 @@ describe "Group with public project access", feature: true do it { is_expected.to be_allowed_for :visitor } end - describe "GET /groups/:path/members" do - subject { members_group_path(group) } + describe "GET /groups/:path/group_members" do + subject { group_group_members_path(group) } it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } |