diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index bb25090425579f489004e1bbc951a8342401c08f..2505deaf757c2e142396987e1e19b992c352221c 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -286,10 +286,6 @@ color: #555; } -.project_member_row form { - margin: 0; -} - .transfer-project .select2-container { min-width: 200px; } diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 48dbf656e84a9207381c89f6820578ab3bdd2dc7..2ebc506040f4d89f979fdb2614165be8a929615f 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -1,11 +1,11 @@ class Groups::GroupMembersController < Groups::ApplicationController # Authorize - before_action :authorize_admin_group_member!, except: [:index, :leave] + before_action :authorize_admin_group_member!, except: [:index, :leave, :request_access] def index @project = @group.projects.find(params[:project_id]) if params[:project_id] @members = @group.group_members - @members = @members.non_invite unless can?(current_user, :admin_group, @group) + @members = @members.non_pending unless can?(current_user, :admin_group, @group) if params[:search].present? users = @group.users.search(params[:search]).to_a @@ -36,7 +36,7 @@ class Groups::GroupMembersController < Groups::ApplicationController return render_403 unless can?(current_user, :destroy_group_member, @group_member) - @group_member.destroy + @group_member.request? ? @group_member.decline_request : @group_member.destroy respond_to do |format| format.html { redirect_to group_group_members_path(@group), notice: 'User was successfully removed from group.' } @@ -59,12 +59,20 @@ class Groups::GroupMembersController < Groups::ApplicationController end def leave - @group_member = @group.group_members.find_by(user_id: current_user) + @group_member = + @group.group_members.find_by(user_id: current_user.id) || + @group.group_members.find_by(created_by_id: current_user.id) if can?(current_user, :destroy_group_member, @group_member) + notice = + if @group_member.request? + 'You withdrawn your access request to the group.' + else + "You left #{@group.name} group." + end @group_member.destroy - redirect_to(dashboard_groups_path, notice: "You left #{group.name} group.") + redirect_to dashboard_groups_path, notice: notice else if @group.last_owner?(current_user) redirect_to(dashboard_groups_path, alert: "You can not leave #{group.name} group because you're the last owner. Transfer or delete the group.") @@ -74,6 +82,22 @@ class Groups::GroupMembersController < Groups::ApplicationController end end + def request_access + @group.request_access(current_user) + + redirect_to group_path(@group), notice: 'Your request for access has been queued for review.' + end + + def approve + @group_member = @group.group_members.request.find(params[:id]) + + return render_403 unless can?(current_user, :update_group_member, @group_member) + + @group_member.accept_request + + redirect_to group_group_members_path(@group) + end + protected def member_params diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index ba5ef30be38198460238399710c8851dc331b1e3..c979c5e9fa99ad108ba4518da5c4455e0d85f897 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -14,9 +14,10 @@ class Projects::ProjectMembersController < Projects::ApplicationController @project_members = @project_members.order('access_level DESC') @group = @project.group + if @group @group_members = @group.group_members - @group_members = @group_members.non_invite unless can?(current_user, :admin_group, @group) + @group_members = @group_members.non_pending unless can?(current_user, :admin_group, @group) if params[:search].present? users = @group.users.search(params[:search]).to_a @@ -49,7 +50,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController return render_403 unless can?(current_user, :destroy_project_member, @project_member) - @project_member.destroy + @project_member.request? ? @project_member.decline_request : @project_member.destroy respond_to do |format| format.html do @@ -74,15 +75,20 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def leave - @project_member = @project.project_members.find_by(user_id: current_user) + @project_member = + @project.project_members.find_by(user_id: current_user.id) || + @project.project_members.find_by(created_by_id: current_user.id) if can?(current_user, :destroy_project_member, @project_member) + notice = + if @project_member.request? + 'You withdrawn your access request to the project.' + else + 'You left the project.' + end @project_member.destroy - respond_to do |format| - format.html { redirect_to dashboard_projects_path, notice: "You left the project." } - format.js { head :ok } - end + redirect_to dashboard_projects_path, notice: notice else if current_user == @project.owner message = 'You can not leave your own project. Transfer or delete the project.' @@ -94,30 +100,20 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def request_access - redirect_path = namespace_project_path(@project.namespace, @project) - # current_user - # @project - @project_member = ProjectMember.new(source: @project, access_level: ProjectMember::DEVELOPER, user_id: current_user.id, created_by_id: current_user.id, requested: true) - @project_member.save! - + @project.request_access(current_user) - redirect_to redirect_path, notice: 'Your request for access has been queued for review.' + redirect_to namespace_project_path(@project.namespace, @project), + notice: 'Your request for access has been queued for review.' end - def approval - @project_member = @project.project_members.find(params[:id]) + def approve + @project_member = @project.project_members.request.find(params[:id]) return render_403 unless can?(current_user, :update_project_member, @project_member) - @project_member.requested = nil - @project_member.save! + @project_member.accept_request - respond_to do |format| - format.html do - redirect_to namespace_project_project_members_path(@project.namespace, @project) - end - format.js { render nothing: true } - end + redirect_to namespace_project_project_members_path(@project.namespace, @project) end def apply_import diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 4cac69c6795272dc5afa715e8bc83306f5bc3ebd..b9211e884733f693e0905a3fbcc2c748564a244f 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -1,24 +1,4 @@ module GroupsHelper - def remove_user_from_group_message(group, member) - if member.user - "Are you sure you want to remove \"#{member.user.name}\" from \"#{group.name}\"?" - else - "Are you sure you want to revoke the invitation for \"#{member.invite_email}\" to join \"#{group.name}\"?" - end - end - - def leave_group_message(group) - "Are you sure you want to leave \"#{group}\" group?" - end - - def should_user_see_group_roles?(user, group) - if user - user.is_admin? || group.members.exists?(user_id: user.id) - else - false - end - end - def can_change_group_visibility_level?(group) can?(current_user, :change_visibility_level, group) end diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb new file mode 100644 index 0000000000000000000000000000000000000000..6599c59d1c9325d1a187fbc3dfa015431dd89529 --- /dev/null +++ b/app/helpers/members_helper.rb @@ -0,0 +1,117 @@ +module MembersHelper + def member_class(member) + "#{member.source.class.to_s}Member".constantize + end + + def members_association(entity) + "#{entity.class.to_s.underscore}_members".to_sym + end + + def action_member_permission(action, member) + "#{action}_#{member.source.class.to_s.underscore}_member".to_sym + end + + def can_see_entity_roles?(user, entity) + return false unless user + + user.is_admin? || entity.send(members_association(entity)).exists?(user_id: user.id) + end + + def member_path(member) + case member.source + when Project + namespace_project_project_member_path(member.source.namespace, member.source, member) + when Group + group_group_member_path(member.source, member) + else + raise ArgumentError.new('Unknown object class') + end + end + + def resend_invite_member_path(member) + case member.source + when Project + resend_invite_namespace_project_project_member_path(member.source.namespace, member.source, member) + when Group + resend_invite_group_group_member_path(member.source, member) + else + raise ArgumentError.new('Unknown object class') + end + end + + def request_access_path(entity) + case entity + when Project + request_access_namespace_project_project_members_path(entity.namespace, entity) + when Group + request_access_group_group_members_path(entity) + else + raise ArgumentError.new('Unknown object class') + end + end + + def approve_request_member_path(member) + case member.source + when Project + approve_namespace_project_project_member_path(member.source.namespace, member.source, member) + when Group + approve_group_group_member_path(member.source, member) + else + raise ArgumentError.new('Unknown object class') + end + end + + def leave_path(entity) + case entity + when Project + leave_namespace_project_project_members_path(entity.namespace, entity) + when Group + leave_group_group_members_path(entity) + else + raise ArgumentError.new('Unknown object class') + end + end + + def withdraw_request_message(entity) + "Are you sure you want to withdraw your access request for the \"#{entity_name(entity)}\" #{entity_type(entity)}?" + end + + def remove_member_message(member) + entity = member.source + entity_type = entity_type(entity) + entity_name = entity_name(entity) + + if member.request? + "You are going to deny #{member.created_by.name}'s request to join the #{entity_name} #{entity_type}. Are you sure?" + elsif member.invite? + "You are going to revoke the invitation for #{member.invite_email} to join the #{entity_name} #{entity_type}. Are you sure?" + else + "You are going to remove #{member.user.name} from the #{entity_name} #{entity_type}. Are you sure?" + end + end + + def remove_member_title(member) + member.request? ? 'Deny access request' : 'Remove user' + end + + def leave_confirmation_message(entity) + "Are you sure you want to leave \"#{entity_name(entity)}\" #{entity_type(entity)}?" + end + + private + + def entity_type(entity) + entity.class.to_s.underscore + end + + def entity_name(entity) + case entity + when Project + entity.name_with_namespace + when Group + entity.name + else + raise ArgumentError.new('Unknown object class') + end + end +end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index a015b5e6a02ca4729f659bd5e57a7b11ae630626..03941f87b13c72c8f35be9b4938d1d777daf9b0d 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -1,16 +1,6 @@ module ProjectsHelper - def remove_from_project_team_message(project, member) - if !member.user - "You are going to revoke the invitation for #{member.invite_email} to join #{project.name} project team. Are you sure?" - elsif member.request? - "You are going to deny #{member.user.name}'s request to join #{project.name} project team. Are you sure?" - else - "You are going to remove #{member.user.name} from #{project.name} project team. Are you sure?" - end - end - - def approve_for_project_team_message(project, member) - "You are going to approve #{member.user.name}'s request for #{member.human_access} access to the #{project.name} project team. Are you sure?" + def max_access_level(project, user) + Gitlab::Access.options_with_owner.key(project.team.max_member_access(user.id)) end def link_to_project(project) @@ -121,14 +111,6 @@ module ProjectsHelper end end - def user_max_access_in_project(user_id, project) - level = project.team.max_member_access(user_id) - - if level - Gitlab::Access.options_with_owner.key(level) - end - end - def license_short_name(project) return 'LICENSE' if project.repository.license_key.nil? @@ -292,10 +274,6 @@ module ProjectsHelper end end - def leave_project_message(project) - "Are you sure you want to leave \"#{project.name}\" project?" - end - def new_readme_path ref = @repository.root_ref if @repository ref ||= 'master' diff --git a/app/mailers/emails/groups.rb b/app/mailers/emails/groups.rb index 1c43f95dc8c61529b0ea4c18fadf91692f0d3575..fe218bfbe05a1f1e15c041a854e88a1f1f6a250b 100644 --- a/app/mailers/emails/groups.rb +++ b/app/mailers/emails/groups.rb @@ -1,22 +1,38 @@ module Emails module Groups + def group_access_requested_email(group_member_id) + setup_group_member_mail(group_member_id) + + @requester = @group_member.created_by + + group_admins = User.where(id: @group.group_members.admins.pluck(:user_id)).pluck(:notification_email) + + mail(to: group_admins, + subject: subject("Request to join #{@group.name} group")) + end + def group_access_granted_email(group_member_id) - @group_member = GroupMember.find(group_member_id) - @group = @group_member.group + setup_group_member_mail(group_member_id) - @target_url = group_url(@group) @current_user = @group_member.user - mail(to: @group_member.user.notification_email, - subject: subject("Access to group was granted")) + mail(to: @current_user.notification_email, + subject: subject("Access to #{@group.name} group was granted")) + end + + def group_access_denied_email(group_id, user_id) + @group = Group.find(group_id) + @current_user = User.find(user_id) + @target_url = group_url(@group) + + mail(to: @current_user.notification_email, + subject: subject("Access to #{@group.name} group was denied")) end def group_member_invited_email(group_member_id, token) - @group_member = GroupMember.find group_member_id - @group = @group_member.group - @token = token + setup_group_member_mail(group_member_id) - @target_url = group_url(@group) + @token = token @current_user = @group_member.user mail(to: @group_member.invite_email, @@ -24,15 +40,12 @@ module Emails end def group_invite_accepted_email(group_member_id) - @group_member = GroupMember.find group_member_id + setup_group_member_mail(group_member_id) return if @group_member.created_by.nil? - @group = @group_member.group - - @target_url = group_url(@group) @current_user = @group_member.created_by - mail(to: @group_member.created_by.notification_email, + mail(to: @current_user.notification_email, subject: subject("Invitation accepted")) end @@ -43,10 +56,18 @@ module Emails @current_user = @created_by = User.find(created_by_id) @access_level = access_level @invite_email = invite_email - + @target_url = group_url(@group) mail(to: @created_by.notification_email, subject: subject("Invitation declined")) end + + private + + def setup_group_member_mail(group_member_id) + @group_member = GroupMember.find(group_member_id) + @group = @group_member.group + @target_url = group_url(@group) + end end end diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index 6662c407c2c5c12d5fac381d2b340e844ca6da0b..43a2a7e80a882a5050d9457dd7a30bc75bbe0632 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -1,64 +1,38 @@ module Emails module Projects - def project_access_granted_email(project_member_id) - @project_member = ProjectMember.find project_member_id - @project = @project_member.project - - @target_url = namespace_project_url(@project.namespace, @project) - @current_user = @project_member.user + def project_access_requested_email(project_member_id) + setup_project_member_mail(project_member_id) - mail(to: @project_member.user.notification_email, - subject: subject("Access to project was granted")) - end + @requester = @project_member.created_by - def project_member_requested_access(project_member_id) - @project_member = ProjectMember.find project_member_id - @project = @project_member.project - @target_url = namespace_project_url(@project.namespace, @project) + project_admins = User.where(id: @project.project_members.admins.pluck(:user_id)).pluck(:notification_email) - project_admins = ProjectMember.in_project(@project) - .where(access_level: [Gitlab::Access::OWNER, Gitlab::Access::MASTER]) - .pluck(:notification_email) - - project_admins.each do |address| - mail(to: address, - subject: subject("Request to join project: #{@project.name_with_namespace}")) - end + mail(to: project_admins, + subject: subject("Request to join #{@project.name_with_namespace} project")) end - def project_request_access_accepted_email(project_member_id) - @project_member = ProjectMember.find project_member_id - return if @project_member.created_by.nil? - - @project = @project_member.project + def project_access_granted_email(project_member_id) + setup_project_member_mail(project_member_id) - @target_url = namespace_project_url(@project.namespace, @project) - @current_user = @project_member.created_by + @current_user = @project_member.user - mail(to: @project_member.created_by.notification_email, - subject: subject('Request for access granted')) + mail(to: @current_user.notification_email, + subject: subject("Access to #{@project.name_with_namespace} project was granted")) end - def project_request_access_declined_email(project_member_id) - @project_member = ProjectMember.find project_member_id - return if @project_member.created_by.nil? - - @project = @project_member.project - + def project_access_denied_email(project_id, user_id) + @project = Project.find(project_id) + @current_user = User.find(user_id) @target_url = namespace_project_url(@project.namespace, @project) - @current_user = @project_member.created_by - mail(to: @project_member.created_by.notification_email, - subject: subject('Request for access declined')) + mail(to: @current_user.notification_email, + subject: subject("Access to #{@project.name_with_namespace} project was denied")) end - def project_member_invited_email(project_member_id, token) - @project_member = ProjectMember.find project_member_id - @project = @project_member.project - @token = token + setup_project_member_mail(project_member_id) - @target_url = namespace_project_url(@project.namespace, @project) + @token = token @current_user = @project_member.user mail(to: @project_member.invite_email, @@ -66,12 +40,9 @@ module Emails end def project_invite_accepted_email(project_member_id) - @project_member = ProjectMember.find project_member_id + setup_project_member_mail(project_member_id) return if @project_member.created_by.nil? - @project = @project_member.project - - @target_url = namespace_project_url(@project.namespace, @project) @current_user = @project_member.created_by mail(to: @project_member.created_by.notification_email, @@ -117,5 +88,13 @@ module Emails reply_to: @message.reply_to, subject: @message.subject) end + + private + + def setup_project_member_mail(project_member_id) + @project_member = ProjectMember.find(project_member_id) + @project = @project_member.project + @target_url = namespace_project_url(@project.namespace, @project) + end end end diff --git a/app/models/ability.rb b/app/models/ability.rb index b3db26f989e81ffd5364aac93a32c7bb016d5ffc..90156bf130c9fa27f9b68522bf64c9b58bd97ac1 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -153,7 +153,7 @@ class Ability RequestStore.store[key] ||= begin # Push abilities on the users team role - rules.push(*project_team_rules(project.team, user)) unless project.team.pending?(user) + rules.push(*project_team_rules(project.team, user)) if project.owner == user || (project.group && project.group.has_owner?(user)) || @@ -187,6 +187,8 @@ class Ability project_report_rules elsif team.guest?(user) project_guest_rules + else + [] end end @@ -458,6 +460,8 @@ class Ability rules << :destroy_group_member elsif user == target_user rules << :destroy_group_member + elsif subject.request? && user == subject.created_by + rules << :destroy_group_member end end @@ -477,6 +481,8 @@ class Ability rules << :destroy_project_member elsif user == target_user rules << :destroy_project_member + elsif subject.request? && user == subject.created_by + rules << :destroy_project_member end end diff --git a/app/models/concerns/access_requestable.rb b/app/models/concerns/access_requestable.rb new file mode 100644 index 0000000000000000000000000000000000000000..cf37284e31a3239abd1d140a05dfe7405432dc77 --- /dev/null +++ b/app/models/concerns/access_requestable.rb @@ -0,0 +1,27 @@ +# == AccessRequestable concern +# +# Contains functionality related to objects that can receive request for access. +# +# Used by Project, and Group. +# +module AccessRequestable + extend ActiveSupport::Concern + + def request_access(user) + members.create( + access_level: Gitlab::Access::DEVELOPER, + created_by: user, + requested_at: Time.now.utc) + end + + def access_requested?(user) + members.where(created_by_id: user.id).where.not(requested_at: nil).any? + end + + private + + # Returns a `<entities>_members` association, e.g.: project_members, group_members + def members + @members ||= send("#{self.class.to_s.underscore}_members".to_sym) + end +end diff --git a/app/models/group.rb b/app/models/group.rb index aec92e335e63a36a8928de46ab96fefa8fba0130..b6929112cba2e06bd6c2577fe24cc2332bdca519 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -3,6 +3,7 @@ require 'carrierwave/orm/activerecord' class Group < Namespace include Gitlab::ConfigHelper include Gitlab::VisibilityLevel + include AccessRequestable include Referable has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember' diff --git a/app/models/member.rb b/app/models/member.rb index 2210e7dd66a8e2b6014ea6a9f94b8db6a2e6dc77..5c3a5eab406cca26db5464d8e1bf2247ab897bde 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -8,7 +8,7 @@ class Member < ActiveRecord::Base belongs_to :user belongs_to :source, polymorphic: true - validates :user, presence: true, unless: :invite? + validates :user, presence: true, unless: :pending? validates :source, presence: true validates :user_id, uniqueness: { scope: [:source_type, :source_id], message: "already exists in source", @@ -26,29 +26,25 @@ class Member < ActiveRecord::Base allow_nil: true } - scope :invite, -> { where(user_id: nil) } - scope :non_invite, -> { where('user_id IS NOT NULL') } - scope :request, -> { where(requested: true) } - scope :non_request, -> { where(requested: nil) } - scope :pending, -> { where("user_id IS NULL OR requested") } - scope :non_pending, -> { self.non_invite.non_request } + scope :invite, -> { where.not(invite_token: nil) } + scope :request, -> { where.not(requested_at: nil) } + scope :non_request, -> { where(requested_at: nil) } + scope :non_pending, -> { where.not(user_id: nil) } 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) } + scope :admins, -> { where(access_level: [OWNER, MASTER]) } before_validation :generate_invite_token, on: :create, if: -> (member) { member.invite_email.present? } after_create :send_invite, if: :invite? - after_create :send_request_access, if: :request? - + after_create :send_request, if: :request? after_create :create_notification_setting, unless: :pending? after_create :post_create_hook, unless: :pending? - after_update :post_update_hook, unless: :pending? - after_destroy :post_destroy_hook, unless: :pending? delegate :name, :username, :email, to: :user, prefix: true @@ -111,31 +107,29 @@ class Member < ActiveRecord::Base end def request? - self.requested + user.nil? && created_by.present? && requested_at.present? end def invite? self.invite_token.present? end - def accept_request_access! + def accept_request return false unless request? - self.request = false - saved = self.save + updated = self.update(user: created_by, requested_at: nil) + after_accept_request if updated - after_accept_request_access if saved - - saved + updated end - def decline_request_access! + def decline_request return false unless request? - destroyed = self.destroy - after_decline_request_access if destroyed + self.destroy + after_decline_request if destroyed? - destroyed + destroyed? end def accept_invite!(new_user) @@ -191,11 +185,11 @@ class Member < ActiveRecord::Base private - def send_request_access + def send_invite # override in subclass end - def send_invite + def send_request # override in subclass end @@ -211,19 +205,19 @@ class Member < ActiveRecord::Base system_hook_service.execute_hooks_for(self, :destroy) end - def after_accept_request_access + def after_accept_invite post_create_hook end - def after_decline_request_access + def after_decline_invite # override in subclass end - def after_accept_invite + def after_accept_request post_create_hook end - def after_decline_invite + def after_decline_request # override in subclass end diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb index f63a0debf1a80304d660ff765b6033d92d0aab78..476b4816b90ef9e37a9a780d57377b461b2c51b7 100644 --- a/app/models/members/group_member.rb +++ b/app/models/members/group_member.rb @@ -8,9 +8,6 @@ class GroupMember < Member validates_format_of :source_type, with: /\ANamespace\z/ default_scope { where(source_type: SOURCE_TYPE) } - scope :with_group, ->(group) { where(source_id: group.id) } - scope :with_user, ->(user) { where(user_id: user.id) } - def self.access_level_roles Gitlab::Access.options_with_owner end @@ -31,6 +28,12 @@ class GroupMember < Member super end + def send_request + notification_service.new_group_access_request(self) + + super + end + def post_create_hook notification_service.new_group_member(self) @@ -56,4 +59,10 @@ class GroupMember < Member super end + + def after_decline_request + notification_service.decline_group_access_request(group, created_by) + + super + end end diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index 9db8db8450d8227a2f1328d0f849c42f867f5437..c6fd1a5c3d1a1243ba17f550afb0a48ba7ace82b 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -11,8 +11,6 @@ class ProjectMember < Member default_scope { where(source_type: SOURCE_TYPE) } 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) } before_destroy :delete_member_todos @@ -84,7 +82,7 @@ class ProjectMember < Member Gitlab::Access.sym_options end - def access_roles + def access_level_roles Gitlab::Access.options end end @@ -107,14 +105,14 @@ class ProjectMember < Member user.todos.where(project_id: source_id).destroy_all if user end - def send_request_access - notification_service.request_access_project_member(self) + def send_invite + notification_service.invite_project_member(self, @raw_invite_token) super end - def send_invite - notification_service.invite_project_member(self, @raw_invite_token) + def send_request + notification_service.new_project_access_request(self) super end @@ -142,18 +140,6 @@ class ProjectMember < Member super end - def after_accept_request_access - notification_service.accept_project_request_access(self) - - super - end - - def after_decline_request_access - notification_service.decline_project_request_access(self) - - super - end - def after_accept_invite notification_service.accept_project_invite(self) @@ -166,6 +152,12 @@ class ProjectMember < Member super end + def after_decline_request + notification_service.decline_project_access_request(project, created_by) + + super + end + def event_service EventCreateService.new end diff --git a/app/models/project.rb b/app/models/project.rb index dfa99fe0df273ac1174e902f3ea5c18c236d5363..ef6653734959f2edd67e364ddaed508bae8660e7 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -5,6 +5,7 @@ class Project < ActiveRecord::Base include Gitlab::ShellAdapter include Gitlab::VisibilityLevel include Gitlab::CurrentSettings + include AccessRequestable include Referable include Sortable include AfterCommitQueue @@ -102,7 +103,7 @@ class Project < ActiveRecord::Base has_many :snippets, dependent: :destroy, class_name: 'ProjectSnippet' has_many :hooks, dependent: :destroy, class_name: 'ProjectHook' has_many :protected_branches, dependent: :destroy - has_many :project_members, dependent: :destroy, as: :source, class_name: 'ProjectMember' + has_many :project_members, dependent: :destroy, as: :source, class_name: 'ProjectMember' has_many :users, through: :project_members has_many :deploy_keys_projects, dependent: :destroy has_many :deploy_keys, through: :deploy_keys_projects @@ -680,16 +681,6 @@ class Project < ActiveRecord::Base end end - def project_member_by_name_or_email(name = nil, email = nil) - user = users.find_by('name like ? or email like ?', name, email) - project_members.where(user: user) if user - end - - # Get Team Member record by user id - def project_member_by_id(user_id) - project_members.find_by(user_id: user_id) - end - def name_with_namespace @name_with_namespace ||= begin if namespace diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 769b73666ceada290f43df1515251102d8c1b4ac..7fb17df0e965ce1422572bdfa892f9222a7a05ca 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -21,16 +21,6 @@ class ProjectTeam end end - def find(user_id) - user = project.users.find_by(id: user_id) - - if group - user ||= group.users.find_by(id: user_id) - end - - user - end - def find_member(user_id) member = project.project_members.find_by(user_id: user_id) @@ -61,13 +51,10 @@ class ProjectTeam ProjectMember.truncate_team(project) end - def users - members - end - def members @members ||= fetch_members end + alias_method :users, :members def guests @guests ||= fetch_members(:guests) @@ -115,12 +102,6 @@ class ProjectTeam false end - def pending?(user) - project.project_members.each do |member| - return member.pending? if member.user_id == user.id - end - end - def guest?(user) max_member_access(user.id) == Gitlab::Access::GUEST end @@ -147,10 +128,6 @@ class ProjectTeam end end - def human_max_access(user_id) - Gitlab::Access.options_with_owner.key(max_member_access(user_id)) - end - # This method assumes project and group members are eager loaded for optimal # performance. def max_member_access(user_id) @@ -179,6 +156,7 @@ class ProjectTeam access.compact.max end + private def max_invited_level(user_id) project.project_group_links.map do |group_link| @@ -195,8 +173,6 @@ class ProjectTeam end.compact.max end - private - def fetch_members(level = nil) project_members = project.project_members group_members = group ? group.group_members : [] diff --git a/app/models/user.rb b/app/models/user.rb index a5b3c8afe51b539e4f635b2056a3103abba15e6f..8d0427da5aba4d2f08a95d72d411e0a61c3bcb42 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -56,8 +56,7 @@ class User < ActiveRecord::Base # Groups has_many :members, dependent: :destroy - has_many :project_members, source: 'ProjectMember' - has_many :group_members, source: 'GroupMember' + has_many :group_members, dependent: :destroy, source: 'GroupMember' has_many :groups, through: :group_members has_many :owned_groups, -> { where members: { access_level: Gitlab::Access::OWNER } }, through: :group_members, source: :group has_many :masters_groups, -> { where members: { access_level: Gitlab::Access::MASTER } }, through: :group_members, source: :group @@ -65,13 +64,13 @@ class User < ActiveRecord::Base # Projects has_many :groups_projects, through: :groups, source: :projects has_many :personal_projects, through: :namespace, source: :projects + has_many :project_members, dependent: :destroy, class_name: 'ProjectMember' has_many :projects, through: :project_members has_many :created_projects, foreign_key: :creator_id, class_name: 'Project' has_many :users_star_projects, dependent: :destroy has_many :starred_projects, through: :users_star_projects, source: :project has_many :snippets, dependent: :destroy, foreign_key: :author_id, class_name: "Snippet" - has_many :project_members, dependent: :destroy, class_name: 'ProjectMember' has_many :issues, dependent: :destroy, foreign_key: :author_id has_many :notes, dependent: :destroy, foreign_key: :author_id has_many :merge_requests, dependent: :destroy, foreign_key: :author_id diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index e7676861e9b64f368b6ed77382c51c6ab4b32d17..cd11feb9d7a494a8b069088433904dcf56a80e44 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -173,16 +173,13 @@ class NotificationService end end - def request_access_project_member(project_member) - mailer.project_member_requested_access(project_member.id).deliver_later + # Project access request + def new_project_access_request(project_member) + mailer.project_access_requested_email(project_member.id).deliver_later end - def accept_project_request_access(project_member) - mailer.project_request_access_accepted_email(project_member.id).deliver_later - end - - def decline_project_request_access(project_member) - mailer.project_request_access_declined_email(project_member.id).deliver_later + def decline_project_access_request(project, user) + mailer.project_access_denied_email(project.id, user.id).deliver_later end def invite_project_member(project_member, token) @@ -210,6 +207,15 @@ class NotificationService mailer.project_access_granted_email(project_member.id).deliver_later end + # Group access request + def new_group_access_request(group_member) + mailer.group_access_requested_email(group_member.id).deliver_later + end + + def decline_group_access_request(group, user) + mailer.group_access_denied_email(group.id, user.id).deliver_later + end + def invite_group_member(group_member, token) mailer.group_member_invited_email(group_member.id, token).deliver_later end diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml index f309e80a39a7d8a2b42530f9b3ed242552169be9..5b8a0262ea076b8827a1dbc799ac7389cc1a0d29 100644 --- a/app/views/admin/groups/show.html.haml +++ b/app/views/admin/groups/show.html.haml @@ -109,7 +109,7 @@ %span.pull-right.light = member.human_access - if can?(current_user, :destroy_group_member, member) - = link_to group_group_member_path(@group, member), data: { confirm: remove_user_from_group_message(@group, member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do + = link_to group_group_member_path(@group, member), data: { confirm: remove_member_message(member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do %i.fa.fa-minus.fa-inverse .panel-footer = paginate @members, param_name: 'members_page', theme: 'gitlab' diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index 73986d21bcf73c90a1dc435c3d28489fc9167cb0..9e55a562e18032d4cda87d49032792ec72fe6790 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -142,7 +142,7 @@ %i.fa.fa-pencil-square-o %ul.well-list - @group_members.each do |member| - = render 'groups/group_members/group_member', member: member, show_controls: false + = render 'shared/members/member', member: member, show_controls: false .panel-footer = paginate @group_members, param_name: 'group_members_page', theme: 'gitlab' @@ -172,7 +172,7 @@ %span.light Owner - else %span.light= project_member.human_access - = link_to namespace_project_project_member_path(@project.namespace, @project, project_member), data: { confirm: remove_from_project_team_message(@project, project_member)}, method: :delete, remote: true, class: "btn btn-sm btn-remove" do + = link_to namespace_project_project_member_path(@project.namespace, @project, project_member), data: { confirm: remove_member_message(project_member)}, method: :delete, remote: true, class: "btn btn-sm btn-remove" do %i.fa.fa-times .panel-footer = paginate @project_members, param_name: 'project_members_page', theme: 'gitlab' diff --git a/app/views/admin/users/groups.html.haml b/app/views/admin/users/groups.html.haml index dbecb7bbfd641578894ed8a33f6a1927e0c3e7c2..b0a709a568a62ac73c42b56e888becc704922264 100644 --- a/app/views/admin/users/groups.html.haml +++ b/app/views/admin/users/groups.html.haml @@ -13,7 +13,7 @@ .pull-right %span.light= group_member.human_access - unless group_member.owner? - = link_to group_group_member_path(group, group_member), data: { confirm: remove_user_from_group_message(group, group_member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do + = link_to group_group_member_path(group, group_member), data: { confirm: remove_member_message(group_member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do %i.fa.fa-times.fa-inverse - else .nothing-here-block This user has no groups. diff --git a/app/views/admin/users/projects.html.haml b/app/views/admin/users/projects.html.haml index b655b2a15f5b123dcec41050ee77926de734172b..84b9ceb23b3ba11686904ebfb65b460c81e7a911 100644 --- a/app/views/admin/users/projects.html.haml +++ b/app/views/admin/users/projects.html.haml @@ -38,6 +38,5 @@ %span.light= member.human_access - if member.respond_to? :project - = link_to namespace_project_project_member_path(project.namespace, project, member), data: { confirm: remove_from_project_team_message(project, member) }, remote: true, method: :delete, class: "btn-xs btn btn-remove", title: 'Remove user from project' do + = link_to namespace_project_project_member_path(project.namespace, project, member), data: { confirm: remove_member_message(member) }, remote: true, method: :delete, class: "btn-xs btn btn-remove", title: 'Remove user from project' do %i.fa.fa-times - diff --git a/app/views/groups/group_members/_group_member.html.haml b/app/views/groups/group_members/_group_member.html.haml deleted file mode 100644 index 6bb542e658dba01b2e4fd9b10d65bb0a3b508b23..0000000000000000000000000000000000000000 --- a/app/views/groups/group_members/_group_member.html.haml +++ /dev/null @@ -1,57 +0,0 @@ -- user = member.user -- return unless user || member.invite? -- show_roles = local_assigns.fetch(:show_roles, true) - -%li{class: "#{dom_class(member)} js-toggle-container", id: dom_id(member)} - %span{class: ("list-item-name" if show_controls)} - - if member.user - = image_tag avatar_icon(user, 24), class: "avatar s24", alt: '' - %strong - = link_to user.name, user_path(user) - %span.cgray= user.username - - if user == current_user - %span.label.label-success It's you - - if user.blocked? - %label.label.label-danger - %strong Blocked - - else - = image_tag avatar_icon(member.invite_email, 24), class: "avatar s24", alt: '' - %strong - = member.invite_email - %span.cgray - invited - - if member.created_by - by - = link_to member.created_by.name, user_path(member.created_by) - = time_ago_with_tooltip(member.created_at) - - - if show_controls && can?(current_user, :admin_group_member, @group) - = link_to resend_invite_group_group_member_path(@group, member), method: :post, class: "btn-xs btn", title: 'Resend invite' do - Resend invite - - - if show_roles && should_user_see_group_roles?(current_user, @group) - %span.pull-right - %strong.member-access-level= member.human_access - - if show_controls - - if can?(current_user, :update_group_member, member) - = button_tag class: "btn-xs btn btn-grouped inline js-toggle-button", - title: 'Edit access level', type: 'button' do - = icon('pencil') - - - if can?(current_user, :destroy_group_member, member) - - - if current_user == user - = link_to leave_group_group_members_path(@group), data: { confirm: leave_group_message(@group.name)}, method: :delete, class: "btn-xs btn btn-remove", title: 'Remove user from group' do - = icon("sign-out") - Leave - - else - = link_to group_group_member_path(@group, member), data: { confirm: remove_user_from_group_message(@group, member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do - = icon('trash') - - .edit-member.hide.js-toggle-content - %br - = form_for [@group, member], remote: true do |f| - .prepend-top-10 - = f.select :access_level, options_for_select(GroupMember.access_level_roles, member.access_level), {}, class: 'form-control' - .prepend-top-10 - = f.submit 'Save', class: 'btn btn-save btn-sm' diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml index 0eb6bbd442015e0843a46163d6e6e372cb031cc2..a39d5d3d0f0006d0aa37d54d63a5d5d5c41787ed 100644 --- a/app/views/groups/group_members/index.html.haml +++ b/app/views/groups/group_members/index.html.haml @@ -6,12 +6,13 @@ .panel-heading Add new user to group .panel-body - - if should_user_see_group_roles?(current_user, @group) - %p.light - Members of group have access to all group projects. + %p.light + Members of group have access to all group projects. .new-group-member-holder = render "new_group_member" + = render "shared/members/requests", entity: @group, members: @members + .panel.panel-default .panel-heading %strong #{@group.name} @@ -25,9 +26,8 @@ = button_tag class: 'btn', title: 'Search' do = icon("search") %ul.content-list - - @members.each do |member| - = render 'groups/group_members/group_member', member: member, show_controls: true - = paginate @members, theme: 'gitlab' + = render partial: 'shared/members/member', collection: @members.non_request, as: :member + = paginate @members.non_request, theme: 'gitlab' :javascript $('form.member-search-form').on('submit', function(event) { diff --git a/app/views/groups/group_members/update.js.haml b/app/views/groups/group_members/update.js.haml index df726e2b2b9f355528af9b3ba3247c159815847c..b0b3a51ce58629cbf6d25df0d48b6e567424811a 100644 --- a/app/views/groups/group_members/update.js.haml +++ b/app/views/groups/group_members/update.js.haml @@ -1,2 +1,2 @@ :plain - $("##{dom_id(@group_member)}").replaceWith('#{escape_javascript(render(@group_member, member: @group_member, show_controls: true))}'); + $("##{dom_id(@group_member)}").replaceWith('#{escape_javascript(render(@group_member, member: @group_member))}'); diff --git a/app/views/layouts/nav/_group_settings.html.haml b/app/views/layouts/nav/_group_settings.html.haml index 0b2673f1a8291f6b21140939901a5d474ab5c49e..b461772b87eb1247e1ccb30d18da31044b38fe98 100644 --- a/app/views/layouts/nav/_group_settings.html.haml +++ b/app/views/layouts/nav/_group_settings.html.haml @@ -1,20 +1,3 @@ - if current_user - - if access = @group.users.find_by(id: current_user.id) - .controls - .dropdown.group-settings-dropdown - %a.dropdown-new.btn.btn-default#group-settings-button{href: '#', 'data-toggle' => 'dropdown'} - = icon('cog') - = icon('caret-down') - %ul.dropdown-menu.dropdown-menu-align-right - - if can?(current_user, :admin_group, @group) - = nav_link(path: 'groups#projects') do - = link_to projects_group_path(@group), title: 'Projects' do - Projects - %li.divider - %li - = link_to edit_group_path(@group) do - Edit Group - %li - = link_to leave_group_group_members_path(@group), - data: { confirm: leave_group_message(@group.name) }, method: :delete, title: 'Leave group' do - Leave Group + .controls + = render 'shared/group_or_project_home_dropdown', entity: @group diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 1336191bc5e7174d28aab672b1592222c61210ff..3398794302fdfb03cbbc67b779ad0f4aa89210c6 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -8,19 +8,6 @@ = icon('caret-down') %ul.dropdown-menu.dropdown-menu-align-right = render 'layouts/nav/project_settings' - - - if access - %li - = link_to leave_namespace_project_project_members_path(@project.namespace, @project), - data: { confirm: leave_project_message(@project) }, method: :delete, title: 'Leave project' do - Leave Project - - else - = link_to request_access_namespace_project_project_members_path(@project.namespace, @project), - class: 'btn btn-gray', style: 'margin-left: 10px', method: :post, title: 'Request access' do - Request Access - - - %li.divider - if can_edit %li @@ -28,13 +15,18 @@ Edit Project - if access %li - = link_to leave_namespace_project_project_members_path(@project.namespace, @project), - data: { confirm: leave_project_message(@project) }, method: :delete, title: 'Leave project' do + = link_to leave_path(@project), + data: { confirm: leave_confirmation_message(@project) }, method: :delete do Leave Project + - elsif @project.access_requested?(current_user) + %li + = link_to leave_path(@project), + data: { confirm: withdraw_request_message(@project) }, method: :delete do + Withdraw Request - else %li - = link_to request_access_namespace_project_project_members_path(@project.namespace, @project), - class: 'btn btn-gray', style: 'margin-left: 10px', method: :post, title: 'Request access' do + = link_to request_access_path(@project), + class: 'btn btn-gray', style: 'margin-left: 10px', method: :post do Request Access %div{ class: nav_control_class } diff --git a/app/views/notify/group_access_denied_email.html.haml b/app/views/notify/group_access_denied_email.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..4edfd4e4793855685c11f9d31bd457bf9e85602f --- /dev/null +++ b/app/views/notify/group_access_denied_email.html.haml @@ -0,0 +1,2 @@ +%p + Your request to join group #{link_to @group.name, @target_url} has been denied. diff --git a/app/views/notify/group_access_denied_email.text.erb b/app/views/notify/group_access_denied_email.text.erb new file mode 100644 index 0000000000000000000000000000000000000000..cb32177e8262e5b51c5de7314e3090f17840892a --- /dev/null +++ b/app/views/notify/group_access_denied_email.text.erb @@ -0,0 +1,3 @@ +Your request to join group <%= @group.name %> has been denied. + +<%= @target_url %> diff --git a/app/views/notify/group_access_granted_email.html.haml b/app/views/notify/group_access_granted_email.html.haml index f1916d624b6b35d9fabe1808660022197e8b324d..1283758c576e94d5c4c75b5037895c7b14be4cf7 100644 --- a/app/views/notify/group_access_granted_email.html.haml +++ b/app/views/notify/group_access_granted_email.html.haml @@ -1,4 +1,3 @@ %p - = "You have been granted #{@group_member.human_access} access to group" - = link_to group_url(@group) do - = @group.name + You have been granted #{@group_member.human_access} access to group + #{link_to @group.name, @target_url}. diff --git a/app/views/notify/group_access_granted_email.text.erb b/app/views/notify/group_access_granted_email.text.erb index ef9617bfc16ad3d47aa75524e5d28a379ca1639b..c75683500750bd7e9629cafb2cb9971a656cc9eb 100644 --- a/app/views/notify/group_access_granted_email.text.erb +++ b/app/views/notify/group_access_granted_email.text.erb @@ -1,4 +1,3 @@ +You have been granted <%= @group_member.human_access %> access to group <%= @group.name %>. -You have been granted <%= @group_member.human_access %> access to group <%= @group.name %> - -<%= url_for(group_url(@group)) %> +<%= @target_url %> diff --git a/app/views/notify/group_access_requested_email.html.haml b/app/views/notify/group_access_requested_email.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..4fbcedabae03d9a93d9d754a777ff4e5ff4464a7 --- /dev/null +++ b/app/views/notify/group_access_requested_email.html.haml @@ -0,0 +1,3 @@ +%p + #{link_to @requester.name, @requester} requested #{@group_member.human_access} + access to group #{link_to @group.name, @target_url}. diff --git a/app/views/notify/group_access_requested_email.text.erb b/app/views/notify/group_access_requested_email.text.erb new file mode 100644 index 0000000000000000000000000000000000000000..2f9d293a79e12d26865452ab8e3ef0e3f00c0abc --- /dev/null +++ b/app/views/notify/group_access_requested_email.text.erb @@ -0,0 +1,3 @@ +<%= @requester.name %> (<%= user_url(@requester) %>) requested <%= @group_member.human_access %> access to group <%= @group.name %> + +<%= @target_url %> diff --git a/app/views/notify/project_access_denied_email.html.haml b/app/views/notify/project_access_denied_email.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..cecdaf24f39fc1e885c0b11db4e8fa672904a28e --- /dev/null +++ b/app/views/notify/project_access_denied_email.html.haml @@ -0,0 +1,3 @@ +%p + Your request to join project #{link_to @project.name_with_namespace, @target_url} + has been denied. diff --git a/app/views/notify/project_request_access_denied_email.text.erb b/app/views/notify/project_access_denied_email.text.erb similarity index 58% rename from app/views/notify/project_request_access_denied_email.text.erb rename to app/views/notify/project_access_denied_email.text.erb index a9c57e4cab4d3980cdb305d121739e8ddd96c578..24357e059d2bf14a064a6552d898765f6b1ee675 100644 --- a/app/views/notify/project_request_access_denied_email.text.erb +++ b/app/views/notify/project_access_denied_email.text.erb @@ -1,3 +1,3 @@ Your request to join project <%= @project.name_with_namespace %> has been denied. -<%= namespace_project_url(@project.namespace, @project) %> +<%= @target_url %> diff --git a/app/views/notify/project_access_granted_email.html.haml b/app/views/notify/project_access_granted_email.html.haml index dfc30a2d360cd312cfdb959731994ea6cafa8138..88873e7fe52679794635b1ae065b6d01e4c49289 100644 --- a/app/views/notify/project_access_granted_email.html.haml +++ b/app/views/notify/project_access_granted_email.html.haml @@ -1,5 +1,3 @@ %p - = "You have been granted #{@project_member.human_access} access to project" -%p - = link_to namespace_project_url(@project.namespace, @project) do - = @project.name_with_namespace + You have been granted #{@project_member.human_access} access to project + #{link_to @project.name_with_namespace, @target_url}. diff --git a/app/views/notify/project_access_granted_email.text.erb b/app/views/notify/project_access_granted_email.text.erb index 68eb1611ba7e6f4e73faa8d09ca8de7d01892ba9..f5e4b31385886c4f745a0d11603b3b8e1eeb374e 100644 --- a/app/views/notify/project_access_granted_email.text.erb +++ b/app/views/notify/project_access_granted_email.text.erb @@ -1,4 +1,3 @@ +You have been granted <%= @project_member.human_access %> access to project <%= @project.name_with_namespace %>. -You have been granted <%= @project_member.human_access %> access to project <%= @project.name_with_namespace %> - -<%= url_for(namespace_project_url(@project.namespace, @project)) %> +<%= @target_url %> diff --git a/app/views/notify/project_access_requested_email.html.haml b/app/views/notify/project_access_requested_email.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..2a705ad3b0a0e3a3041b434d30b434a9a524c879 --- /dev/null +++ b/app/views/notify/project_access_requested_email.html.haml @@ -0,0 +1,3 @@ +%p + #{link_to @requester.name, @requester} requested #{@project_member.human_access} + access to project #{link_to @project.name_with_namespace, @target_url}. diff --git a/app/views/notify/project_access_requested_email.text.erb b/app/views/notify/project_access_requested_email.text.erb new file mode 100644 index 0000000000000000000000000000000000000000..2437fa4ee86bd76926fe599a86567e06fa815d29 --- /dev/null +++ b/app/views/notify/project_access_requested_email.text.erb @@ -0,0 +1,3 @@ +<%= @requester.name %> (<%= user_url(@requester) %>) requested <%= @project_member.human_access %> access to project <%= @project.name_with_namespace %>. + +<%= @target_url %> diff --git a/app/views/notify/project_request_access_accepted_email.html.haml b/app/views/notify/project_request_access_accepted_email.html.haml deleted file mode 100644 index dfdf82e70a52f58972d6f1b9f76569672aa8882a..0000000000000000000000000000000000000000 --- a/app/views/notify/project_request_access_accepted_email.html.haml +++ /dev/null @@ -1,4 +0,0 @@ -%p - Your request to join project - #{link_to @project.name_with_namespace, namespace_project_url(@project.namespace, @project)} - has been granted with #{@project_member.human_access} access. diff --git a/app/views/notify/project_request_access_accepted_email.text.erb b/app/views/notify/project_request_access_accepted_email.text.erb deleted file mode 100644 index 9fb6887449467535907890917b4f2c615656858f..0000000000000000000000000000000000000000 --- a/app/views/notify/project_request_access_accepted_email.text.erb +++ /dev/null @@ -1,3 +0,0 @@ -Your request to join project <%= @project.name_with_namespace %> has been granted with <%= @project_member.human_access %> access. - -<%= namespace_project_url(@project.namespace, @project) %> diff --git a/app/views/notify/project_request_access_denied_email.html.haml b/app/views/notify/project_request_access_denied_email.html.haml deleted file mode 100644 index 8ad75b96cf9cf3bd7bb86a9fa56aa0726a066e82..0000000000000000000000000000000000000000 --- a/app/views/notify/project_request_access_denied_email.html.haml +++ /dev/null @@ -1,4 +0,0 @@ -%p - Your request to join project - #{link_to @project.name_with_namespace, namespace_project_url(@project.namespace, @project)} - has been denied. diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index bcdbff080116aea18b6dff84d739766a1f732eb2..112a532f9d3acc0410b889f71b39aee50ce18fb2 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -17,7 +17,7 @@ %a{ href: "##{dom_id(note)}" } = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago') .note-actions - - access = note.project.team.human_max_access(note.author.id) + - access = max_access_level(note.project, note.author) - if access %span.note-role.hidden-xs= access - if current_user diff --git a/app/views/projects/project_members/_group_members.html.haml b/app/views/projects/project_members/_group_members.html.haml index 6671ee2c6d6be9a7cd73f7ba68e4b3643bd3bf32..78c12d52a785667899254f1062f6b12e328bcbb9 100644 --- a/app/views/projects/project_members/_group_members.html.haml +++ b/app/views/projects/project_members/_group_members.html.haml @@ -9,8 +9,13 @@ = link_to group_group_members_path(@group), class: 'btn' do Manage group members %ul.content-list - - members.limit(20).each do |member| - = render 'groups/group_members/group_member', member: member, show_controls: false - - if members.count > 20 + = render partial: 'shared/members/member', + collection: members.limit(20), + as: :member, + locals: { show_controls: false } + - if members.size > 20 %li - and #{members.count - 20} more. For full list visit #{link_to 'group members page', group_group_members_path(@group)} + and + = members.size - 20 + more. For full list visit + = link_to 'group members page', group_group_members_path(@group) diff --git a/app/views/projects/project_members/_new_project_member.html.haml b/app/views/projects/project_members/_new_project_member.html.haml index f0f3bb3c177eab2d04f5b1f01ef0135f4b6c4fb2..82892a3335828e877ba7510598c6fe34bca68e4d 100644 --- a/app/views/projects/project_members/_new_project_member.html.haml +++ b/app/views/projects/project_members/_new_project_member.html.haml @@ -9,7 +9,7 @@ .form-group = f.label :access_level, "Project Access", class: 'control-label' .col-sm-10 - = select_tag :access_level, options_for_select(ProjectMember.access_roles, @project_member.access_level), class: "project-access-select select2" + = select_tag :access_level, options_for_select(ProjectMember.access_level_roles, @project_member.access_level), class: "project-access-select select2" .help-block Read more about role permissions %strong= link_to "here", help_page_path("permissions", "permissions"), class: "vlink" diff --git a/app/views/projects/project_members/_pending.html.haml b/app/views/projects/project_members/_pending.html.haml deleted file mode 100644 index 88ac36937acb4806b267f9734cf855b09afbb3f6..0000000000000000000000000000000000000000 --- a/app/views/projects/project_members/_pending.html.haml +++ /dev/null @@ -1,21 +0,0 @@ -.panel.panel-default - .panel-heading - %strong #{@project.name} - candidates - %small - (#{members.count}) - .controls - = form_tag namespace_project_project_members_path(@project.namespace, @project), method: :get, class: 'form-inline member-search-form' do - .form-group - = search_field_tag :search, params[:search], { placeholder: 'Find existing member by name', class: 'form-control', spellcheck: false } - = button_tag class: 'btn', title: 'Search' do - = icon("search") - %ul.content-list - - members.each do |project_member| - = render 'project_member', member: project_member - -:javascript - $('form.member-search-form').on('submit', function (event) { - event.preventDefault(); - Turbolinks.visit(this.action + '?' + $(this).serialize()); - }); diff --git a/app/views/projects/project_members/_project_member.html.haml b/app/views/projects/project_members/_project_member.html.haml deleted file mode 100644 index 3faf5dba8a28bc03564611da6ecbaed70cdc41db..0000000000000000000000000000000000000000 --- a/app/views/projects/project_members/_project_member.html.haml +++ /dev/null @@ -1,66 +0,0 @@ -- user = member.user -- return unless user || member.invite? - -%li{class: "#{dom_class(member)} js-toggle-container project_member_row access-#{member.human_access.downcase}", id: dom_id(member)} - %span.list-item-name - - if member.user - = image_tag avatar_icon(user, 24), class: "avatar s24", alt: '' - %strong - = link_to user.name, user_path(user) - %span.cgray= user.username - - if user == current_user - %span.label.label-success It's you - - if user.blocked? - %label.label.label-danger - %strong Blocked - - if member.request? - %span.label.label-info - Pending Approval - - else - = image_tag avatar_icon(member.invite_email, 24), class: "avatar s24", alt: '' - %strong - = member.invite_email - %span.cgray - invited - - if member.created_by - by - = link_to member.created_by.name, user_path(member.created_by) - = time_ago_with_tooltip(member.created_at) - - - if can?(current_user, :admin_project_member, @project) - = link_to resend_invite_namespace_project_project_member_path(@project.namespace, @project, member), method: :post, class: "btn-xs btn", title: 'Resend invite' do - Resend invite - - if can?(current_user, :admin_project_member, @project) - .pull-right - %strong= member.human_access - - if can?(current_user, :update_project_member, member) - = button_tag class: "btn-xs btn-grouped inline btn js-toggle-button", - title: 'Edit access level', type: 'button' do - = icon('pencil') - - if member.request? - - = link_to approval_namespace_project_project_member_path(@project.namespace, @project, member), - class: "btn-xs btn btn-success", - title: 'Grant access', type: 'button' do - %i.fa.fa-check.fa-inverse - - - if can?(current_user, :destroy_project_member, member) - - - if member.request? - = link_to namespace_project_project_member_path(@project.namespace, @project, member), data: { confirm: remove_from_project_team_message(@project, member) }, method: :delete, class: "btn-xs btn btn-remove", title: 'Deny access' do - %i.fa.fa-times.fa-inverse - - elsif current_user == user - = link_to leave_namespace_project_project_members_path(@project.namespace, @project), data: { confirm: leave_project_message(@project) }, method: :delete, class: "btn-xs btn btn-remove", title: 'Leave project' do - = icon("sign-out") - Leave - - else - = link_to namespace_project_project_member_path(@project.namespace, @project, member), data: { confirm: remove_from_project_team_message(@project, member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from team' do - = icon('trash') - - .edit-member.hide.js-toggle-content - %br - = form_for member, as: :project_member, url: namespace_project_project_member_path(@project.namespace, @project, member), remote: true do |f| - .prepend-top-10 - = f.select :access_level, options_for_select(ProjectMember.access_roles, member.access_level), {}, class: 'form-control' - .prepend-top-10 - = f.submit 'Save', class: 'btn btn-save' diff --git a/app/views/projects/project_members/_shared_group_members.html.haml b/app/views/projects/project_members/_shared_group_members.html.haml index ae13f8428f0e1a89a9d1d8165b1b9d21ce528dfd..952844acefc3997e195d30ae05f869199a4f840a 100644 --- a/app/views/projects/project_members/_shared_group_members.html.haml +++ b/app/views/projects/project_members/_shared_group_members.html.haml @@ -14,8 +14,10 @@ %i.fa.fa-pencil-square-o Edit group members %ul.content-list - - shared_group.group_members.order('access_level DESC').limit(20).each do |member| - = render 'groups/group_members/group_member', member: member, show_controls: false, show_roles: false + = render partial: 'shared/members/member', + collection: shared_group.group_members.order(access_level: :desc).limit(20), + as: :member, + locals: { show_controls: false, show_roles: false } - if shared_group_users_count > 20 %li and #{shared_group_users_count - 20} more. For full list visit #{link_to 'group members page', group_group_members_path(shared_group)} diff --git a/app/views/projects/project_members/_team.html.haml b/app/views/projects/project_members/_team.html.haml index e8dce30425f13d48bad944618d5abb6046fb626e..03207614258b08c3f914c09cb7ebd5c9f382e168 100644 --- a/app/views/projects/project_members/_team.html.haml +++ b/app/views/projects/project_members/_team.html.haml @@ -11,8 +11,7 @@ = button_tag class: 'btn', title: 'Search' do = icon("search") %ul.content-list - - members.each do |project_member| - = render 'project_member', member: project_member + = render partial: 'shared/members/member', collection: members, as: :member :javascript $('form.member-search-form').on('submit', function (event) { diff --git a/app/views/projects/project_members/index.html.haml b/app/views/projects/project_members/index.html.haml index d5a19799c49f2cc3de0f9eeb287d99ea9c437dcc..61a82724d69eb827fe5f44b2272e2071a519932d 100644 --- a/app/views/projects/project_members/index.html.haml +++ b/app/views/projects/project_members/index.html.haml @@ -12,7 +12,8 @@ %p.light Users with access to this project are listed below. = render "new_project_member" - = render "pending", members: @project_members.request + + = render "shared/members/requests", entity: @project, members: @project_members = render "team", members: @project_members.non_request diff --git a/app/views/shared/_group_or_project_home_dropdown.html.haml b/app/views/shared/_group_or_project_home_dropdown.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..fb9e63f2bd4b452105a8a21d640f646dc2af914c --- /dev/null +++ b/app/views/shared/_group_or_project_home_dropdown.html.haml @@ -0,0 +1,30 @@ +- member = entity.send(members_association(entity)).find_by(user_id: current_user.id) +- can_edit = can?(current_user, "admin_#{entity.class.to_s.underscore}".to_sym, entity) + +- if member || can_edit + .dropdown.project-settings-dropdown + %a.dropdown-new.btn.btn-gray{ href: '#', id: "#{entity.class.to_s.underscore}-settings-button", data: { toggle: 'dropdown' } } + = icon('cog') + = icon('caret-down') + %ul.dropdown-menu.dropdown-menu-align-right + - if can_edit + %li + = link_to "Edit #{entity.class.to_s}", [:edit, entity] + + - if member + %li + = link_to "Leave #{entity.class.to_s}", + leave_path(entity), + method: :delete, + data: { confirm: leave_confirmation_message(entity) } +- elsif entity.access_requested?(current_user) + = link_to 'Withdraw Request', + leave_path(entity), + data: { confirm: withdraw_request_message(entity) }, + method: :delete, + class: 'btn btn-grouped btn-gray' +- else + = link_to 'Request Access', + request_access_path(entity), + method: :post, + class: 'btn btn-grouped btn-gray' diff --git a/app/views/shared/groups/_group.html.haml b/app/views/shared/groups/_group.html.haml index a25365a94f2b9e1380f92c76e136a2da6fa79195..1ad953510056feb56e98f72dd76124d5d63aa94a 100644 --- a/app/views/shared/groups/_group.html.haml +++ b/app/views/shared/groups/_group.html.haml @@ -9,7 +9,7 @@ = link_to edit_group_path(group), class: "btn" do = icon('cogs') - = link_to leave_group_group_members_path(group), data: { confirm: leave_group_message(group.name) }, method: :delete, class: "btn", title: 'Leave this group' do + = link_to leave_group_group_members_path(group), data: { confirm: leave_confirmation_message(group) }, method: :delete, class: "btn", title: 'Leave this group' do = icon('sign-out') .stats diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..7e119155a6c416aaeebb03e526321420b65cacd0 --- /dev/null +++ b/app/views/shared/members/_member.html.haml @@ -0,0 +1,77 @@ +- show_roles = local_assigns.fetch(:show_roles, true) +- show_controls = local_assigns.fetch(:show_controls, true) +- user = member.request? ? member.created_by : member.user + +%li.js-toggle-container{ class: dom_class(member), id: dom_id(member) } + %span{ class: ("list-item-name" if show_controls) } + - if user + = image_tag avatar_icon(user, 24), class: "avatar s24", alt: '' + %strong + = link_to user.name, user_path(user) + %span.cgray= user.username + + - if user == current_user + %span.label.label-success It's you + + - if user.blocked? + %label.label.label-danger + %strong Blocked + + - if member.request? + %small + – Requested + = time_ago_with_tooltip(member.requested_at) + - else + = image_tag avatar_icon(member.invite_email, 24), class: "avatar s24", alt: '' + %strong= member.invite_email + %span.cgray + invited + - if member.created_by + by + = link_to member.created_by.name, user_path(member.created_by) + = time_ago_with_tooltip(member.created_at) + + - if show_controls && can?(current_user, action_member_permission(:admin, member), member.source) + = link_to 'Resend invite', resend_invite_member_path(member), + method: :post, + class: 'btn-xs btn' + + - if show_roles && can_see_entity_roles?(current_user, member.source) + %span.pull-right + %strong= member.human_access + - if show_controls + - if can?(current_user, action_member_permission(:update, member), member) + = button_tag icon('pencil'), + type: 'button', + class: 'btn-xs btn btn-grouped inline js-toggle-button', + title: 'Edit access level' + + - if member.request? + + = link_to icon('check inverse'), approve_request_member_path(member), + method: :post, + type: 'button', + class: 'btn-xs btn btn-success', + title: 'Grant access' + + - if can?(current_user, action_member_permission(:destroy, member), member) + + - if current_user == user + = link_to leave_path(member.source), data: { confirm: leave_confirmation_message(member.source)}, method: :delete, class: "btn-xs btn btn-remove", title: 'Remove user from group' do + = icon("sign-out") + Leave + - else + = link_to icon('trash'), member_path(member), + method: :delete, + remote: true, + data: { confirm: remove_member_message(member) }, + class: 'btn-xs btn btn-remove', + title: remove_member_title(member) + + .edit-member.hide.js-toggle-content + %br + = form_for member_path(member), as: "#{member.source.class.to_s.underscore}_member".to_sym, remote: true do |f| + .prepend-top-10 + = f.select :access_level, options_for_select(member_class(member).access_level_roles, member.access_level), {}, class: 'form-control' + .prepend-top-10 + = f.submit 'Save', class: 'btn btn-save btn-sm' diff --git a/app/views/shared/members/_requests.html.haml b/app/views/shared/members/_requests.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..ffbb380f79428982fbc0b22b6eb40b9d61ec3343 --- /dev/null +++ b/app/views/shared/members/_requests.html.haml @@ -0,0 +1,10 @@ +- requesters = members.request + +- if requesters.any? + .panel.panel-default + .panel-heading + %strong= entity.name + access requests + %small= "(#{requesters.size})" + %ul.content-list + = render partial: 'shared/members/member', collection: requesters, as: :member diff --git a/config/routes.rb b/config/routes.rb index fb35bf9dcf043ad42bdfc066e40a3bab401d5418..62c892ee9f43905ed6eef4a3a9836305fbf21a9c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -410,8 +410,15 @@ Rails.application.routes.draw do scope module: :groups do resources :group_members, only: [:index, :create, :update, :destroy] do - post :resend_invite, on: :member - delete :leave, on: :collection + collection do + delete :leave + post :request_access + end + + member do + post :resend_invite + post :approve + end end resource :avatar, only: [:destroy] @@ -778,7 +785,7 @@ Rails.application.routes.draw do member do post :resend_invite - post :approval + post :approve end end diff --git a/db/migrate/20160314114439_add_membership_request.rb b/db/migrate/20160314114439_add_membership_request.rb deleted file mode 100644 index 319b750e6c6b4efec58e4adb273cdef46e9e1ea5..0000000000000000000000000000000000000000 --- a/db/migrate/20160314114439_add_membership_request.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddMembershipRequest < ActiveRecord::Migration - def change - add_column :members, :requested, :boolean - end -end diff --git a/db/migrate/20160314114439_add_requested_at_to_members.rb b/db/migrate/20160314114439_add_requested_at_to_members.rb new file mode 100644 index 0000000000000000000000000000000000000000..273819d4cd8069f160d0a360bfa72f1610d16f03 --- /dev/null +++ b/db/migrate/20160314114439_add_requested_at_to_members.rb @@ -0,0 +1,5 @@ +class AddRequestedAtToMembers < ActiveRecord::Migration + def change + add_column :members, :requested_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index b59552fbbe7253c669256db9be8c2fcedfc4b317..f425479da1978a8c109a1ec528b2efc1789e7a59 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -536,7 +536,7 @@ ActiveRecord::Schema.define(version: 20160610301627) do t.string "invite_email" t.string "invite_token" t.datetime "invite_accepted_at" - t.boolean "requested" + t.datetime "requested_at" end add_index "members", ["access_level"], name: "index_members_on_access_level", using: :btree diff --git a/features/steps/group/members.rb b/features/steps/group/members.rb index 0706df3aec5d709e923f27d79dbd61839beb5095..9de82765df1205f88afb0ceccf7a195e1be1f9d0 100644 --- a/features/steps/group/members.rb +++ b/features/steps/group/members.rb @@ -128,9 +128,7 @@ class Spinach::Features::GroupMembers < Spinach::FeatureSteps member = mary_jane_member page.within "#group_member_#{member.id}" do - page.within '.member-access-level' do - expect(page).to have_content "Developer" - end + expect(page).to have_content "Developer" end end diff --git a/lib/api/project_members.rb b/lib/api/project_members.rb index 4aefdf319c68283ce3832dc08e0e57c93033645f..b703da0557a095a2a14f0b4338fac8d7e8a9d95d 100644 --- a/lib/api/project_members.rb +++ b/lib/api/project_members.rb @@ -46,7 +46,7 @@ module API required_attributes! [:user_id, :access_level] # either the user is already a team member or a new one - project_member = user_project.project_member_by_id(params[:user_id]) + project_member = user_project.project_member(params[:user_id]) if project_member.nil? project_member = user_project.project_members.new( user_id: params[:user_id], diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index a59865987151f57d85e0f12194a65c79ced1ab86..aea809f890b82f079c3fd11c9b438b4b95160e59 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -4,17 +4,211 @@ describe Groups::GroupMembersController do let(:user) { create(:user) } let(:group) { create(:group) } - context "index" do + describe '#index' do before do group.add_owner(user) stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) end it 'renders index with group members' do - get :index, group_id: group.path + get :index, group_id: group expect(response.status).to eq(200) expect(response).to render_template(:index) end end + + describe '#destroy' do + let(:group) { create(:group, :public) } + + context 'when member is not found' do + it 'returns 403' do + delete :destroy, group_id: group, + id: 42 + + expect(response.status).to eq(403) + end + end + + context 'when member is found' do + let(:user) { create(:user) } + let(:group_user) { create(:user) } + let(:member) do + group.add_developer(group_user) + group.group_members.find_by(user_id: group_user.id) + end + + context 'when user does not have enough rights' do + before do + group.add_developer(user) + sign_in(user) + end + + it 'returns 403' do + delete :destroy, group_id: group, + id: member + + expect(response.status).to eq(403) + expect(group.users).to include group_user + end + end + + context 'when user has enough rights' do + before do + group.add_owner(user) + sign_in(user) + end + + it '[HTML] removes user from members' do + delete :destroy, group_id: group, + id: member + + expect(response).to set_flash.to 'User was successfully removed from group.' + expect(response).to redirect_to(group_group_members_path(group)) + expect(group.users).not_to include group_user + end + + it '[JS] removes user from members' do + xhr :delete, :destroy, group_id: group, + id: member + + expect(response).to be_success + expect(group.users).not_to include group_user + end + end + end + end + + describe '#leave' do + let(:group) { create(:group, :public) } + let(:user) { create(:user) } + + context 'when member is not found' do + before { sign_in(user) } + + it 'returns 403' do + delete :leave, group_id: group + + expect(response.status).to eq(403) + end + end + + context 'when member is found' do + context 'and is not an owner' do + before do + group.add_developer(user) + sign_in(user) + end + + it 'removes user from members' do + delete :leave, group_id: group + + expect(response).to set_flash.to "You left #{group.name} group." + expect(response).to redirect_to(dashboard_groups_path) + expect(group.users).not_to include user + end + end + + context 'and is an owner' do + before do + group.add_owner(user) + sign_in(user) + end + + it 'cannot removes himself from the group' do + delete :leave, group_id: group + + expect(response).to redirect_to(dashboard_groups_path) + expect(response).to set_flash[:alert].to "You can not leave #{group.name} group because you're the last owner. Transfer or delete the group." + expect(group.users).to include user + end + end + + context 'and is a requester' do + before do + group.request_access(user) + sign_in(user) + end + + it 'removes user from members' do + delete :leave, group_id: group + + expect(response).to set_flash.to 'You withdrawn your access request to the group.' + expect(response).to redirect_to(dashboard_groups_path) + expect(group.group_members.request).to be_empty + expect(group.users).not_to include user + end + end + end + end + + describe '#request_access' do + let(:group) { create(:group, :public) } + let(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'creates a new GroupMember that is not a team member' do + post :request_access, group_id: group + + expect(response).to set_flash.to 'Your request for access has been queued for review.' + expect(response).to redirect_to(group_path(group)) + expect(group.group_members.request.find_by(created_by_id: user.id).created_by).to eq user + expect(group.users).not_to include user + end + end + + describe '#approve' do + let(:group) { create(:group, :public) } + + context 'when member is not found' do + it 'returns 403' do + post :approve, group_id: group, + id: 42 + + expect(response.status).to eq(403) + end + end + + context 'when member is found' do + let(:user) { create(:user) } + let(:group_requester) { create(:user) } + let(:member) do + group.request_access(group_requester) + group.group_members.request.find_by(created_by_id: group_requester.id) + end + + context 'when user does not have enough rights' do + before do + group.add_developer(user) + sign_in(user) + end + + it 'returns 403' do + post :approve, group_id: group, + id: member + + expect(response.status).to eq(403) + expect(group.users).not_to include group_requester + end + end + + context 'when user has enough rights' do + before do + group.add_owner(user) + sign_in(user) + end + + it 'adds user to members' do + post :approve, group_id: group, + id: member + + expect(response).to redirect_to(group_group_members_path(group)) + expect(group.users).to include group_requester + end + end + end + end end diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 750fbecdd0710518f1f3bb57f4588a5ec38095a3..2ea09f43f2665af035059865b7becbe11b3b6705 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -1,22 +1,22 @@ require('spec_helper') describe Projects::ProjectMembersController do - let(:project) { create(:project) } - let(:another_project) { create(:project, :private) } - let(:user) { create(:user) } - let(:member) { create(:user) } - - before do - project.team << [user, :master] - another_project.team << [member, :guest] - sign_in(user) - end - describe '#apply_import' do + let(:project) { create(:project) } + let(:another_project) { create(:project, :private) } + let(:user) { create(:user) } + let(:member) { create(:user) } + + before do + project.team << [user, :master] + another_project.team << [member, :guest] + sign_in(user) + end + shared_context 'import applied' do before do - post(:apply_import, namespace_id: project.namespace.to_param, - project_id: project.to_param, + post(:apply_import, namespace_id: project.namespace, + project_id: project, source_project_id: another_project.id) end end @@ -48,18 +48,231 @@ describe Projects::ProjectMembersController do end describe '#index' do - let(:project) { create(:project, :private) } - context 'when user is member' do - let(:member) { create(:user) } - before do + project = create(:project, :private) + member = create(:user) project.team << [member, :guest] sign_in(member) - get :index, namespace_id: project.namespace.to_param, project_id: project.to_param + + get :index, namespace_id: project.namespace, project_id: project end it { expect(response.status).to eq(200) } end end + + describe '#destroy' do + let(:project) { create(:project, :public) } + + context 'when member is not found' do + it 'returns 404' do + delete :destroy, namespace_id: project.namespace, + project_id: project, + id: 42 + + expect(response.status).to eq(404) + end + end + + context 'when member is found' do + let(:user) { create(:user) } + let(:team_user) { create(:user) } + let(:member) do + project.team << [team_user, :developer] + project.project_members.find_by(user_id: team_user.id) + end + + context 'when user does not have enough rights' do + before do + project.team << [user, :developer] + sign_in(user) + end + + it 'returns 404' do + delete :destroy, namespace_id: project.namespace, + project_id: project, + id: member + + expect(response.status).to eq(404) + expect(project.users).to include team_user + end + end + + context 'when user has enough rights' do + before do + project.team << [user, :master] + sign_in(user) + end + + it '[HTML] removes user from members' do + delete :destroy, namespace_id: project.namespace, + project_id: project, + id: member + + expect(response).to redirect_to( + namespace_project_project_members_path(project.namespace, project) + ) + expect(project.users).not_to include team_user + end + + it '[JS] removes user from members' do + xhr :delete, :destroy, namespace_id: project.namespace, + project_id: project, + id: member + + expect(response).to be_success + expect(project.users).not_to include team_user + end + end + end + end + + describe '#leave' do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + + context 'when member is not found' do + before { sign_in(user) } + + it 'returns 403' do + delete :leave, namespace_id: project.namespace, + project_id: project + + expect(response.status).to eq(403) + end + end + + context 'when member is found' do + context 'and is not an owner' do + before do + project.team << [user, :developer] + sign_in(user) + end + + it 'removes user from members' do + delete :leave, namespace_id: project.namespace, + project_id: project + + expect(response).to set_flash.to 'You left the project.' + expect(response).to redirect_to(dashboard_projects_path) + expect(project.users).not_to include user + end + end + + context 'and is an owner' do + before do + project.update(namespace_id: user.namespace_id) + project.team << [user, :master, user] + sign_in(user) + end + + it 'cannot removes himself from the project' do + delete :leave, namespace_id: project.namespace, + project_id: project + + expect(response).to redirect_to( + namespace_project_project_members_path(project.namespace, project) + ) + expect(response).to set_flash[:alert].to 'You can not leave your own project. Transfer or delete the project.' + expect(project.users).to include user + end + end + + context 'and is a requester' do + before do + project.request_access(user) + sign_in(user) + end + + it 'removes user from members' do + delete :leave, namespace_id: project.namespace, + project_id: project + + expect(response).to set_flash.to 'You withdrawn your access request to the project.' + expect(response).to redirect_to(dashboard_projects_path) + expect(project.project_members.request).to be_empty + expect(project.users).not_to include user + end + end + end + end + + describe '#request_access' do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'creates a new ProjectMember that is not a team member' do + post :request_access, namespace_id: project.namespace, + project_id: project + + expect(response).to set_flash.to 'Your request for access has been queued for review.' + expect(response).to redirect_to( + namespace_project_path(project.namespace, project) + ) + expect(project.project_members.request.find_by(created_by_id: user.id).created_by).to eq user + expect(project.users).not_to include user + end + end + + describe '#approve' do + let(:project) { create(:project, :public) } + + context 'when member is not found' do + it 'returns 404' do + post :approve, namespace_id: project.namespace, + project_id: project, + id: 42 + + expect(response.status).to eq(404) + end + end + + context 'when member is found' do + let(:user) { create(:user) } + let(:team_requester) { create(:user) } + let(:member) do + project.request_access(team_requester) + project.project_members.request.find_by(created_by_id: team_requester.id) + end + + context 'when user does not have enough rights' do + before do + project.team << [user, :developer] + sign_in(user) + end + + it 'returns 404' do + post :approve, namespace_id: project.namespace, + project_id: project, + id: member + + expect(response.status).to eq(404) + expect(project.users).not_to include team_requester + end + end + + context 'when user has enough rights' do + before do + project.team << [user, :master] + sign_in(user) + end + + it 'adds user to members' do + post :approve, namespace_id: project.namespace, + project_id: project, + id: member + + expect(response).to redirect_to( + namespace_project_project_members_path(project.namespace, project) + ) + expect(project.users).to include team_requester + end + end + end + end end diff --git a/spec/features/groups/members/owner_manages_access_requests_spec.rb b/spec/features/groups/members/owner_manages_access_requests_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..d5b5e0e35ea28b88bb6794ed396574fb7e66162c --- /dev/null +++ b/spec/features/groups/members/owner_manages_access_requests_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +feature 'Groups > Members > Owner manages access requests', feature: true do + let(:user) { create(:user) } + let(:owner) { create(:user) } + let(:group) { create(:group, :public) } + + background do + group.request_access(user) + group.add_owner(owner) + login_as(owner) + end + + scenario 'owner can see access requests' do + visit group_group_members_path(group) + + expect_visible_access_request(group, user) + end + + scenario 'master can grant access' do + visit group_group_members_path(group) + + expect_visible_access_request(group, user) + + perform_enqueued_jobs do + click_on 'Grant access' + end + + expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email] + expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{group.name} group was granted/ + end + + scenario 'master can deny access' do + visit group_group_members_path(group) + + expect_visible_access_request(group, user) + + perform_enqueued_jobs do + click_on 'Deny access' + end + + expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email] + expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{group.name} group was denied/ + end + + + def expect_visible_access_request(group, user) + expect(group.access_requested?(user)).to be_truthy + expect(page).to have_content "#{group.name} access requests (1)" + expect(page).to have_content user.name + end +end diff --git a/spec/features/groups/members/user_requests_access_spec.rb b/spec/features/groups/members/user_requests_access_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..9b8492807fa16c046be89836540aaa423afb528f --- /dev/null +++ b/spec/features/groups/members/user_requests_access_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +feature 'Groups > Members > User requests access', feature: true do + let(:user) { create(:user) } + let(:owner) { create(:user) } + let(:group) { create(:group, :public) } + + background do + group.add_owner(owner) + login_as(user) + end + + scenario 'user can request access to a group' do + visit group_path(group) + + perform_enqueued_jobs do + click_link 'Request Access' + end + + expect(ActionMailer::Base.deliveries.last.to).to eq [owner.notification_email] + expect(ActionMailer::Base.deliveries.last.subject).to match /Request to join #{group.name} group/ + + expect(group.access_requested?(user)).to be_truthy + expect(page).to have_content 'Your request for access has been queued for review.' + expect(page).to have_content 'Withdraw Request' + end + + scenario 'user is not listed in the group members page' do + visit group_path(group) + + click_link 'Request Access' + + expect(group.access_requested?(user)).to be_truthy + + click_link 'Members' + + visit group_group_members_path(group) + page.within('.content') do + expect(page).not_to have_content(user.name) + end + end + + scenario 'user can withdraw its request for access' do + visit group_path(group) + click_link 'Request Access' + + expect(group.access_requested?(user)).to be_truthy + + click_link 'Withdraw Request' + + expect(group.access_requested?(user)).to be_falsey + expect(page).to have_content 'You withdrawn your access request to the group.' + end +end diff --git a/spec/features/projects/members/master_manages_access_requests_spec.rb b/spec/features/projects/members/master_manages_access_requests_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..1b5490ba97f26bb8661d9b6bc0ac704298eb2d1e --- /dev/null +++ b/spec/features/projects/members/master_manages_access_requests_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +feature 'Projects > Members > Master manages access requests', feature: true do + let(:user) { create(:user) } + let(:master) { create(:user) } + let(:project) { create(:project, :public) } + + background do + project.request_access(user) + project.team << [master, :master] + login_as(master) + end + + scenario 'master can see access requests' do + visit namespace_project_project_members_path(project.namespace, project) + + expect_visible_access_request(project, user) + end + + scenario 'master can grant access' do + visit namespace_project_project_members_path(project.namespace, project) + + expect_visible_access_request(project, user) + + perform_enqueued_jobs do + click_on 'Grant access' + end + + expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email] + expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{project.name_with_namespace} project was granted/ + end + + scenario 'master can deny access' do + visit namespace_project_project_members_path(project.namespace, project) + + expect_visible_access_request(project, user) + + perform_enqueued_jobs do + click_on 'Deny access' + end + + expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email] + expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{project.name_with_namespace} project was denied/ + end + + def expect_visible_access_request(project, user) + expect(project.access_requested?(user)).to be_truthy + expect(page).to have_content "#{project.name} access requests (1)" + expect(page).to have_content user.name + end +end diff --git a/spec/features/projects/members/user_requests_access_spec.rb b/spec/features/projects/members/user_requests_access_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..58a7ec1880df2f3417df4a076da13bc90eee12da --- /dev/null +++ b/spec/features/projects/members/user_requests_access_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +feature 'Projects > Members > User requests access', feature: true do + let(:user) { create(:user) } + let(:master) { create(:user) } + let(:project) { create(:project, :public) } + + background do + project.team << [master, :master] + login_as(user) + end + + scenario 'user can request access to a project' do + visit namespace_project_path(project.namespace, project) + + perform_enqueued_jobs do + click_link 'Request Access' + end + + expect(ActionMailer::Base.deliveries.last.to).to eq [master.notification_email] + expect(ActionMailer::Base.deliveries.last.subject).to match /Request to join #{project.name_with_namespace} project/ + + expect(project.access_requested?(user)).to be_truthy + expect(page).to have_content 'Your request for access has been queued for review.' + expect(page).to have_content 'Withdraw Request' + end + + scenario 'user is not listed in the project members page' do + visit namespace_project_path(project.namespace, project) + + click_link 'Request Access' + + expect(project.access_requested?(user)).to be_truthy + + click_link 'Members' + + visit namespace_project_project_members_path(project.namespace, project) + page.within('.content') do + expect(page).not_to have_content(user.name) + end + end + + scenario 'user can withdraw its request for access' do + visit namespace_project_path(project.namespace, project) + click_link 'Request Access' + + expect(project.access_requested?(user)).to be_truthy + + click_link 'Withdraw Request' + + expect(project.access_requested?(user)).to be_falsey + expect(page).to have_content 'You withdrawn your access request to the project.' + end +end diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..f1782146241d8286e82fa10691d9757ae7ef3162 --- /dev/null +++ b/spec/helpers/members_helper_spec.rb @@ -0,0 +1,139 @@ +require 'spec_helper' + +describe MembersHelper do + describe '#member_class' do + let(:project_member) { build(:project_member) } + let(:group_member) { build(:group_member) } + + it { expect(member_class(project_member)).to eq ProjectMember } + it { expect(member_class(group_member)).to eq GroupMember } + end + + describe '#members_association' do + let(:project) { build_stubbed(:project) } + let(:group) { build_stubbed(:group) } + + it { expect(members_association(project)).to eq :project_members } + it { expect(members_association(group)).to eq :group_members } + end + + describe '#action_member_permission' do + let(:project_member) { build(:project_member) } + let(:group_member) { build(:group_member) } + + it { expect(action_member_permission(:admin, project_member)).to eq :admin_project_member } + it { expect(action_member_permission(:admin, group_member)).to eq :admin_group_member } + end + + describe '#can_see_entity_roles?' do + let(:project) { create(:project) } + let(:group) { create(:group) } + let(:user) { build(:user) } + let(:admin) { build(:user, :admin) } + let(:project_member) { create(:project_member, project: project) } + let(:group_member) { create(:group_member, group: group) } + + it { expect(can_see_entity_roles?(nil, project)).to be_falsy } + it { expect(can_see_entity_roles?(nil, group)).to be_falsy } + it { expect(can_see_entity_roles?(admin, project)).to be_truthy } + it { expect(can_see_entity_roles?(admin, group)).to be_truthy } + it { expect(can_see_entity_roles?(project_member.user, project)).to be_truthy } + it { expect(can_see_entity_roles?(group_member.user, group)).to be_truthy } + end + + describe '#member_path' do + let(:project_member) { create(:project_member) } + let(:group_member) { create(:group_member) } + + it { expect(member_path(project_member)).to eq namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) } + it { expect(member_path(group_member)).to eq group_group_member_path(group_member.source, group_member) } + it { expect { member_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' } + end + + describe '#resend_invite_member_path' do + let(:project_member) { create(:project_member) } + let(:group_member) { create(:group_member) } + + it { expect(resend_invite_member_path(project_member)).to eq resend_invite_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) } + it { expect(resend_invite_member_path(group_member)).to eq resend_invite_group_group_member_path(group_member.source, group_member) } + it { expect { resend_invite_member_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' } + end + + describe '#request_access_path' do + let(:project) { build_stubbed(:project) } + let(:group) { build_stubbed(:group) } + + it { expect(request_access_path(project)).to eq request_access_namespace_project_project_members_path(project.namespace, project) } + it { expect(request_access_path(group)).to eq request_access_group_group_members_path(group) } + it { expect { request_access_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' } + end + + describe '#approve_request_member_path' do + let(:project_member) { create(:project_member) } + let(:group_member) { create(:group_member) } + + it { expect(approve_request_member_path(project_member)).to eq approve_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) } + it { expect(approve_request_member_path(group_member)).to eq approve_group_group_member_path(group_member.source, group_member) } + it { expect { approve_request_member_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' } + end + + describe '#leave_path' do + let(:project) { build_stubbed(:project) } + let(:group) { build_stubbed(:group) } + + it { expect(leave_path(project)).to eq leave_namespace_project_project_members_path(project.namespace, project) } + it { expect(leave_path(group)).to eq leave_group_group_members_path(group) } + it { expect { leave_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' } + end + + describe '#withdraw_request_message' do + let(:project) { build_stubbed(:project) } + let(:group) { build_stubbed(:group) } + + it { expect(withdraw_request_message(project)).to eq "Are you sure you want to withdraw your access request for the \"#{project.name_with_namespace}\" project?" } + it { expect(withdraw_request_message(group)).to eq "Are you sure you want to withdraw your access request for the \"#{group.name}\" group?" } + end + + describe '#remove_member_message' do + let(:requester) { build(:user) } + let(:project) { create(:project) } + let(:project_member) { build(:project_member, project: project) } + let(:project_member_invite) { build(:project_member, project: project).tap { |m| m.generate_invite_token! } } + let(:project_member_request) { project.request_access(requester) } + let(:group) { create(:group) } + let(:group_member) { build(:group_member, group: group) } + let(:group_member_invite) { build(:group_member, group: group).tap { |m| m.generate_invite_token! } } + let(:group_member_request) { group.request_access(requester) } + + it { expect(remove_member_message(project_member)).to eq "You are going to remove #{project_member.user.name} from the #{project.name_with_namespace} project. Are you sure?" } + it { expect(remove_member_message(project_member_invite)).to eq "You are going to revoke the invitation for #{project_member_invite.invite_email} to join the #{project.name_with_namespace} project. Are you sure?" } + it { expect(remove_member_message(project_member_request)).to eq "You are going to deny #{requester.name}'s request to join the #{project.name_with_namespace} project. Are you sure?" } + it { expect(remove_member_message(group_member)).to eq "You are going to remove #{group_member.user.name} from the #{group.name} group. Are you sure?" } + it { expect(remove_member_message(group_member_invite)).to eq "You are going to revoke the invitation for #{group_member_invite.invite_email} to join the #{group.name} group. Are you sure?" } + it { expect(remove_member_message(group_member_request)).to eq "You are going to deny #{requester.name}'s request to join the #{group.name} group. Are you sure?" } + end + + describe '#remove_member_title' do + let(:requester) { build(:user) } + let(:project) { create(:project) } + let(:project_member) { build(:project_member, project: project) } + let(:project_member_request) { project.request_access(requester) } + let(:group) { create(:group) } + let(:group_member) { build(:group_member, group: group) } + let(:group_member_request) { group.request_access(requester) } + + it { expect(remove_member_title(project_member)).to eq 'Remove user' } + it { expect(remove_member_title(project_member_request)).to eq 'Deny access request' } + it { expect(remove_member_title(group_member)).to eq 'Remove user' } + it { expect(remove_member_title(group_member_request)).to eq 'Deny access request' } + end + + describe '#leave_confirmation_message' do + let(:project) { build_stubbed(:project) } + let(:group) { build_stubbed(:group) } + let(:user) { build_stubbed(:user) } + + it { expect(leave_confirmation_message(project)).to eq "Are you sure you want to leave \"#{project.name_with_namespace}\" project?" } + it { expect(leave_confirmation_message(group)).to eq "Are you sure you want to leave \"#{group.name}\" group?" } + end +end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index ac5af8740dc142c2b1b8ea3d6a5e567e978209cb..fa81c28849e37ac50d22d04ffe072bbab730c29f 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -1,6 +1,25 @@ require 'spec_helper' describe ProjectsHelper do + describe '#max_access_level' do + let(:master) { create(:user) } + let(:owner) { create(:user) } + let(:reporter) { create(:user) } + let(:group) { create(:group) } + let(:project) { build_stubbed(:empty_project, namespace: group) } + + before do + group.add_master(master) + group.add_owner(owner) + group.add_reporter(reporter) + end + + it { expect(max_access_level(project, master)).to eq 'Master' } + it { expect(max_access_level(project, owner)).to eq 'Owner' } + it { expect(max_access_level(project, reporter)).to eq 'Reporter' } + it { expect(max_access_level(project, build_stubbed(:user))).to be_nil } + end + describe "#project_status_css_class" do it "returns appropriate class" do expect(project_status_css_class("started")).to eq("active") @@ -45,16 +64,6 @@ describe ProjectsHelper do end end - describe 'user_max_access_in_project' do - let(:project) { create(:project) } - let(:user) { create(:user) } - before do - project.team.add_user(user, Gitlab::Access::MASTER) - end - - it { expect(helper.user_max_access_in_project(user.id, project)).to eq('Master') } - end - describe "readme_cache_key" do let(:project) { create(:project) } diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 818825b1477e2452ec593c63f67b1848184b22aa..2d86038030e64271f9ed5024b373f2da64a22613 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -400,6 +400,54 @@ describe Notify do end end + describe 'project access requested' do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:project_member) do + project.request_access(user) + project.project_members.find_by(created_by_id: user.id) + end + subject { Notify.project_access_requested_email(project_member.id) } + + it_behaves_like 'an email sent from GitLab' + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like "a user cannot unsubscribe through footer link" + + it 'has the correct subject' do + is_expected.to have_subject /Request to join #{project.name_with_namespace} project/ + end + + it 'contains name of project' do + is_expected.to have_body_text /#{project.name}/ + end + + it 'contains new user role' do + is_expected.to have_body_text /#{project_member.human_access}/ + end + end + + describe 'project access denied' do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:project_member) do + project.request_access(user) + project.project_members.find_by(created_by_id: user.id) + end + subject { Notify.project_access_denied_email(project.id, user.id) } + + it_behaves_like 'an email sent from GitLab' + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like "a user cannot unsubscribe through footer link" + + it 'has the correct subject' do + is_expected.to have_subject /Access to #{project.name_with_namespace} project was denied/ + end + + it 'contains name of project' do + is_expected.to have_body_text /#{project.name}/ + end + end + describe 'project access changed' do let(:project) { create(:project) } let(:user) { create(:user) } @@ -411,7 +459,7 @@ describe Notify do it_behaves_like "a user cannot unsubscribe through footer link" it 'has the correct subject' do - is_expected.to have_subject /Access to project was granted/ + is_expected.to have_subject /Access to #{project.name_with_namespace} project was granted/ end it 'contains name of project' do @@ -535,6 +583,54 @@ describe Notify do end end + describe 'group access requested' do + let(:group) { create(:group) } + let(:user) { create(:user) } + let(:group_member) do + group.request_access(user) + group.group_members.find_by(created_by_id: user.id) + end + subject { Notify.group_access_requested_email(group_member.id) } + + it_behaves_like 'an email sent from GitLab' + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like "a user cannot unsubscribe through footer link" + + it 'has the correct subject' do + is_expected.to have_subject /Request to join #{group.name} group/ + end + + it 'contains name of group' do + is_expected.to have_body_text /#{group.name}/ + end + + it 'contains new user role' do + is_expected.to have_body_text /#{group_member.human_access}/ + end + end + + describe 'group access denied' do + let(:group) { create(:group) } + let(:user) { create(:user) } + let(:group_member) do + group.request_access(user) + group.group_members.find_by(created_by_id: user.id) + end + subject { Notify.group_access_denied_email(group.id, user.id) } + + it_behaves_like 'an email sent from GitLab' + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like "a user cannot unsubscribe through footer link" + + it 'has the correct subject' do + is_expected.to have_subject /Access to #{group.name} group was denied/ + end + + it 'contains name of group' do + is_expected.to have_body_text /#{group.name}/ + end + end + describe 'group access changed' do let(:group) { create(:group) } let(:user) { create(:user) } @@ -547,7 +643,7 @@ describe Notify do it_behaves_like "a user cannot unsubscribe through footer link" it 'has the correct subject' do - is_expected.to have_subject /Access to group was granted/ + is_expected.to have_subject /Access to #{group.name} group was granted/ end it 'contains name of project' do diff --git a/spec/models/concerns/access_requestable_spec.rb b/spec/models/concerns/access_requestable_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..2dfed1eb4c4227ac342fa7b2e557b5b4665d9f4f --- /dev/null +++ b/spec/models/concerns/access_requestable_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +describe AccessRequestable do + describe 'Group' do + describe '#request_access' do + let(:group) { create(:group, :public) } + let(:user) { create(:user) } + + it { expect(group.request_access(user)).to be_a(GroupMember) } + it { expect(group.request_access(user).user).to be_nil } + it { expect(group.request_access(user).created_by).to eq(user) } + end + + describe '#access_requested?' do + let(:group) { create(:group, :public) } + let(:user) { create(:user) } + + before { group.request_access(user) } + + it { expect(group.access_requested?(user)).to be_truthy } + end + end + + describe 'Project' do + describe '#request_access' do + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + + it { expect(project.request_access(user)).to be_a(ProjectMember) } + end + + describe '#access_requested?' do + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + + before { project.request_access(user) } + + it { expect(project.access_requested?(user)).to be_truthy } + end + end +end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 6fa16be7f045bb1e6be3e77800955dc820e6f168..52f9d57bc0a5842e43511ce22a915c321c6bb81e 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -5,7 +5,22 @@ describe Group, models: true do describe 'associations' do it { is_expected.to have_many :projects } - it { is_expected.to have_many :group_members } + it { is_expected.to have_many(:group_members).dependent(:destroy) } + it { is_expected.to have_many(:users).through(:group_members) } + it { is_expected.to have_many(:project_group_links).dependent(:destroy) } + it { is_expected.to have_many(:shared_projects).through(:project_group_links) } + it { is_expected.to have_many(:notification_settings).dependent(:destroy) } + + describe '#group_members' do + let(:user) { create(:user) } + let(:group) { create(:group) } + + before { group.request_access(user) } + + it 'does not includes membership requests' do + expect(user.group_members).to be_empty + end + end end describe 'modules' do @@ -131,4 +146,46 @@ describe Group, models: true do expect(described_class.search(group.path.upcase)).to eq([group]) end end + + describe '#has_owner?' do + before { @members = setup_group_members(group) } + + it { expect(group.has_owner?(@members[:owner])).to be_truthy } + it { expect(group.has_owner?(@members[:master])).to be_falsey } + it { expect(group.has_owner?(@members[:developer])).to be_falsey } + it { expect(group.has_owner?(@members[:reporter])).to be_falsey } + it { expect(group.has_owner?(@members[:guest])).to be_falsey } + it { expect(group.has_owner?(@members[:requester])).to be_falsey } + end + + describe '#has_master?' do + before { @members = setup_group_members(group) } + + it { expect(group.has_master?(@members[:owner])).to be_falsey } + it { expect(group.has_master?(@members[:master])).to be_truthy } + it { expect(group.has_master?(@members[:developer])).to be_falsey } + it { expect(group.has_master?(@members[:reporter])).to be_falsey } + it { expect(group.has_master?(@members[:guest])).to be_falsey } + it { expect(group.has_master?(@members[:requester])).to be_falsey } + end + + def setup_group_members(group) + members = { + owner: create(:user), + master: create(:user), + developer: create(:user), + reporter: create(:user), + guest: create(:user), + requester: create(:user) + } + + group.add_user(members[:owner], GroupMember::OWNER) + group.add_user(members[:master], GroupMember::MASTER) + group.add_user(members[:developer], GroupMember::DEVELOPER) + group.add_user(members[:reporter], GroupMember::REPORTER) + group.add_user(members[:guest], GroupMember::GUEST) + group.request_access(members[:requester]) + + members + end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 6e51730eecd207ff08e016b9c28913cd9d8fdb89..a3d525d8d565a06196e3f9ca3df35759970af648 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -55,6 +55,47 @@ describe Member, models: true do end end + describe 'Scopes' do + before do + project = create(:project) + @invited_member = build(:project_member, user: nil).tap { |m| m.generate_invite_token! } + @accepted_invite_member = build(:project_member, user: nil).tap { |m| m.generate_invite_token! && m.accept_invite!(build(:user)) } + + requested_user = create(:user).tap { |u| project.request_access(u) } + @requested_member = project.project_members.find_by(created_by_id: requested_user.id) + accepted_request_user = create(:user).tap { |u| project.request_access(u) } + @accepted_request_member = project.project_members.find_by(created_by_id: accepted_request_user.id).tap { |m| m.accept_request } + end + + describe '#invite' do + it { expect(described_class.invite).to include @invited_member } + it { expect(described_class.invite).not_to include @accepted_invite_member } + it { expect(described_class.invite).not_to include @requested_member } + it { expect(described_class.invite).not_to include @accepted_request_member } + end + + describe '#request' do + it { expect(described_class.request).not_to include @invited_member } + it { expect(described_class.request).not_to include @accepted_invite_member } + it { expect(described_class.request).to include @requested_member } + it { expect(described_class.request).not_to include @accepted_request_member } + end + + describe '#non_request' do + it { expect(described_class.non_request).to include @invited_member } + it { expect(described_class.non_request).to include @accepted_invite_member } + it { expect(described_class.non_request).not_to include @requested_member } + it { expect(described_class.non_request).to include @accepted_request_member } + end + + describe '#non_pending' do + it { expect(described_class.non_pending).not_to include @invited_member } + it { expect(described_class.non_pending).to include @accepted_invite_member } + it { expect(described_class.non_pending).not_to include @requested_member } + it { expect(described_class.non_pending).to include @accepted_request_member } + end + end + describe "Delegate methods" do it { is_expected.to respond_to(:user_name) } it { is_expected.to respond_to(:user_email) } @@ -97,6 +138,54 @@ describe Member, models: true do end end + describe '#accept_request' do + let(:user) { create(:user) } + let(:member) { create(:project_member, requested_at: Time.now.utc, user: nil, created_by: user) } + + it 'returns true' do + expect(member.accept_request).to be_truthy + end + + it 'sets the user' do + member.accept_request + + expect(member.user).to eq(user) + end + + it 'clears requested_at' do + member.accept_request + + expect(member.requested_at).to be_nil + end + + it 'calls #after_accept_request' do + expect(member).to receive(:after_accept_request) + + member.accept_request + end + end + + describe '#decline_request' do + let(:user) { create(:user) } + let(:member) { create(:project_member, requested_at: Time.now.utc, user: nil, created_by: user) } + + it 'returns true' do + expect(member.decline_request).to be_truthy + end + + it 'destroys the member' do + member.decline_request + + expect(member).to be_destroyed + end + + it 'calls #after_decline_request' do + expect(member).to receive(:after_decline_request) + + member.decline_request + end + end + describe "#accept_invite!" do let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } let(:user) { create(:user) } diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 5424c9b9cba9fb4dc96b6aa40dee2597b45ce4b7..c3070d4cb7831303f6ab3a40585ad14c1a3a9e63 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -20,7 +20,7 @@ require 'spec_helper' describe GroupMember, models: true do - context 'notification' do + describe 'notifications' do describe "#after_create" do it "should send email to user" do membership = build(:group_member) @@ -50,5 +50,25 @@ describe GroupMember, models: true do @group_member.update_attribute(:access_level, GroupMember::OWNER) end end + + describe 'after accept_request' do + let(:member) { create(:group_member, user: nil, created_by: build_stubbed(:user), requested_at: Time.now) } + + it "calls #accept_group_access_request" do + expect_any_instance_of(NotificationService).to receive(:new_group_member) + + member.accept_request + end + end + + describe 'after decline_request' do + let(:member) { create(:group_member, user: nil, created_by: build_stubbed(:user), requested_at: Time.now) } + + it "calls #decline_group_access_request" do + expect_any_instance_of(NotificationService).to receive(:decline_group_access_request) + + member.decline_request + end + end end end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 9f13874b532acaf0b36c12e558ed08f8ae6df94c..99b3c77c6cd255a23bd2539c741cb701670ce9e5 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -135,4 +135,26 @@ describe ProjectMember, models: true do it { expect(@project_1.users).to be_empty } it { expect(@project_2.users).to be_empty } end + + describe 'notifications' do + describe 'after accept_request' do + let(:member) { create(:project_member, user: nil, created_by: build_stubbed(:user), requested_at: Time.now) } + + it 'calls #accept_project_access_request' do + expect_any_instance_of(NotificationService).to receive(:new_project_member) + + member.accept_request + end + end + + describe 'after decline_request' do + let(:member) { create(:project_member, user: nil, created_by: build_stubbed(:user), requested_at: Time.now) } + + it 'calls #decline_project_access_request' do + expect_any_instance_of(NotificationService).to receive(:decline_project_access_request) + + member.decline_request + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index de8815f5a38271560ccf24602e7562edc477715c..d5a4b73affd434eb71cc5a81a712c45b8ea38ba0 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -29,6 +29,17 @@ describe Project, models: true do it { is_expected.to have_many(:variables) } it { is_expected.to have_many(:triggers) } it { is_expected.to have_many(:todos).dependent(:destroy) } + + describe '#project_members' do + let(:user) { create(:user) } + let(:project) { create(:project) } + + before { project.request_access(user) } + + it 'does not includes membership requests' do + expect(user.project_members).to be_empty + end + end end describe 'modules' do diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 8bebd6a944721de4fa468e25df124bbd18a3f57f..36b1f439955c6ee6d16f898c04ed3af54fbfe7f2 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -73,69 +73,107 @@ describe ProjectTeam, models: true do end end - describe :max_invited_level do - let(:group) { create(:group) } - let(:project) { create(:empty_project) } - - before do - project.project_group_links.create( - group: group, - group_access: Gitlab::Access::DEVELOPER - ) - - group.add_user(master, Gitlab::Access::MASTER) - group.add_user(reporter, Gitlab::Access::REPORTER) + describe '#find_member' do + context 'personal project' do + let(:project) { create(:empty_project) } + let(:requester) { create(:user) } + + before do + project.team << [master, :master] + project.team << [reporter, :reporter] + project.team << [guest, :guest] + project.request_access(requester) + end + + it { expect(project.team.find_member(master.id)).to be_a(ProjectMember) } + it { expect(project.team.find_member(reporter.id)).to be_a(ProjectMember) } + it { expect(project.team.find_member(guest.id)).to be_a(ProjectMember) } + it { expect(project.team.find_member(nonmember.id)).to be_nil } + it { expect(project.team.find_member(requester.id)).to be_nil } end - it { expect(project.team.max_invited_level(master.id)).to eq(Gitlab::Access::DEVELOPER) } - it { expect(project.team.max_invited_level(reporter.id)).to eq(Gitlab::Access::REPORTER) } - it { expect(project.team.max_invited_level(nonmember.id)).to be_nil } - end - - describe :max_member_access do - let(:group) { create(:group) } - let(:project) { create(:empty_project) } - - before do - project.project_group_links.create( - group: group, - group_access: Gitlab::Access::DEVELOPER - ) - - group.add_user(master, Gitlab::Access::MASTER) - group.add_user(reporter, Gitlab::Access::REPORTER) - end - - it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::DEVELOPER) } - it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } - it { expect(project.team.max_member_access(nonmember.id)).to be_nil } - - it "does not have an access" do - project.namespace.update(share_with_group_lock: true) - expect(project.team.max_member_access(master.id)).to be_nil - expect(project.team.max_member_access(reporter.id)).to be_nil + context 'group project' do + let(:group) { create(:group) } + let(:project) { create(:empty_project, group: group) } + let(:requester) { create(:user) } + + before do + group.add_master(master) + group.add_reporter(reporter) + group.add_guest(guest) + group.request_access(requester) + end + + it { expect(project.team.find_member(master.id)).to be_a(GroupMember) } + it { expect(project.team.find_member(reporter.id)).to be_a(GroupMember) } + it { expect(project.team.find_member(guest.id)).to be_a(GroupMember) } + it { expect(project.team.find_member(nonmember.id)).to be_nil } + it { expect(project.team.find_member(requester.id)).to be_nil } end end - describe "#human_max_access" do - it 'returns Master role' do - user = create(:user) - group = create(:group) - group.add_master(user) - - project = build_stubbed(:empty_project, namespace: group) - - expect(project.team.human_max_access(user.id)).to eq 'Master' + describe '#max_member_access' do + let(:requester) { create(:user) } + + context 'personal project' do + let(:project) { create(:empty_project) } + + context 'when project is not shared with group' do + before do + project.team << [master, :master] + project.team << [reporter, :reporter] + project.team << [guest, :guest] + project.request_access(requester) + end + + it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::MASTER) } + it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } + it { expect(project.team.max_member_access(guest.id)).to eq(Gitlab::Access::GUEST) } + it { expect(project.team.max_member_access(nonmember.id)).to be_nil } + it { expect(project.team.max_member_access(requester.id)).to be_nil } + end + + context 'when project is shared with group' do + before do + group = create(:group) + project.project_group_links.create( + group: group, + group_access: Gitlab::Access::DEVELOPER) + + group.add_master(master) + group.add_reporter(reporter) + end + + it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::DEVELOPER) } + it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } + it { expect(project.team.max_member_access(nonmember.id)).to be_nil } + it { expect(project.team.max_member_access(requester.id)).to be_nil } + + context 'but share_with_group_lock is true' do + before { project.namespace.update(share_with_group_lock: true) } + + it { expect(project.team.max_member_access(master.id)).to be_nil } + it { expect(project.team.max_member_access(reporter.id)).to be_nil } + end + end end - it 'returns Owner role' do - user = create(:user) - group = create(:group) - group.add_owner(user) - - project = build_stubbed(:empty_project, namespace: group) - - expect(project.team.human_max_access(user.id)).to eq 'Owner' + context 'group project' do + let(:group) { create(:group) } + let(:project) { create(:empty_project, group: group) } + + before do + group.add_master(master) + group.add_reporter(reporter) + group.add_guest(guest) + group.request_access(requester) + end + + it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::MASTER) } + it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } + it { expect(project.team.max_member_access(guest.id)).to eq(Gitlab::Access::GUEST) } + it { expect(project.team.max_member_access(nonmember.id)).to be_nil } + it { expect(project.team.max_member_access(requester.id)).to be_nil } end end end