diff --git a/app/models/member.rb b/app/models/member.rb index d92e69b2ce6..ec79f531740 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -26,16 +26,94 @@ class Member < ActiveRecord::Base belongs_to :user belongs_to :source, polymorphic: true - validates :user, presence: true + validates :user, presence: true, unless: :invite? validates :source, presence: true - validates :user_id, uniqueness: { scope: [:source_type, :source_id], message: "already exists in source" } + validates :user_id, uniqueness: { scope: [:source_type, :source_id], + message: "already exists in source", + allow_nil: true } validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true + validates :invite_email, presence: { if: :invite? }, + email: { strict_mode: true, allow_nil: true }, + uniqueness: { scope: [:source_type, :source_id], allow_nil: true } + scope :invite, -> { where(user_id: nil) } + scope :non_invite, -> { where("user_id IS NOT NULL") } scope :guests, -> { where(access_level: GUEST) } scope :reporters, -> { where(access_level: REPORTER) } scope :developers, -> { where(access_level: DEVELOPER) } scope :masters, -> { where(access_level: MASTER) } scope :owners, -> { where(access_level: OWNER) } + before_validation :generate_invite_token, on: :create, if: -> (member) { member.invite_email.present? } + after_create :send_invite, if: :invite? + after_create :post_create_hook, unless: :invite? + after_update :post_update_hook, unless: :invite? + after_destroy :post_destroy_hook, unless: :invite? + delegate :name, :username, :email, to: :user, prefix: true + + def invite? + self.invite_token.present? + end + + def accept_invite!(new_user) + self.invite_token = nil + self.invite_accepted_at = Time.now.utc + + self.user = new_user + + saved = self.save + + after_accept_invite if saved + + saved + end + + def generate_invite_token + raw, enc = Devise.token_generator.generate(self.class, :invite_token) + @raw_invite_token = raw + self.invite_token = enc + end + + def generate_invite_token! + generate_invite_token && save(validate: false) + end + + def resend_invite + return unless invite? + + generate_invite_token! unless @raw_invite_token + + send_invite + end + + private + + def send_invite + # override in subclass + end + + def post_create_hook + system_hook_service.execute_hooks_for(self, :create) + end + + def post_update_hook + # override in subclass + end + + def post_destroy_hook + system_hook_service.execute_hooks_for(self, :destroy) + end + + def after_accept_invite + post_create_hook + end + + def system_hook_service + SystemHooksService.new + end + + def notification_service + NotificationService.new + end end diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb index 28d0b4483b4..ccbbab6afc5 100644 --- a/app/models/members/group_member.rb +++ b/app/models/members/group_member.rb @@ -27,10 +27,6 @@ class GroupMember < Member scope :with_group, ->(group) { where(source_id: group.id) } scope :with_user, ->(user) { where(user_id: user.id) } - after_create :post_create_hook - after_update :notify_update - after_destroy :post_destroy_hook - def self.access_level_roles Gitlab::Access.options_with_owner end @@ -43,26 +39,19 @@ class GroupMember < Member access_level end + private + def post_create_hook notification_service.new_group_member(self) - system_hook_service.execute_hooks_for(self, :create) + + super end - def notify_update + def post_update_hook if access_level_changed? notification_service.update_group_member(self) end - end - def post_destroy_hook - system_hook_service.execute_hooks_for(self, :destroy) - end - - def system_hook_service - SystemHooksService.new - end - - def notification_service - NotificationService.new + super end end diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index 2205041cd51..6c5d161940d 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -27,10 +27,6 @@ class ProjectMember < Member validates_format_of :source_type, with: /\AProject\z/ default_scope { where(source_type: SOURCE_TYPE) } - after_create :post_create_hook - after_update :post_update_hook - after_destroy :post_destroy_hook - scope :in_project, ->(project) { where(source_id: project.id) } scope :in_projects, ->(projects) { where(source_id: projects.pluck(:id)) } scope :with_user, ->(user) { where(user_id: user.id) } @@ -110,41 +106,40 @@ class ProjectMember < Member access_level end + def project + source + end + def owner? project.owner == user end + private + def post_create_hook unless owner? event_service.join_project(self.project, self.user) notification_service.new_project_member(self) end - system_hook_service.execute_hooks_for(self, :create) + super end def post_update_hook - notification_service.update_project_member(self) if self.access_level_changed? + if access_level_changed? + notification_service.update_project_member(self) + end + + super end def post_destroy_hook event_service.leave_project(self.project, self.user) - system_hook_service.execute_hooks_for(self, :destroy) + + super end def event_service EventCreateService.new end - - def notification_service - NotificationService.new - end - - def system_hook_service - SystemHooksService.new - end - - def project - source - end end diff --git a/spec/models/members_spec.rb b/spec/models/members_spec.rb index dfd3f7feb6b..3f14a111443 100644 --- a/spec/models/members_spec.rb +++ b/spec/models/members_spec.rb @@ -11,10 +11,89 @@ describe Member do it { is_expected.to validate_presence_of(:user) } it { is_expected.to validate_presence_of(:source) } it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) } + + context "when an invite email is provided" do + + let(:member) { build(:project_member, invite_email: "user@example.com", user: nil) } + + it "doesn't require a user" do + expect(member).to be_valid + end + + it "requires a valid invite email" do + member.invite_email = "nope" + + expect(member).not_to be_valid + end + + it "requires a unique invite email scoped to this source" do + create(:project_member, source: member.source, invite_email: member.invite_email) + + expect(member).not_to be_valid + end + + it "is valid otherwise" do + expect(member).to be_valid + end + end + + context "when an invite email is not provided" do + + let(:member) { build(:project_member) } + + it "requires a user" do + member.user = nil + + expect(member).not_to be_valid + end + + it "is valid otherwise" do + expect(member).to be_valid + end + end end describe "Delegate methods" do it { is_expected.to respond_to(:user_name) } it { is_expected.to respond_to(:user_email) } end + + describe "#accept_invite!" do + + let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } + let(:user) { create(:user) } + + it "resets the invite token" do + member.accept_invite!(user) + + expect(member.invite_token).to be_nil + end + + it "sets the invite accepted timestamp" do + member.accept_invite!(user) + + expect(member.invite_accepted_at).not_to be_nil + end + + it "sets the user" do + member.accept_invite!(user) + + expect(member.user).to eq(user) + end + + it "calls #after_accept_invite" do + expect(member).to receive(:after_accept_invite) + + member.accept_invite!(user) + end + end + + describe "#generate_invite_token" do + + let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } + + it "sets the invite token" do + expect { member.generate_invite_token }.to change { member.invite_token} + end + end end