Read about our upcoming Code of Conduct on this issue

Commit 542807e8 authored by Douwe Maan's avatar Douwe Maan
Browse files

Merge branch '45572-members-invitations-scheduled-before-commit' into 'master'

Resolve "Members invitations scheduled before commit"

Closes #45572

See merge request gitlab-org/gitlab-ce!18538
......@@ -37,20 +37,20 @@ def notifiable_options
private
def send_invite
notification_service.invite_group_member(self, @raw_invite_token)
run_after_commit_or_now { notification_service.invite_group_member(self, @raw_invite_token) }
super
end
def post_create_hook
notification_service.new_group_member(self)
run_after_commit_or_now { notification_service.new_group_member(self) }
super
end
def post_update_hook
if access_level_changed?
notification_service.update_group_member(self)
run_after_commit { notification_service.update_group_member(self) }
end
super
......
......@@ -92,7 +92,7 @@ def notifiable_options
private
def send_invite
notification_service.invite_project_member(self, @raw_invite_token)
run_after_commit_or_now { notification_service.invite_project_member(self, @raw_invite_token) }
super
end
......@@ -100,7 +100,7 @@ def send_invite
def post_create_hook
unless owner?
event_service.join_project(self.project, self.user)
notification_service.new_project_member(self)
run_after_commit_or_now { notification_service.new_project_member(self) }
end
super
......@@ -108,7 +108,7 @@ def post_create_hook
def post_update_hook
if access_level_changed?
notification_service.update_project_member(self)
run_after_commit { notification_service.update_project_member(self) }
end
super
......
---
title: Ensure member notifications are sent after the member actual creation/update in the DB
merge_request: 18538
author:
type: fixed
require 'spec_helper'
feature 'Groups > Members > Manage access requests' do
let(:user) { create(:user) }
let(:owner) { create(:user) }
let(:group) { create(:group, :public, :access_requestable) }
background do
group.request_access(user)
group.add_owner(owner)
sign_in(owner)
end
scenario 'owner can see access requests' do
visit group_group_members_path(group)
expect_visible_access_request(group, user)
end
scenario 'owner can grant access' do
visit group_group_members_path(group)
expect_visible_access_request(group, user)
perform_enqueued_jobs { click_on 'Grant access' }
expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{group.name} group was granted"
end
scenario 'owner can deny access' do
visit group_group_members_path(group)
expect_visible_access_request(group, user)
perform_enqueued_jobs { click_on 'Deny access' }
expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{group.name} group was denied"
end
def expect_visible_access_request(group, user)
expect(group.requesters.exists?(user_id: user)).to be_truthy
expect(page).to have_content "Users requesting access to #{group.name} 1"
expect(page).to have_content user.name
end
end
require 'spec_helper'
feature 'Groups > Members > Master manages access requests' do
it_behaves_like 'Master manages access requests' do
let(:entity) { create(:group, :public, :access_requestable) }
let(:members_page_path) { group_group_members_path(entity) }
end
end
require 'spec_helper'
feature 'Projects > Members > Master manages access requests' do
let(:user) { create(:user) }
let(:master) { create(:user) }
let(:project) { create(:project, :public, :access_requestable) }
background do
project.request_access(user)
project.add_master(master)
sign_in(master)
end
scenario 'master can see access requests' do
visit project_project_members_path(project)
expect_visible_access_request(project, user)
end
scenario 'master can grant access' do
visit project_project_members_path(project)
expect_visible_access_request(project, user)
perform_enqueued_jobs { click_on 'Grant access' }
expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{project.full_name} project was granted"
end
scenario 'master can deny access' do
visit project_project_members_path(project)
expect_visible_access_request(project, user)
perform_enqueued_jobs { click_on 'Deny access' }
expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{project.full_name} project was denied"
end
def expect_visible_access_request(project, user)
expect(project.requesters.exists?(user_id: user)).to be_truthy
expect(page).to have_content "Users requesting access to #{project.name} 1"
expect(page).to have_content user.name
it_behaves_like 'Master manages access requests' do
let(:entity) { create(:project, :public, :access_requestable) }
let(:members_page_path) { project_project_members_path(entity) }
end
end
......@@ -28,52 +28,12 @@
end
end
describe 'notifications' do
describe "#after_create" do
it "sends email to user" do
membership = build(:group_member)
it_behaves_like 'members notifications', :group
allow(membership).to receive(:notification_service)
.and_return(double('NotificationService').as_null_object)
expect(membership).to receive(:notification_service)
describe '#real_source_type' do
subject { create(:group_member).real_source_type }
membership.save
end
end
describe "#after_update" do
before do
@group_member = create :group_member
allow(@group_member).to receive(:notification_service)
.and_return(double('NotificationService').as_null_object)
end
it "sends email to user" do
expect(@group_member).to receive(:notification_service)
@group_member.update_attribute(:access_level, GroupMember::MASTER)
end
it "does not send an email when the access level has not changed" do
expect(@group_member).not_to receive(:notification_service)
@group_member.update_attribute(:access_level, GroupMember::OWNER)
end
end
describe '#after_accept_request' do
it 'calls NotificationService.accept_group_access_request' do
member = create(:group_member, user: build(:user), requested_at: Time.now)
expect_any_instance_of(NotificationService).to receive(:new_group_member)
member.__send__(:after_accept_request)
end
end
describe '#real_source_type' do
subject { create(:group_member).real_source_type }
it { is_expected.to eq 'Group' }
end
it { is_expected.to eq 'Group' }
end
describe '#update_two_factor_requirement' do
......
......@@ -123,15 +123,5 @@
it { expect(@project_2.users).to be_empty }
end
describe 'notifications' do
describe '#after_accept_request' do
it 'calls NotificationService.new_project_member' do
member = create(:project_member, user: create(:user), requested_at: Time.now)
expect_any_instance_of(NotificationService).to receive(:new_project_member)
member.__send__(:after_accept_request)
end
end
end
it_behaves_like 'members notifications', :project
end
RSpec.shared_examples 'Master manages access requests' do
let(:user) { create(:user) }
let(:master) { create(:user) }
before do
entity.request_access(user)
entity.respond_to?(:add_owner) ? entity.add_owner(master) : entity.add_master(master)
sign_in(master)
end
it 'master can see access requests' do
visit members_page_path
expect_visible_access_request(entity, user)
end
it 'master can grant access', :js do
visit members_page_path
expect_visible_access_request(entity, user)
accept_confirm { click_on 'Grant access' }
expect_no_visible_access_request(entity, user)
page.within('.members-list') do
expect(page).to have_content user.name
end
end
it 'master can deny access', :js do
visit members_page_path
expect_visible_access_request(entity, user)
accept_confirm { click_on 'Deny access' }
expect_no_visible_access_request(entity, user)
expect(page).not_to have_content user.name
end
def expect_visible_access_request(entity, user)
expect(entity.requesters.exists?(user_id: user)).to be_truthy
expect(page).to have_content "Users requesting access to #{entity.name} 1"
expect(page).to have_content user.name
end
def expect_no_visible_access_request(entity, user)
expect(entity.requesters.exists?(user_id: user)).to be_falsy
expect(page).not_to have_content "Users requesting access to #{entity.name}"
end
end
RSpec.shared_examples 'members notifications' do |entity_type|
let(:notification_service) { double('NotificationService').as_null_object }
before do
allow(member).to receive(:notification_service).and_return(notification_service)
end
describe "#after_create" do
let(:member) { build(:"#{entity_type}_member") }
it "sends email to user" do
expect(notification_service).to receive(:"new_#{entity_type}_member").with(member)
member.save
end
end
describe "#after_update" do
let(:member) { create(:"#{entity_type}_member", :developer) }
it "calls NotificationService.update_#{entity_type}_member" do
expect(notification_service).to receive(:"update_#{entity_type}_member").with(member)
member.update_attribute(:access_level, Member::MASTER)
end
it "does not send an email when the access level has not changed" do
expect(notification_service).not_to receive(:"update_#{entity_type}_member")
member.touch
end
end
describe '#accept_request' do
let(:member) { create(:"#{entity_type}_member", :access_request) }
it "calls NotificationService.new_#{entity_type}_member" do
expect(notification_service).to receive(:"new_#{entity_type}_member").with(member)
member.accept_request
end
end
describe "#accept_invite!" do
let(:member) { create(:"#{entity_type}_member", :invited) }
it "calls NotificationService.accept_#{entity_type}_invite" do
expect(notification_service).to receive(:"accept_#{entity_type}_invite").with(member)
member.accept_invite!(build(:user))
end
end
describe "#decline_invite!" do
let(:member) { create(:"#{entity_type}_member", :invited) }
it "calls NotificationService.decline_#{entity_type}_invite" do
expect(notification_service).to receive(:"decline_#{entity_type}_invite").with(member)
member.decline_invite!
end
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