From 4d7fa59af27409813cc72312b9b2a47da512b5ed Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Mon, 14 Jan 2019 22:53:37 +0100 Subject: Sent notification only to authorized users When moving a project, it's possible that some users who had access to the project in old path can not access the project in the new path. Because `project_authorizations` records are updated asynchronously, when we send the notification about moved project the list of project team members contains old project members, we want to notify all these members except the old users who can not access the new location. --- app/models/member.rb | 2 ++ app/models/project_team.rb | 12 ++++++++++++ app/services/notification_service.rb | 3 ++- 3 files changed, 16 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/models/member.rb b/app/models/member.rb index b0f049438eb..a0f755f88b5 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -84,6 +84,8 @@ class Member < ActiveRecord::Base scope :order_recent_sign_in, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.last_sign_in_at', 'DESC')) } scope :order_oldest_sign_in, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.last_sign_in_at', 'ASC')) } + scope :on_project_and_ancestors, ->(project) { where(source: [project] + project.ancestors) } + before_validation :generate_invite_token, on: :create, if: -> (member) { member.invite_email.present? } after_create :send_invite, if: :invite?, unless: :importing? diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 33bc6a561f9..aeba2843e5d 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -74,6 +74,14 @@ class ProjectTeam end alias_method :users, :members + # `members` method uses project_authorizations table which + # is updated asynchronously, on project move it still contains + # old members who may not have access to the new location, + # so we filter out only members of project or project's group + def members_in_project_and_ancestors + members.where(id: member_user_ids) + end + def guests @guests ||= fetch_members(Gitlab::Access::GUEST) end @@ -191,4 +199,8 @@ class ProjectTeam def group project.group end + + def member_user_ids + Member.on_project_and_ancestors(project).select(:user_id) + end end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index e1cf327209b..1a65561dd70 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -373,7 +373,8 @@ class NotificationService end def project_was_moved(project, old_path_with_namespace) - recipients = notifiable_users(project.team.members, :mention, project: project) + recipients = project.private? ? project.team.members_in_project_and_ancestors : project.team.members + recipients = notifiable_users(recipients, :mention, project: project) recipients.each do |recipient| mailer.project_was_moved_email( -- cgit v1.2.3