diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 9831a2089be3ec5f504d8be1c70faa68d96a2563..3da44cbc9d6edd6df5bbc2c1951c8d84ec401e85 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -52,8 +52,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController respond_to do |format| format.html do - redirect_to namespace_project_project_members_path(@project.namespace, - @project) + redirect_to namespace_project_project_members_path(@project.namespace, @project) end format.js { render nothing: true } end diff --git a/app/models/group.rb b/app/models/group.rb index 2de397f90f71421386d9dd30946db96954ad5178..1386a9eccc9dece0ba96dd65c164c42a523443af 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -47,25 +47,8 @@ class Group < Namespace end def add_users(user_ids, access_level, current_user = nil) - users = user_ids.map do |user_id| - (user_id if user_id.is_a?(User)) || - User.find_by(id: user_id) || - User.find_by(email: user_id) || - user_id - end - - users.compact.each do |user| - if user.is_a?(User) - member = self.group_members.find_or_initialize_by(user_id: user.id) - else - member = self.group_members.build - member.invite_email = user - end - - member.created_by ||= current_user - member.access_level = access_level - - member.save + user_ids.each do |user_id| + Member.add_user(self.group_members, user_id, access_level, current_user) end end diff --git a/app/models/member.rb b/app/models/member.rb index 2421222eaa2ec42e40a2359a2e4a811e17799459..9340c4421725d93dea7e80508b037e8c00251dbd 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -52,9 +52,39 @@ class Member < ActiveRecord::Base delegate :name, :username, :email, to: :user, prefix: true - def self.find_by_invite_token(invite_token) - invite_token = Devise.token_generator.digest(self, :invite_token, invite_token) - find_by(invite_token: invite_token) + class << self + def find_by_invite_token(invite_token) + invite_token = Devise.token_generator.digest(self, :invite_token, invite_token) + find_by(invite_token: invite_token) + end + + # This method is used to find users that have been entered into the "Add members" field. + # These can be the User objects directly, their IDs, their emails, or new emails to be invited. + def user_for_id(user_id) + return user_id if user_id.is_a?(User) + + user = User.find_by(id: user_id) + user ||= User.find_by(email: user_id) + user ||= user_id + user + end + + def add_user(members, user_id, access_level, current_user = nil) + user = user_for_id(user_id) + + # `user` can be either a User object or an email to be invited + if user.is_a?(User) + member = members.find_or_initialize_by(user_id: user.id) + else + member = members.build + member.invite_email = user + end + + member.created_by ||= current_user + member.access_level = access_level + + member.save + end end def invite? diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index 8af7499dd828b8ece4e704b3b5d4304696c83567..ce704a60d634e99e76b843eba632648a6cfa19ef 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -60,29 +60,14 @@ class ProjectMember < Member raise "Non valid access" end - users = user_ids.map do |user_id| - (user_id if user_id.is_a?(User)) || - User.find_by(id: user_id) || - User.find_by(email: user_id) || - user_id - end + users = user_ids.map { |user_id| Member.user_for_id(user_id) } ProjectMember.transaction do project_ids.each do |project_id| project = Project.find(project_id) users.each do |user| - if user.is_a?(User) - member = project.project_members.find_or_initialize_by(user_id: user.id) - else - member = project.project_members.build - member.invite_email = user - end - - member.created_by ||= current_user - member.access_level = access_level - - member.save + Member.add_user(project.project_members, user, access_level, current_user) end end end diff --git a/spec/models/members_spec.rb b/spec/models/member_spec.rb similarity index 71% rename from spec/models/members_spec.rb rename to spec/models/member_spec.rb index c3e47ab7e9aa198f8c90b80e65def2499a96974b..a27931cd4e4d7a6d87bd72198d7a72bb7a972bcc 100644 --- a/spec/models/members_spec.rb +++ b/spec/models/member_spec.rb @@ -58,6 +58,43 @@ describe Member do it { is_expected.to respond_to(:user_email) } end + describe ".add_user" do + let!(:user) { create(:user) } + let(:project) { create(:project) } + + context "when called with a user id" do + it "adds the user as a member" do + Member.add_user(project.project_members, user.id, ProjectMember::MASTER) + + expect(project.users).to include(user) + end + end + + context "when called with a user object" do + it "adds the user as a member" do + Member.add_user(project.project_members, user, ProjectMember::MASTER) + + expect(project.users).to include(user) + end + end + + context "when called with a known user email" do + it "adds the user as a member" do + Member.add_user(project.project_members, user.email, ProjectMember::MASTER) + + expect(project.users).to include(user) + end + end + + context "when called with an unknown user email" do + it "adds a member invite" do + Member.add_user(project.project_members, "user@example.com", ProjectMember::MASTER) + + expect(project.project_members.invite.pluck(:invite_email)).to include("user@example.com") + end + end + end + describe "#accept_invite!" do let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } @@ -89,7 +126,6 @@ describe Member do end describe "#decline_invite!" do - let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } it "destroys the member" do @@ -106,7 +142,6 @@ describe Member do end describe "#generate_invite_token" do - let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } it "sets the invite token" do