Commit 9af7d71b authored by Douwe Maan's avatar Douwe Maan Committed by Rémy Coutable

Merge branch '18755-fix-destroy-project-causes-post_decline_request-to-be-executed' into 'master'

Resolve "Destroying a project causes post_decline_request to be executed"

## What does this MR do?

Ensure we don't send "access request declined" to access requesters when a project is deleted.

## Are there points in the code the reviewer needs to double check?

I've created a service to decouple the notification sending from the AR model.

## Why was this MR needed?

Because there was an issue.

## What are the relevant issue numbers?

Fixes #18755, #18750.

## Does this MR meet the acceptance criteria?

- [x] No CHANGELOG needed.
- [x] Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !4744
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent 88ece310
...@@ -6,6 +6,7 @@ v 8.9.1 (unreleased) ...@@ -6,6 +6,7 @@ v 8.9.1 (unreleased)
- Document `GIT_STRATEGY` and `GIT_DEPTH`. !4720 - Document `GIT_STRATEGY` and `GIT_DEPTH`. !4720
- Add documentation for the export & import features. !4732 - Add documentation for the export & import features. !4732
- Add some docs for Docker Registry configuration. !4738 - Add some docs for Docker Registry configuration. !4738
- Ensure we don't send the "access request declined" email to access requesters on project deletion. !4744
- Fix MR-auto-close text added to description. !4836 - Fix MR-auto-close text added to description. !4836
- Fix typo in export failure email. !4847 - Fix typo in export failure email. !4847
......
...@@ -36,6 +36,10 @@ class ApplicationController < ActionController::Base ...@@ -36,6 +36,10 @@ class ApplicationController < ActionController::Base
render_404 render_404
end end
rescue_from Gitlab::Access::AccessDeniedError do |exception|
render_403
end
def redirect_back_or_default(default: root_path, options: {}) def redirect_back_or_default(default: root_path, options: {})
redirect_to request.referer.present? ? :back : default, options redirect_to request.referer.present? ? :back : default, options
end end
......
...@@ -21,29 +21,18 @@ module MembershipActions ...@@ -21,29 +21,18 @@ module MembershipActions
def leave def leave
@member = membershipable.members.find_by(user_id: current_user) @member = membershipable.members.find_by(user_id: current_user)
return render_403 unless @member Members::DestroyService.new(@member, current_user).execute
source_type = @member.real_source_type.humanize(capitalize: false) source_type = @member.real_source_type.humanize(capitalize: false)
notice =
if can?(current_user, action_member_permission(:destroy, @member), @member) if @member.request?
notice = "Your access request to the #{source_type} has been withdrawn."
if @member.request?
"Your access request to the #{source_type} has been withdrawn."
else
"You left the \"#{@member.source.human_name}\" #{source_type}."
end
@member.destroy
redirect_to [:dashboard, @member.real_source_type.tableize], notice: notice
else
if cannot_leave?
alert = "You can not leave the \"#{@member.source.human_name}\" #{source_type}."
alert << " Transfer or delete the #{source_type}."
redirect_to polymorphic_url(membershipable), alert: alert
else else
render_403 "You left the \"#{@member.source.human_name}\" #{source_type}."
end end
end redirect_path = @member.request? ? @member.source : [:dashboard, @member.real_source_type.tableize]
redirect_to redirect_path, notice: notice
end end
protected protected
...@@ -51,8 +40,4 @@ module MembershipActions ...@@ -51,8 +40,4 @@ module MembershipActions
def membershipable def membershipable
raise NotImplementedError raise NotImplementedError
end end
def cannot_leave?
raise NotImplementedError
end
end end
...@@ -36,9 +36,7 @@ class Groups::GroupMembersController < Groups::ApplicationController ...@@ -36,9 +36,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
def destroy def destroy
@group_member = @group.group_members.find(params[:id]) @group_member = @group.group_members.find(params[:id])
return render_403 unless can?(current_user, :destroy_group_member, @group_member) Members::DestroyService.new(@group_member, current_user).execute
@group_member.destroy
respond_to do |format| respond_to do |format|
format.html { redirect_to group_group_members_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.' }
...@@ -68,8 +66,4 @@ class Groups::GroupMembersController < Groups::ApplicationController ...@@ -68,8 +66,4 @@ class Groups::GroupMembersController < Groups::ApplicationController
# MembershipActions concern # MembershipActions concern
alias_method :membershipable, :group alias_method :membershipable, :group
def cannot_leave?
@group.last_owner?(current_user)
end
end end
...@@ -50,9 +50,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController ...@@ -50,9 +50,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
def destroy def destroy
@project_member = @project.project_members.find(params[:id]) @project_member = @project.project_members.find(params[:id])
return render_403 unless can?(current_user, :destroy_project_member, @project_member) Members::DestroyService.new(@project_member, current_user).execute
@project_member.destroy
respond_to do |format| respond_to do |format|
format.html do format.html do
...@@ -98,8 +96,4 @@ class Projects::ProjectMembersController < Projects::ApplicationController ...@@ -98,8 +96,4 @@ class Projects::ProjectMembersController < Projects::ApplicationController
# MembershipActions concern # MembershipActions concern
alias_method :membershipable, :project alias_method :membershipable, :project
def cannot_leave?
current_user == @project.owner
end
end end
...@@ -48,7 +48,6 @@ class Member < ActiveRecord::Base ...@@ -48,7 +48,6 @@ class Member < ActiveRecord::Base
after_create :post_create_hook, unless: [:pending?, :importing?] after_create :post_create_hook, unless: [:pending?, :importing?]
after_update :post_update_hook, unless: [:pending?, :importing?] after_update :post_update_hook, unless: [:pending?, :importing?]
after_destroy :post_destroy_hook, unless: :pending? after_destroy :post_destroy_hook, unless: :pending?
after_destroy :post_decline_request, if: :request?
delegate :name, :username, :email, to: :user, prefix: true delegate :name, :username, :email, to: :user, prefix: true
...@@ -188,7 +187,7 @@ class Member < ActiveRecord::Base ...@@ -188,7 +187,7 @@ class Member < ActiveRecord::Base
end end
def send_request def send_request
# override in subclass notification_service.new_access_request(self)
end end
def post_create_hook def post_create_hook
...@@ -215,10 +214,6 @@ class Member < ActiveRecord::Base ...@@ -215,10 +214,6 @@ class Member < ActiveRecord::Base
post_create_hook post_create_hook
end end
def post_decline_request
# override in subclass
end
def system_hook_service def system_hook_service
SystemHooksService.new SystemHooksService.new
end end
......
...@@ -33,12 +33,6 @@ class GroupMember < Member ...@@ -33,12 +33,6 @@ class GroupMember < Member
super super
end end
def send_request
notification_service.new_group_access_request(self)
super
end
def post_create_hook def post_create_hook
notification_service.new_group_member(self) notification_service.new_group_member(self)
...@@ -64,10 +58,4 @@ class GroupMember < Member ...@@ -64,10 +58,4 @@ class GroupMember < Member
super super
end end
def post_decline_request
notification_service.decline_group_access_request(self)
super
end
end end
...@@ -111,12 +111,6 @@ class ProjectMember < Member ...@@ -111,12 +111,6 @@ class ProjectMember < Member
super super
end end
def send_request
notification_service.new_project_access_request(self)
super
end
def post_create_hook def post_create_hook
unless owner? unless owner?
event_service.join_project(self.project, self.user) event_service.join_project(self.project, self.user)
...@@ -152,12 +146,6 @@ class ProjectMember < Member ...@@ -152,12 +146,6 @@ class ProjectMember < Member
super super
end end
def post_decline_request
notification_service.decline_project_access_request(self)
super
end
def event_service def event_service
EventCreateService.new EventCreateService.new
end end
......
module Members
class DestroyService < BaseService
attr_accessor :member, :current_user
def initialize(member, user)
@member, @current_user = member, user
end
def execute
unless member && can?(current_user, "destroy_#{member.type.underscore}".to_sym, member)
raise Gitlab::Access::AccessDeniedError
end
member.destroy
if member.request? && member.user != current_user
notification_service.decline_access_request(member)
end
end
end
end
...@@ -181,15 +181,16 @@ class NotificationService ...@@ -181,15 +181,16 @@ class NotificationService
end end
end end
# Project access request # Members
def new_project_access_request(project_member) def new_access_request(member)
mailer.member_access_requested_email(project_member.real_source_type, project_member.id).deliver_later mailer.member_access_requested_email(member.real_source_type, member.id).deliver_later
end end
def decline_project_access_request(project_member) def decline_access_request(member)
mailer.member_access_denied_email(project_member.real_source_type, project_member.project.id, project_member.user.id).deliver_later mailer.member_access_denied_email(member.real_source_type, member.source_id, member.user_id).deliver_later
end end
# Project invite
def invite_project_member(project_member, token) def invite_project_member(project_member, token)
mailer.member_invited_email(project_member.real_source_type, project_member.id, token).deliver_later mailer.member_invited_email(project_member.real_source_type, project_member.id, token).deliver_later
end end
...@@ -216,15 +217,7 @@ class NotificationService ...@@ -216,15 +217,7 @@ class NotificationService
mailer.member_access_granted_email(project_member.real_source_type, project_member.id).deliver_later mailer.member_access_granted_email(project_member.real_source_type, project_member.id).deliver_later
end end
# Group access request # Group invite
def new_group_access_request(group_member)
mailer.member_access_requested_email(group_member.real_source_type, group_member.id).deliver_later
end
def decline_group_access_request(group_member)
mailer.member_access_denied_email(group_member.real_source_type, group_member.group.id, group_member.user.id).deliver_later
end
def invite_group_member(group_member, token) def invite_group_member(group_member, token)
mailer.member_invited_email(group_member.real_source_type, group_member.id, token).deliver_later mailer.member_invited_email(group_member.real_source_type, group_member.id, token).deliver_later
end end
......
- if current_user - if current_user
- if access = @group.users.find_by(id: current_user.id) - can_edit = can?(current_user, :admin_group, @group)
.controls - member = @group.members.non_request.find_by(user_id: current_user.id)
.dropdown.group-settings-dropdown - can_leave = member && can?(current_user, :destroy_group_member, member)
%a.dropdown-new.btn.btn-default#group-settings-button{href: '#', 'data-toggle' => 'dropdown'}
= icon('cog') .controls
= icon('caret-down') .dropdown.group-settings-dropdown
%ul.dropdown-menu.dropdown-menu-align-right %a.dropdown-new.btn.btn-default#group-settings-button{href: '#', 'data-toggle' => 'dropdown'}
- if can?(current_user, :admin_group, @group) = icon('cog')
= nav_link(path: 'groups#projects') do = icon('caret-down')
= link_to projects_group_path(@group), title: 'Projects' do %ul.dropdown-menu.dropdown-menu-align-right
Projects = nav_link(path: 'groups#projects') do
%li.divider = link_to 'Projects', projects_group_path(@group), title: 'Projects'
%li %li.divider
= link_to edit_group_path(@group) do - if can_edit
Edit Group %li
= link_to 'Edit Group', edit_group_path(@group)
- if can_leave
%li
= link_to polymorphic_path([:leave, @group, :members]),
data: { confirm: leave_confirmation_message(@group) }, method: :delete, title: 'Leave group' do
Leave Group
...@@ -5,19 +5,20 @@ ...@@ -5,19 +5,20 @@
= icon('cog') = icon('cog')
= icon('caret-down') = icon('caret-down')
%ul.dropdown-menu.dropdown-menu-align-right %ul.dropdown-menu.dropdown-menu-align-right
- is_project_member = @project.users.exists?(current_user.id)
- access = @project.team.max_member_access(current_user.id)
- can_edit = can?(current_user, :admin_project, @project) - 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)
- can_leave = member && can?(current_user, :destroy_project_member, member)
= render 'layouts/nav/project_settings', access: access, can_edit: can_edit = render 'layouts/nav/project_settings', can_edit: can_edit
- if can_edit || is_project_member - if can_edit || can_leave
%li.divider %li.divider
- if can_edit - if can_edit
%li %li
= link_to edit_project_path(@project) do = link_to edit_project_path(@project) do
Edit Project Edit Project
- if is_project_member - if can_leave
%li %li
= link_to polymorphic_path([:leave, @project, :members]), = link_to polymorphic_path([:leave, @project, :members]),
data: { confirm: leave_confirmation_message(@project) }, method: :delete, title: 'Leave project' do data: { confirm: leave_confirmation_message(@project) }, method: :delete, title: 'Leave project' do
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
= link_to namespace_project_project_members_path(@project.namespace, @project), title: 'Members', class: 'team-tab tab' do = link_to namespace_project_project_members_path(@project.namespace, @project), title: 'Members', class: 'team-tab tab' do
%span %span
Members Members
- if access && can_edit - if can_edit
- if @project.allowed_to_share_with_group? - if @project.allowed_to_share_with_group?
= nav_link(controller: :group_links) do = nav_link(controller: :group_links) do
= link_to namespace_project_group_links_path(@project.namespace, @project), title: "Groups" do = link_to namespace_project_group_links_path(@project.namespace, @project), title: "Groups" do
......
...@@ -5,53 +5,9 @@ Feature: Dashboard Group ...@@ -5,53 +5,9 @@ Feature: Dashboard Group
And "John Doe" is owner of group "Owned" And "John Doe" is owner of group "Owned"
And "John Doe" is guest of group "Guest" And "John Doe" is guest of group "Guest"
# Leave groups
@javascript
Scenario: Owner should be able to leave from group if he is not the last owner
Given "Mary Jane" is owner of group "Owned"
When I visit dashboard groups page
Then I should see group "Owned" in group list
Then I should see group "Guest" in group list
When I click on the "Leave" button for group "Owned"
And I visit dashboard groups page
Then I should not see group "Owned" in group list
Then I should see group "Guest" in group list
@javascript
Scenario: Owner should not be able to leave from group if he is the last owner
Given "Mary Jane" is guest of group "Owned"
When I visit dashboard groups page
Then I should see group "Owned" in group list
Then I should see group "Guest" in group list
When I click on the "Leave" button for group "Owned"
Then I should see the "Can not leave message"
@javascript
Scenario: Guest should be able to leave from group
Given "Mary Jane" is guest of group "Guest"
When I visit dashboard groups page
Then I should see group "Owned" in group list
Then I should see group "Guest" in group list
When I click on the "Leave" button for group "Guest"
When I visit dashboard groups page
Then I should see group "Owned" in group list
Then I should not see group "Guest" in group list
@javascript
Scenario: Guest should be able to leave from group even if he is the only user in the group
When I visit dashboard groups page
Then I should see group "Owned" in group list
Then I should see group "Guest" in group list
When I click on the "Leave" button for group "Guest"
When I visit dashboard groups page
Then I should see group "Owned" in group list
Then I should not see group "Guest" in group list
Scenario: Create a group from dasboard Scenario: Create a group from dasboard
And I visit dashboard groups page And I visit dashboard groups page
And I click new group link And I click new group link
And submit form with new group "Samurai" info And submit form with new group "Samurai" info
Then I should be redirected to group "Samurai" page Then I should be redirected to group "Samurai" page
And I should see newly created group "Samurai" And I should see newly created group "Samurai"
...@@ -4,44 +4,6 @@ class Spinach::Features::DashboardGroup < Spinach::FeatureSteps ...@@ -4,44 +4,6 @@ class Spinach::Features::DashboardGroup < Spinach::FeatureSteps
include SharedPaths include SharedPaths
include SharedUser include SharedUser
# Leave
step 'I click on the "Leave" button for group "Owned"' do
find(:css, 'li', text: "Owner").find(:css, 'i.fa.fa-sign-out').click
# poltergeist always confirms popups.
end
step 'I click on the "Leave" button for group "Guest"' do
find(:css, 'li', text: "Guest").find(:css, 'i.fa.fa-sign-out').click
# poltergeist always confirms popups.
end
step 'I should not see the "Leave" button for group "Owned"' do
expect(find(:css, 'li', text: "Owner")).not_to have_selector(:css, 'i.fa.fa-sign-out')
# poltergeist always confirms popups.
end
step 'I should not see the "Leave" button for groupr "Guest"' do
expect(find(:css, 'li', text: "Guest")).not_to have_selector(:css, 'i.fa.fa-sign-out')
# poltergeist always confirms popups.
end
step 'I should see group "Owned" in group list' do
expect(page).to have_content("Owned")
end
step 'I should not see group "Owned" in group list' do
expect(page).not_to have_content("Owned")
end
step 'I should see group "Guest" in group list' do
expect(page).to have_content("Guest")
end
step 'I should not see group "Guest" in group list' do
expect(page).not_to have_content("Guest")
end
step 'I click new group link' do step 'I click new group link' do
click_link "New Group" click_link "New Group"
end end
...@@ -60,8 +22,4 @@ class Spinach::Features::DashboardGroup < Spinach::FeatureSteps ...@@ -60,8 +22,4 @@ class Spinach::Features::DashboardGroup < Spinach::FeatureSteps
expect(page).to have_content "Samurai" expect(page).to have_content "Samurai"
expect(page).to have_content "Tokugawa Shogunate" expect(page).to have_content "Tokugawa Shogunate"
end end
step 'I should see the "Can not leave message"' do
expect(page).to have_content "You can not leave the \"Owned\" group."
end
end end
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
# #
module Gitlab module Gitlab
module Access module Access
class AccessDeniedError < StandardError; end
GUEST = 10 GUEST = 10
REPORTER = 20 REPORTER = 20
DEVELOPER = 30 DEVELOPER = 30
......
...@@ -118,9 +118,7 @@ describe Groups::GroupMembersController do ...@@ -118,9 +118,7 @@ describe Groups::GroupMembersController do
it 'cannot removes himself from the group' do it 'cannot removes himself from the group' do
delete :leave, group_id: group delete :leave, group_id: group
expect(response).to redirect_to(group_path(group)) expect(response.status).to eq(403)
expect(response).to set_flash[:alert].to "You can not leave the \"#{group.name}\" group. Transfer or delete the group."
expect(group.users).to include user
end end
end end
...@@ -134,7 +132,7 @@ describe Groups::GroupMembersController do ...@@ -134,7 +132,7 @@ describe Groups::GroupMembersController do
delete :leave, group_id: group delete :leave, group_id: group
expect(response).to set_flash.to 'Your access request to the group has been withdrawn.' expect(response).to set_flash.to 'Your access request to the group has been withdrawn.'
expect(response).to redirect_to(dashboard_groups_path) expect(response).to redirect_to(group_path(group))
expect(group.members.request).to be_empty expect(group.members.request).to be_empty
expect(group.users).not_to include user expect(group.users).not_to include user
end end
......
...@@ -171,11 +171,7 @@ describe Projects::ProjectMembersController do ...@@ -171,11 +171,7 @@ describe Projects::ProjectMembersController do
delete :leave, namespace_id: project.namespace, delete :leave, namespace_id: project.namespace,
project_id: project project_id: project
expect(response).to redirect_to( expect(response.status).to eq(403)
namespace_project_path(project.namespace, project)
)
expect(response).to set_flash[:alert].to "You can not leave the \"#{project.human_name}\" project. Transfer or delete the project."
expect(project.users).to include user
end end
end end
...@@ -190,7 +186,7 @@ describe Projects::ProjectMembersController do ...@@ -190,7 +186,7 @@ describe Projects::ProjectMembersController do
project_id: project project_id: project
expect(response).to set_flash.to 'Your access request to the project has been withdrawn.' expect(response).to set_flash.to 'Your access request to the project has been withdrawn.'
expect(response).to redirect_to(dashboard_projects_path) expect(response).to redirect_to(namespace_project_path(project.namespace, project))
expect(project.members.request).to be_empty expect(project.members.request).to be_empty
expect(project.users).not_to include user expect(project.users).not_to include user
end end
......
require 'spec_helper'
feature 'Groups > Members > Last owner cannot leave group', feature: true do
let(:owner) { create(:user) }
let(:group) { create(:group) }
background do
group.add_owner(owner)
login_as(owner)
visit group_path(group)
end
scenario 'user does not see a "Leave Group" link' do
expect(page).not_to have_content 'Leave Group'
end
end
require 'spec_helper'
feature 'Groups > Members > Member leaves group', feature: true do
let(:user) { create(:user) }
let(:owner) { create(:user) }
let(:group) { create(:group, :public) }
background do
group.add_owner(owner)
group.add_developer(user)
login_as(user)
visit group_path(group)
end
scenario 'user leaves group' do
click_link 'Leave Group'
expect(current_path).to eq(dashboard_groups_path)
expect(group.users.exists?(user.id)).to be_falsey
end
end
...@@ -21,6 +21,7 @@ feature 'Groups > Members > User requests access', feature: true do ...@@ -21,6 +21,7 @@ feature 'Groups > Members > User requests access', feature: true do
expect(page).to have_content 'Your request for access has been queued for review.' expect(page).to have_content 'Your request for access has been queued for review.'
expect(page).to have_content 'Withdraw Access Request' expect(page).to have_content 'Withdraw Access Request'
expect(page).not_to have_content 'Leave Group'
end end
scenario 'user is not listed in the group members page' do scenario 'user is not listed in the group members page' do
......
require 'spec_helper'
feature 'Projects > Members > Member leaves project', feature: true do
let(:user) { create(:user) }
let(:project) { create(:project) }
background do
project.team << [user, :developer]
login_as(user)
visit namespace_project_path(project.namespace, project)
end
scenario 'user leaves project' do
click_link 'Leave Project'
expect(current_path).to eq(dashboard_projects_path)
expect(project.users.exists?(user.id)).to be_falsey
end
end
require 'spec_helper'
feature 'Projects > Members > Owner cannot leave project', feature: true do
let(:owner) { create(:user) }
let(:project) { create(:project) }
background do
project.team << [owner, :owner]
login_as(owner)
visit namespace_project_path(project.namespace, project)
end
scenario 'user does not see a "Leave Project" link' do
expect(page).not_to have_content 'Leave Project'
end
end
...@@ -21,6 +21,7 @@ feature 'Projects > Members > User requests access', feature: true do ...@@ -21,6 +21,7 @@ feature 'Projects > Members > User requests access', feature: true do
expect(page).to have_content 'Your request for access has been queued for review.' expect(page).to have_content 'Your request for access has been queued for review.'
expect(page).to have_content 'Withdraw Access Request' expect(page).to have_content 'Withdraw Access Request'
expect(page).not_to have_content 'Leave Project'
end end
scenario 'user is not listed in the project members page' do scenario 'user is not listed in the project members page' do
......
...@@ -70,22 +70,6 @@ feature 'Project', feature: true do ...@@ -70,22 +70,6 @@ feature 'Project', feature: true do
end end
end end
describe 'leave project link' do
let(:user) { create(:user) }
let(:project) { create(:project, namespace: user.namespace) }
before do
login_with(user)
project.team.add_user(user, Gitlab::Access::MASTER)
visit namespace_project_path(project.namespace, project)
end
it 'click project-settings and find leave project' do
find('#project-settings-button').click
expect(page).to have_link('Leave Project')
end
end
describe 'project title' do describe 'project title' do
include WaitForAjax include WaitForAjax
......
...@@ -134,18 +134,6 @@ describe Member, models: true do ...@@ -134,18 +134,6 @@ describe Member, models: true do
it { is_expected.to respond_to(:user_email) } it { is_expected.to respond_to(:user_email) }
end end
describe 'Callbacks' do
describe 'after_destroy :post_decline_request, if: :request?' do
let(:member) { create(:project_member, requested_at: Time.now.utc) }
it 'calls #post_decline_request' do
expect(member).to receive(:post_decline_request)
member.destroy
end
end
end
describe ".add_user" do describe ".add_user" do
let!(:user) { create(:user) } let!(:user) { create(:user) }
let(:project) { create(:project) } let(:project) { create(:project) }
......
...@@ -61,16 +61,6 @@ describe GroupMember, models: true do ...@@ -61,16 +61,6 @@ describe GroupMember, models: true do
end end
end end
describe '#post_decline_request' do
it 'calls NotificationService.decline_group_access_request' do
member = create(:group_member, user: build_stubbed(:user), requested_at: Time.now)
expect_any_instance_of(NotificationService).to receive(:decline_group_access_request)
member.__send__(:post_decline_request)
end
end
describe '#real_source_type' do describe '#real_source_type' do
subject { create(:group_member).real_source_type } subject { create(:group_member).real_source_type }
......
...@@ -152,15 +152,5 @@ describe ProjectMember, models: true do ...@@ -152,15 +152,5 @@ describe ProjectMember, models: true do
member.__send__(:after_accept_request) member.__send__(:after_accept_request)
end end
end end
describe '#post_decline_request' do
it 'calls NotificationService.decline_project_access_request' do
member = create(:project_member, user: build_stubbed(:user), requested_at: Time.now)
expect_any_instance_of(NotificationService).to receive(:decline_project_access_request)
member.__send__(:post_decline_request)
end
end
end end
end end
require 'spec_helper'
describe Members::DestroyService, services: true do
let(:user) { create(:user) }
let(:project) { create(:project) }
let!(:member) { create(:project_member, source: project) }
context 'when member is nil' do
before do
project.team << [user, :developer]
end
it 'does not destroy the member' do
expect { destroy_member(nil, user) }.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
context 'when current user cannot destroy the given member' do
before do
project.team << [user, :developer]
end
it 'does not destroy the member' do
expect { destroy_member(member, user) }.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
context 'when current user can destroy the given member' do
before do
project.team << [user, :master]
end
it 'destroys the member' do
destroy_member(member, user)
expect(member).to be_destroyed
end
context 'when the given member is a requester' do
before do
member.update_column(:requested_at, Time.now)
end
it 'calls Member#after_decline_request' do
expect_any_instance_of(NotificationService).to receive(:decline_access_request).with(member)
destroy_member(member, user)
end
context 'when current user is the member' do
it 'does not call Member#after_decline_request' do
expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member)
destroy_member(member, member.user)
end
end
context 'when current user is the member and ' do
it 'does not call Member#after_decline_request' do
expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member)
destroy_member(member, member.user)
end
end
end
end
def destroy_member(member, user)
Members::DestroyService.new(member, user).execute
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment