Commit 507e3fbc authored by Douwe Maan's avatar Douwe Maan

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
parents 0bbed2ec 8d381b35
...@@ -37,20 +37,20 @@ class GroupMember < Member ...@@ -37,20 +37,20 @@ class GroupMember < Member
private private
def send_invite 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 super
end end
def post_create_hook def post_create_hook
notification_service.new_group_member(self) run_after_commit_or_now { notification_service.new_group_member(self) }
super super
end end
def post_update_hook def post_update_hook
if access_level_changed? if access_level_changed?
notification_service.update_group_member(self) run_after_commit { notification_service.update_group_member(self) }
end end
super super
......
...@@ -92,7 +92,7 @@ class ProjectMember < Member ...@@ -92,7 +92,7 @@ class ProjectMember < Member
private private
def send_invite 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 super
end end
...@@ -100,7 +100,7 @@ class ProjectMember < Member ...@@ -100,7 +100,7 @@ class ProjectMember < Member
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)
notification_service.new_project_member(self) run_after_commit_or_now { notification_service.new_project_member(self) }
end end
super super
...@@ -108,7 +108,7 @@ class ProjectMember < Member ...@@ -108,7 +108,7 @@ class ProjectMember < Member
def post_update_hook def post_update_hook
if access_level_changed? if access_level_changed?
notification_service.update_project_member(self) run_after_commit { notification_service.update_project_member(self) }
end end
super 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' require 'spec_helper'
feature 'Projects > Members > Master manages access requests' do feature 'Projects > Members > Master manages access requests' do
let(:user) { create(:user) } it_behaves_like 'Master manages access requests' do
let(:master) { create(:user) } let(:entity) { create(:project, :public, :access_requestable) }
let(:project) { create(:project, :public, :access_requestable) } let(:members_page_path) { project_project_members_path(entity) }
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
end end
end end
...@@ -28,52 +28,12 @@ describe GroupMember do ...@@ -28,52 +28,12 @@ describe GroupMember do
end end
end end
describe 'notifications' do it_behaves_like 'members notifications', :group
describe "#after_create" do
it "sends email to user" do
membership = build(:group_member)
allow(membership).to receive(:notification_service) describe '#real_source_type' do
.and_return(double('NotificationService').as_null_object) subject { create(:group_member).real_source_type }
expect(membership).to receive(:notification_service)
membership.save it { is_expected.to eq 'Group' }
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
end end
describe '#update_two_factor_requirement' do describe '#update_two_factor_requirement' do
......
...@@ -123,15 +123,5 @@ describe ProjectMember do ...@@ -123,15 +123,5 @@ describe ProjectMember do
it { expect(@project_2.users).to be_empty } it { expect(@project_2.users).to be_empty }
end end
describe 'notifications' do it_behaves_like 'members notifications', :project
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
end 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