diff options
31 files changed, 225 insertions, 98 deletions
diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index a6db4690df0..94b5aaa71d0 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -10,6 +10,7 @@ class Admin::GroupsController < Admin::ApplicationController def show @members = @group.members.order("access_level DESC").page(params[:members_page]) + @requesters = @group.requesters @projects = @group.projects.page(params[:projects_page]) end diff --git a/app/controllers/admin/projects_controller.rb b/app/controllers/admin/projects_controller.rb index 87986fdf8b1..4c9c6362ffc 100644 --- a/app/controllers/admin/projects_controller.rb +++ b/app/controllers/admin/projects_controller.rb @@ -20,7 +20,8 @@ class Admin::ProjectsController < Admin::ApplicationController @group_members = @group.members.order("access_level DESC").page(params[:group_members_page]) end - @project_members = @project.project_members.page(params[:project_members_page]) + @project_members = @project.members.page(params[:project_members_page]) + @requesters = @project.requesters end def transfer diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index 52dc396af6a..52682ef9dc9 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -10,7 +10,7 @@ module MembershipActions end def approve_access_request - @member = membershipable.members.request.find(params[:id]) + @member = membershipable.requesters.find(params[:id]) return render_403 unless can?(current_user, action_member_permission(:update, @member), @member) @@ -20,7 +20,8 @@ module MembershipActions end def leave - @member = membershipable.members.find_by(user_id: current_user) + @member = membershipable.members.find_by(user_id: current_user) || + membershipable.requesters.find_by(user_id: current_user) Members::DestroyService.new(@member, current_user).execute source_type = @member.real_source_type.humanize(capitalize: false) diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 2c49fe3833e..9fc41a12536 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -7,7 +7,7 @@ class Groups::GroupMembersController < Groups::ApplicationController def index @project = @group.projects.find(params[:project_id]) if params[:project_id] @members = @group.group_members - @members = @members.non_pending unless can?(current_user, :admin_group, @group) + @members = @members.non_invite unless can?(current_user, :admin_group, @group) if params[:search].present? users = @group.users.search(params[:search]).to_a @@ -15,6 +15,7 @@ class Groups::GroupMembersController < Groups::ApplicationController end @members = @members.order('access_level DESC').page(params[:page]).per(50) + @requesters = @group.requesters if can?(current_user, :admin_group, @group) @group_member = @group.group_members.new end @@ -34,7 +35,8 @@ class Groups::GroupMembersController < Groups::ApplicationController end def destroy - @group_member = @group.group_members.find(params[:id]) + @group_member = @group.members.find_by(id: params[:id]) || + @group.requesters.find_by(id: params[:id]) Members::DestroyService.new(@group_member, current_user).execute diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 6ba32d33403..3435a118964 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -6,7 +6,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController def index @project_members = @project.project_members - @project_members = @project_members.non_pending unless can?(current_user, :admin_project, @project) + @project_members = @project_members.non_invite unless can?(current_user, :admin_project, @project) if params[:search].present? users = @project.users.search(params[:search]).to_a @@ -19,7 +19,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController if @group @group_members = @group.group_members - @group_members = @group_members.non_pending unless can?(current_user, :admin_group, @group) + @group_members = @group_members.non_invite unless can?(current_user, :admin_group, @group) if params[:search].present? users = @group.users.search(params[:search]).to_a @@ -29,6 +29,8 @@ class Projects::ProjectMembersController < Projects::ApplicationController @group_members = @group_members.order('access_level DESC') end + @requesters = @project.requesters if can?(current_user, :admin_project, @project) + @project_member = @project.project_members.new @project_group_links = @project.project_group_links end @@ -48,7 +50,8 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def destroy - @project_member = @project.project_members.find(params[:id]) + @project_member = @project.members.find_by(id: params[:id]) || + @project.requesters.find_by(id: params[:id]) Members::DestroyService.new(@project_member, current_user).execute diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb index ec106418f2d..c70cd19b587 100644 --- a/app/helpers/members_helper.rb +++ b/app/helpers/members_helper.rb @@ -12,6 +12,17 @@ module MembersHelper can?(current_user, action_member_permission(:admin, member), member.source) end + def can_see_request_access_button?(source) + source_parent = source.respond_to?(:group) && source.group + + return false if source_parent && source.group.members.exists?(user_id: current_user.id) + return false if source_parent && source.group.requesters.exists?(user_id: current_user.id) + return false if source.members.exists?(user_id: current_user.id) + return true if source.requesters.exists?(user_id: current_user.id) + + true + end + def remove_member_message(member, user: nil) user = current_user if defined?(current_user) diff --git a/app/models/group.rb b/app/models/group.rb index c70c719e338..a8be7004ee8 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -6,15 +6,16 @@ class Group < Namespace include AccessRequestable include Referable - has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember' + has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'GroupMember' alias_method :members, :group_members - has_many :users, -> { where(members: { requested_at: nil }) }, through: :group_members - + has_many :users, through: :group_members has_many :owners, - -> { where(members: { requested_at: nil, access_level: Gitlab::Access::OWNER }) }, + -> { where(members: { access_level: Gitlab::Access::OWNER }) }, through: :group_members, source: :user + has_many :requesters, -> { where.not(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'GroupMember' + has_many :project_group_links, dependent: :destroy has_many :shared_projects, through: :project_group_links, source: :project has_many :notification_settings, dependent: :destroy, as: :source diff --git a/app/models/member.rb b/app/models/member.rb index 57161397e2b..44db3d977fa 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -30,8 +30,6 @@ class Member < ActiveRecord::Base scope :invite, -> { where.not(invite_token: nil) } scope :non_invite, -> { where(invite_token: nil) } scope :request, -> { where.not(requested_at: nil) } - scope :non_request, -> { where(requested_at: nil) } - scope :non_pending, -> { non_request.non_invite } scope :has_access, -> { where('access_level > 0') } scope :guests, -> { where(access_level: GUEST) } diff --git a/app/models/project.rb b/app/models/project.rb index 6a950ee830d..ae96f00a705 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -108,9 +108,13 @@ class Project < ActiveRecord::Base has_many :snippets, dependent: :destroy, class_name: 'ProjectSnippet' has_many :hooks, dependent: :destroy, class_name: 'ProjectHook' has_many :protected_branches, dependent: :destroy - has_many :project_members, dependent: :destroy, as: :source, class_name: 'ProjectMember' + + has_many :project_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'ProjectMember' alias_method :members, :project_members - has_many :users, -> { where(members: { requested_at: nil }) }, through: :project_members + has_many :users, through: :project_members + + has_many :requesters, -> { where.not(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'ProjectMember' + has_many :deploy_keys_projects, dependent: :destroy has_many :deploy_keys, through: :deploy_keys_projects has_many :users_star_projects, dependent: :destroy diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 0865b979ce0..0b700930641 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -22,12 +22,12 @@ class ProjectTeam end def find_member(user_id) - member = project.members.non_request.find_by(user_id: user_id) + member = project.members.find_by(user_id: user_id) # If user is not in project members # we should check for group membership if group && !member - member = group.members.non_request.find_by(user_id: user_id) + member = group.members.find_by(user_id: user_id) end member @@ -137,10 +137,10 @@ class ProjectTeam def max_member_access(user_id) access = [] - access += project.members.non_request.where(user_id: user_id).has_access.pluck(:access_level) + access += project.members.where(user_id: user_id).has_access.pluck(:access_level) if group - access += group.members.non_request.where(user_id: user_id).has_access.pluck(:access_level) + access += group.members.where(user_id: user_id).has_access.pluck(:access_level) end if project.invited_groups.any? && project.allowed_to_share_with_group? @@ -168,14 +168,14 @@ class ProjectTeam end def fetch_members(level = nil) - project_members = project.members.non_request - group_members = group ? group.members.non_request : [] + project_members = project.members + group_members = group ? group.members : [] invited_members = [] if project.invited_groups.any? && project.allowed_to_share_with_group? project.project_group_links.each do |group_link| invited_group = group_link.group - im = invited_group.members.non_request + im = invited_group.members if level int_level = GroupMember.access_level_roles[level.to_s.singularize.titleize] diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml index 50770465f07..522153b37e3 100644 --- a/app/views/admin/groups/show.html.haml +++ b/app/views/admin/groups/show.html.haml @@ -89,16 +89,16 @@ %hr = button_tag 'Add users to group', class: "btn btn-create" - = render 'shared/members/requests', membership_source: @group, members: @members.request + = render 'shared/members/requests', membership_source: @group, requesters: @requesters .panel.panel-default .panel-heading %strong= @group.name group members - %span.badge= @group.members.non_request.size + %span.badge= @group.members.size .pull-right = link_to icon('pencil-square-o', text: 'Manage Access'), polymorphic_url([@group, :members]), class: "btn btn-xs" %ul.well-list.group-users-list.content-list - = render partial: 'shared/members/member', collection: @members.non_request, as: :member, locals: { show_controls: false } + = render partial: 'shared/members/member', collection: @members, as: :member, locals: { show_controls: false } .panel-footer - = paginate @members.non_request, param_name: 'members_page', theme: 'gitlab' + = paginate @members, param_name: 'members_page', theme: 'gitlab' diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index 461d588415d..82d3169c6f9 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -137,16 +137,16 @@ .panel-heading %strong= @group.name group members - %span.badge= @group_members.non_request.size + %span.badge= @group_members.size .pull-right = link_to admin_group_path(@group), class: 'btn btn-xs' do = icon('pencil-square-o', text: 'Manage Access') %ul.well-list.content-list - = render partial: 'shared/members/member', collection: @group_members.non_request, as: :member, locals: { show_controls: false } + = render partial: 'shared/members/member', collection: @group_members, as: :member, locals: { show_controls: false } .panel-footer - = paginate @group_members.non_request, param_name: 'group_members_page', theme: 'gitlab' + = paginate @group_members, param_name: 'group_members_page', theme: 'gitlab' - = render 'shared/members/requests', membership_source: @project, members: @project_members.request + = render 'shared/members/requests', membership_source: @project, requesters: @requesters .panel.panel-default .panel-heading @@ -156,6 +156,6 @@ .pull-right = link_to icon('pencil-square-o', text: 'Manage Access'), polymorphic_url([@project, :members]), class: "btn btn-xs" %ul.well-list.project_members.content-list - = render partial: 'shared/members/member', collection: @project_members.non_request, as: :member, locals: { show_controls: false } + = render partial: 'shared/members/member', collection: @project_members, as: :member, locals: { show_controls: false } .panel-footer - = paginate @project_members.non_request, param_name: 'project_members_page', theme: 'gitlab' + = paginate @project_members, param_name: 'project_members_page', theme: 'gitlab' diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml index d6acade84f1..90f362c052b 100644 --- a/app/views/groups/group_members/index.html.haml +++ b/app/views/groups/group_members/index.html.haml @@ -1,7 +1,7 @@ - page_title "Members" .group-members-page.prepend-top-default - - if current_user && current_user.can?(:admin_group_member, @group) + - if can?(current_user, :admin_group_member, @group) .panel.panel-default .panel-heading Add new user to group @@ -11,13 +11,13 @@ .new-group-member-holder = render "new_group_member" - = render 'shared/members/requests', membership_source: @group, members: @members.request + = render 'shared/members/requests', membership_source: @group, requesters: @requesters .panel.panel-default .panel-heading %strong #{@group.name} group members - %span.badge= @members.non_request.size + %span.badge= @members.size .controls = form_tag group_group_members_path(@group), method: :get, class: 'form-inline member-search-form' do .form-group @@ -25,8 +25,8 @@ = button_tag class: 'btn', title: 'Search' do = icon("search") %ul.content-list - = render partial: 'shared/members/member', collection: @members.non_request, as: :member - = paginate @members.non_request, theme: 'gitlab' + = render partial: 'shared/members/member', collection: @members, as: :member + = paginate @members, theme: 'gitlab' :javascript $('form.member-search-form').on('submit', function(event) { diff --git a/app/views/layouts/nav/_group_settings.html.haml b/app/views/layouts/nav/_group_settings.html.haml index 3a24b09ab7e..bf9a7ecb786 100644 --- a/app/views/layouts/nav/_group_settings.html.haml +++ b/app/views/layouts/nav/_group_settings.html.haml @@ -1,6 +1,6 @@ - if current_user - can_edit = can?(current_user, :admin_group, @group) - - member = @group.members.non_request.find_by(user_id: current_user.id) + - member = @group.members.find_by(user_id: current_user.id) - can_leave = member && can?(current_user, :destroy_group_member, member) .controls diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index dcef427cda3..9e65d94186b 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -7,7 +7,7 @@ %ul.dropdown-menu.dropdown-menu-align-right - can_edit = can?(current_user, :admin_project, @project) -# We don't use @project.team.find_member because it searches for group members too... - - member = @project.members.non_request.find_by(user_id: current_user.id) + - member = @project.members.find_by(user_id: current_user.id) - can_leave = member && can?(current_user, :destroy_project_member, member) = render 'layouts/nav/project_settings', can_edit: can_edit diff --git a/app/views/projects/project_members/_shared_group_members.html.haml b/app/views/projects/project_members/_shared_group_members.html.haml index 840b57c2e63..77370c14def 100644 --- a/app/views/projects/project_members/_shared_group_members.html.haml +++ b/app/views/projects/project_members/_shared_group_members.html.haml @@ -1,6 +1,6 @@ - @project_group_links.each do |group_links| - shared_group = group_links.group - - shared_group_members = shared_group.members.non_request + - shared_group_members = shared_group.members - shared_group_users_count = shared_group_members.size .panel.panel-default .panel-heading diff --git a/app/views/projects/project_members/index.html.haml b/app/views/projects/project_members/index.html.haml index a2026c41d01..9031f01b496 100644 --- a/app/views/projects/project_members/index.html.haml +++ b/app/views/projects/project_members/index.html.haml @@ -13,12 +13,12 @@ Users with access to this project are listed below. = render "new_project_member" - = render 'shared/members/requests', membership_source: @project, members: @project_members.request + = render 'shared/members/requests', membership_source: @project, requesters: @requesters - = render 'team', members: @project_members.non_request + = render 'team', members: @project_members - if @group - = render "group_members", members: @group_members.non_request + = render "group_members", members: @group_members - if @project_group_links.any? && @project.allowed_to_share_with_group? = render "shared_group_members" diff --git a/app/views/shared/members/_access_request_buttons.html.haml b/app/views/shared/members/_access_request_buttons.html.haml index c56418f052a..35dcdccc921 100644 --- a/app/views/shared/members/_access_request_buttons.html.haml +++ b/app/views/shared/members/_access_request_buttons.html.haml @@ -1,13 +1,9 @@ -- member = source.members.find_by(user_id: current_user.id) -- group_member = source.group.members.find_by(user_id: current_user.id) if source.respond_to?(:group) && source.group - -- unless group_member - - if member - - if member.request? - = link_to 'Withdraw Access Request', polymorphic_path([:leave, source, :members]), - method: :delete, - data: { confirm: remove_member_message(member) }, - class: 'btn' +- if can_see_request_access_button?(source) + - if requester = source.requesters.find_by(user_id: current_user.id) + = link_to 'Withdraw Access Request', polymorphic_path([:leave, source, :members]), + method: :delete, + data: { confirm: remove_member_message(requester) }, + class: 'btn' - else = link_to 'Request Access', polymorphic_path([:request_access, source, :members]), method: :post, diff --git a/app/views/shared/members/_requests.html.haml b/app/views/shared/members/_requests.html.haml index e4bd2bdc265..40b39e850b0 100644 --- a/app/views/shared/members/_requests.html.haml +++ b/app/views/shared/members/_requests.html.haml @@ -1,8 +1,8 @@ -- if members.any? +- if requesters.any? .panel.panel-default .panel-heading %strong= membership_source.name access requests - %span.badge= members.size + %span.badge= requesters.size %ul.content-list - = render partial: 'shared/members/member', collection: members, as: :member + = render partial: 'shared/members/member', collection: requesters, as: :member diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index ddc54108a7b..c34475976c6 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -133,7 +133,7 @@ describe Groups::GroupMembersController do expect(response).to set_flash.to 'Your access request to the group has been withdrawn.' expect(response).to redirect_to(group_path(group)) - expect(group.members.request).to be_empty + expect(group.requesters).to be_empty expect(group.users).not_to include user end end @@ -153,7 +153,7 @@ describe Groups::GroupMembersController do expect(response).to set_flash.to 'Your request for access has been queued for review.' expect(response).to redirect_to(group_path(group)) - expect(group.members.request.exists?(user_id: user)).to be_truthy + expect(group.requesters.exists?(user_id: user)).to be_truthy expect(group.users).not_to include user end end @@ -175,7 +175,7 @@ describe Groups::GroupMembersController do let(:group_requester) { create(:user) } let(:member) do group.request_access(group_requester) - group.members.request.find_by(user_id: group_requester) + group.requesters.find_by(user_id: group_requester) end context 'when user does not have enough rights' do diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 29aaceb2302..5e2a8cf3849 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -187,7 +187,7 @@ describe Projects::ProjectMembersController do expect(response).to set_flash.to 'Your access request to the project has been withdrawn.' expect(response).to redirect_to(namespace_project_path(project.namespace, project)) - expect(project.members.request).to be_empty + expect(project.requesters).to be_empty expect(project.users).not_to include user end end @@ -210,7 +210,7 @@ describe Projects::ProjectMembersController do expect(response).to redirect_to( namespace_project_path(project.namespace, project) ) - expect(project.members.request.exists?(user_id: user)).to be_truthy + expect(project.requesters.exists?(user_id: user)).to be_truthy expect(project.users).not_to include user end end @@ -233,7 +233,7 @@ describe Projects::ProjectMembersController do let(:team_requester) { create(:user) } let(:member) do project.request_access(team_requester) - project.members.request.find_by(user_id: team_requester.id) + project.requesters.find_by(user_id: team_requester.id) end context 'when user does not have enough rights' do diff --git a/spec/features/groups/members/owner_manages_access_requests_spec.rb b/spec/features/groups/members/owner_manages_access_requests_spec.rb index 321c9bad7d0..2aae3e00261 100644 --- a/spec/features/groups/members/owner_manages_access_requests_spec.rb +++ b/spec/features/groups/members/owner_manages_access_requests_spec.rb @@ -41,7 +41,7 @@ feature 'Groups > Members > Owner manages access requests', feature: true do def expect_visible_access_request(group, user) - expect(group.members.request.exists?(user_id: user)).to be_truthy + expect(group.requesters.exists?(user_id: user)).to be_truthy expect(page).to have_content "#{group.name} access requests 1" expect(page).to have_content user.name end diff --git a/spec/features/groups/members/user_requests_access_spec.rb b/spec/features/groups/members/user_requests_access_spec.rb index 4944301c938..d1a6a98ab72 100644 --- a/spec/features/groups/members/user_requests_access_spec.rb +++ b/spec/features/groups/members/user_requests_access_spec.rb @@ -18,7 +18,7 @@ feature 'Groups > Members > User requests access', feature: true do expect(ActionMailer::Base.deliveries.last.to).to eq [owner.notification_email] expect(ActionMailer::Base.deliveries.last.subject).to match "Request to join the #{group.name} group" - expect(group.members.request.exists?(user_id: user)).to be_truthy + expect(group.requesters.exists?(user_id: user)).to be_truthy expect(page).to have_content 'Your request for access has been queued for review.' expect(page).to have_content 'Withdraw Access Request' @@ -42,7 +42,7 @@ feature 'Groups > Members > User requests access', feature: true do scenario 'user is not listed in the group members page' do click_link 'Request Access' - expect(group.members.request.exists?(user_id: user)).to be_truthy + expect(group.requesters.exists?(user_id: user)).to be_truthy click_link 'Members' @@ -54,11 +54,11 @@ feature 'Groups > Members > User requests access', feature: true do scenario 'user can withdraw its request for access' do click_link 'Request Access' - expect(group.members.request.exists?(user_id: user)).to be_truthy + expect(group.requesters.exists?(user_id: user)).to be_truthy click_link 'Withdraw Access Request' - expect(group.members.request.exists?(user_id: user)).to be_falsey + expect(group.requesters.exists?(user_id: user)).to be_falsey expect(page).to have_content 'Your access request to the group has been withdrawn.' end end diff --git a/spec/features/projects/members/master_manages_access_requests_spec.rb b/spec/features/projects/members/master_manages_access_requests_spec.rb index aa2d906fa2e..f7fcd9b6731 100644 --- a/spec/features/projects/members/master_manages_access_requests_spec.rb +++ b/spec/features/projects/members/master_manages_access_requests_spec.rb @@ -40,7 +40,7 @@ feature 'Projects > Members > Master manages access requests', feature: true do end def expect_visible_access_request(project, user) - expect(project.members.request.exists?(user_id: user)).to be_truthy + expect(project.requesters.exists?(user_id: user)).to be_truthy expect(page).to have_content "#{project.name} access requests 1" expect(page).to have_content user.name end diff --git a/spec/features/projects/members/user_requests_access_spec.rb b/spec/features/projects/members/user_requests_access_spec.rb index af420c170ef..f2fe3ef364d 100644 --- a/spec/features/projects/members/user_requests_access_spec.rb +++ b/spec/features/projects/members/user_requests_access_spec.rb @@ -17,7 +17,7 @@ feature 'Projects > Members > User requests access', feature: true do expect(ActionMailer::Base.deliveries.last.to).to eq [master.notification_email] expect(ActionMailer::Base.deliveries.last.subject).to eq "Request to join the #{project.name_with_namespace} project" - expect(project.members.request.exists?(user_id: user)).to be_truthy + expect(project.requesters.exists?(user_id: user)).to be_truthy expect(page).to have_content 'Your request for access has been queued for review.' expect(page).to have_content 'Withdraw Access Request' @@ -27,7 +27,7 @@ feature 'Projects > Members > User requests access', feature: true do scenario 'user is not listed in the project members page' do click_link 'Request Access' - expect(project.members.request.exists?(user_id: user)).to be_truthy + expect(project.requesters.exists?(user_id: user)).to be_truthy open_project_settings_menu click_link 'Members' @@ -41,11 +41,11 @@ feature 'Projects > Members > User requests access', feature: true do scenario 'user can withdraw its request for access' do click_link 'Request Access' - expect(project.members.request.exists?(user_id: user)).to be_truthy + expect(project.requesters.exists?(user_id: user)).to be_truthy click_link 'Withdraw Access Request' - expect(project.members.request.exists?(user_id: user)).to be_falsey + expect(project.requesters.exists?(user_id: user)).to be_falsey expect(page).to have_content 'Your access request to the project has been withdrawn.' end diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb index f75fdb739f6..7b2155e9a4e 100644 --- a/spec/helpers/members_helper_spec.rb +++ b/spec/helpers/members_helper_spec.rb @@ -57,6 +57,72 @@ describe MembersHelper do end end + describe '#can_see_request_access_button?' do + let(:user) { create(:user) } + let(:group) { create(:group, :public) } + let(:project) { create(:project, :public, group: group) } + + before do + allow(helper).to receive(:current_user).and_return(user) + end + + context 'source is a group' do + context 'current_user is not a member' do + it 'returns true' do + expect(helper.can_see_request_access_button?(group)).to be_truthy + end + end + + context 'current_user is a member' do + it 'returns false' do + group.add_owner(user) + + expect(helper.can_see_request_access_button?(group)).to be_falsy + end + end + + context 'current_user is a requester' do + it 'returns true' do + group.request_access(user) + + expect(helper.can_see_request_access_button?(group)).to be_truthy + end + end + end + + context 'source is a project' do + context 'current_user is not a member' do + it 'returns true' do + expect(helper.can_see_request_access_button?(project)).to be_truthy + end + end + + context 'current_user is a group member' do + it 'returns false' do + group.add_owner(user) + + expect(helper.can_see_request_access_button?(project)).to be_falsy + end + end + + context 'current_user is a group requester' do + it 'returns false' do + group.request_access(user) + + expect(helper.can_see_request_access_button?(project)).to be_falsy + end + end + + context 'current_user is a member' do + it 'returns false' do + project.team << [user, :master] + + expect(helper.can_see_request_access_button?(project)).to be_falsy + end + end + end + end + describe '#remove_member_message' do let(:requester) { build(:user) } let(:project) { create(:project) } diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index ae55a01ebea..e527d649e34 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -406,7 +406,7 @@ describe Notify do let(:user) { create(:user) } let(:project_member) do project.request_access(user) - project.members.request.find_by(user_id: user.id) + project.requesters.find_by(user_id: user.id) end subject { Notify.member_access_requested_email('project', project_member.id) } @@ -433,7 +433,7 @@ describe Notify do let(:user) { create(:user) } let(:project_member) do project.request_access(user) - project.members.request.find_by(user_id: user.id) + project.requesters.find_by(user_id: user.id) end subject { Notify.member_access_requested_email('project', project_member.id) } @@ -459,7 +459,7 @@ describe Notify do let(:user) { create(:user) } let(:project_member) do project.request_access(user) - project.members.request.find_by(user_id: user.id) + project.requesters.find_by(user_id: user.id) end subject { Notify.member_access_denied_email('project', project.id, user.id) } @@ -684,7 +684,7 @@ describe Notify do let(:user) { create(:user) } let(:group_member) do group.request_access(user) - group.members.request.find_by(user_id: user.id) + group.requesters.find_by(user_id: user.id) end subject { Notify.member_access_requested_email('group', group_member.id) } @@ -705,7 +705,7 @@ describe Notify do let(:user) { create(:user) } let(:group_member) do group.request_access(user) - group.members.request.find_by(user_id: user.id) + group.requesters.find_by(user_id: user.id) end subject { Notify.member_access_denied_email('group', group.id, user.id) } diff --git a/spec/models/concerns/access_requestable_spec.rb b/spec/models/concerns/access_requestable_spec.rb index 98307876962..96eee0e8bdd 100644 --- a/spec/models/concerns/access_requestable_spec.rb +++ b/spec/models/concerns/access_requestable_spec.rb @@ -16,7 +16,7 @@ describe AccessRequestable do before { group.request_access(user) } - it { expect(group.members.request.exists?(user_id: user)).to be_truthy } + it { expect(group.requesters.exists?(user_id: user)).to be_truthy } end end @@ -34,7 +34,7 @@ describe AccessRequestable do before { project.request_access(user) } - it { expect(project.members.request.exists?(user_id: user)).to be_truthy } + it { expect(project.requesters.exists?(user_id: user)).to be_truthy } end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 2c19aa3f67f..a878ff1b227 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -7,9 +7,38 @@ describe Group, models: true do it { is_expected.to have_many :projects } it { is_expected.to have_many(:group_members).dependent(:destroy) } it { is_expected.to have_many(:users).through(:group_members) } + it { is_expected.to have_many(:owners).through(:group_members) } + it { is_expected.to have_many(:requesters).dependent(:destroy) } it { is_expected.to have_many(:project_group_links).dependent(:destroy) } it { is_expected.to have_many(:shared_projects).through(:project_group_links) } it { is_expected.to have_many(:notification_settings).dependent(:destroy) } + + describe '#members & #requesters' do + let(:requester) { create(:user) } + let(:developer) { create(:user) } + before do + group.request_access(requester) + group.add_developer(developer) + end + + describe '#members' do + it 'includes members and exclude requesters' do + member_user_ids = group.members.pluck(:user_id) + + expect(member_user_ids).to include(developer.id) + expect(member_user_ids).not_to include(requester.id) + end + end + + describe '#requesters' do + it 'does not include requesters' do + requester_user_ids = group.requesters.pluck(:user_id) + + expect(requester_user_ids).to include(requester.id) + expect(requester_user_ids).not_to include(developer.id) + end + end + end end describe 'modules' do diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index e9134a3d283..40181a8b906 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -73,10 +73,10 @@ describe Member, models: true do @accepted_invite_member = project.members.invite.find_by_invite_email('toto2@example.com').tap { |u| u.accept_invite!(accepted_invite_user) } requested_user = create(:user).tap { |u| project.request_access(u) } - @requested_member = project.members.request.find_by(user_id: requested_user.id) + @requested_member = project.requesters.find_by(user_id: requested_user.id) accepted_request_user = create(:user).tap { |u| project.request_access(u) } - @accepted_request_member = project.members.request.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request } + @accepted_request_member = project.requesters.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request } end describe '.invite' do @@ -103,22 +103,6 @@ describe Member, models: true do it { expect(described_class.request).not_to include @accepted_request_member } end - describe '.non_request' do - it { expect(described_class.non_request).to include @master } - it { expect(described_class.non_request).to include @invited_member } - it { expect(described_class.non_request).to include @accepted_invite_member } - it { expect(described_class.non_request).not_to include @requested_member } - it { expect(described_class.non_request).to include @accepted_request_member } - end - - describe '.non_pending' do - it { expect(described_class.non_pending).to include @master } - it { expect(described_class.non_pending).not_to include @invited_member } - it { expect(described_class.non_pending).to include @accepted_invite_member } - it { expect(described_class.non_pending).not_to include @requested_member } - it { expect(described_class.non_pending).to include @accepted_request_member } - end - describe '.owners_and_masters' do it { expect(described_class.owners_and_masters).to include @owner } it { expect(described_class.owners_and_masters).to include @master } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 42308035d8c..a8c777d1e3e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -11,6 +11,8 @@ describe Project, models: true do it { is_expected.to have_many(:issues).dependent(:destroy) } it { is_expected.to have_many(:milestones).dependent(:destroy) } it { is_expected.to have_many(:project_members).dependent(:destroy) } + it { is_expected.to have_many(:users).through(:project_members) } + it { is_expected.to have_many(:requesters).dependent(:destroy) } it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:snippets).class_name('ProjectSnippet').dependent(:destroy) } it { is_expected.to have_many(:deploy_keys_projects).dependent(:destroy) } @@ -31,6 +33,34 @@ describe Project, models: true do it { is_expected.to have_many(:environments).dependent(:destroy) } it { is_expected.to have_many(:deployments).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } + + describe '#members & #requesters' do + let(:project) { create(:project) } + let(:requester) { create(:user) } + let(:developer) { create(:user) } + before do + project.request_access(requester) + project.team << [developer, :developer] + end + + describe '#members' do + it 'includes members and exclude requesters' do + member_user_ids = project.members.pluck(:user_id) + + expect(member_user_ids).to include(developer.id) + expect(member_user_ids).not_to include(requester.id) + end + end + + describe '#requesters' do + it 'does not include requesters' do + requester_user_ids = project.requesters.pluck(:user_id) + + expect(requester_user_ids).to include(requester.id) + expect(requester_user_ids).not_to include(developer.id) + end + end + end end describe 'modules' do |