kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Ensure member notifications are sent after the member actual creation/update in the DB

Browse source 
Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit is contained in:
Rémy Coutable 2018-04-23 16:38:37 +02:00
parent 928e45e7d3
commit 8d381b359f
No known key found for this signature in database
GPG key ID: 98DFFD1C0C62B70B
10 changed files with 142 additions and 150 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
app/models/members/group_member.rb
View file

@ -37,20 +37,20 @@ class GroupMember < Member
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

6
app/models/members/project_member.rb
View file

@ -92,7 +92,7 @@ class ProjectMember < Member
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 @@ class ProjectMember < Member
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 @@ class ProjectMember < Member
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

5
changelogs/unreleased/45572-members-invitations-scheduled-before-commit.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Ensure member notifications are sent after the member actual creation/update in the DB
merge_request: 18538
author:
type: fixed

47
spec/features/groups/members/manage_access_requests_spec.rb
View file

@ -1,47 +0,0 @@
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

8
spec/features/groups/members/master_manages_access_requests_spec.rb Normal file
View file

@ -0,0 +1,8 @@
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

45
spec/features/projects/members/master_manages_access_requests_spec.rb
View file

@ -1,47 +1,8 @@
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

48
spec/models/members/group_member_spec.rb
View file

@ -28,52 +28,12 @@ describe GroupMember do
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

12
spec/models/members/project_member_spec.rb
View file

@ -123,15 +123,5 @@ describe ProjectMember do
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

52
spec/support/shared_examples/features/master_manages_access_requests_shared_example.rb Normal file
View file

@ -0,0 +1,52 @@
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

63
spec/support/shared_examples/models/members_notifications_shared_example.rb Normal file
View file

@ -0,0 +1,63 @@
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