summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/assets/stylesheets/pages/groups.scss17
-rw-r--r--app/assets/stylesheets/pages/projects.scss17
-rw-r--r--app/controllers/concerns/access_request_actions.rb38
-rw-r--r--app/controllers/concerns/membership_actions.rb58
-rw-r--r--app/controllers/groups/group_members_controller.rb42
-rw-r--r--app/controllers/projects/project_members_controller.rb43
-rw-r--r--app/helpers/gitlab_routing_helper.rb77
-rw-r--r--app/helpers/members_helper.rb126
-rw-r--r--app/helpers/projects_helper.rb4
-rw-r--r--app/mailers/emails/members.rb89
-rw-r--r--app/mailers/notify.rb2
-rw-r--r--app/models/ability.rb4
-rw-r--r--app/models/concerns/access_requestable.rb13
-rw-r--r--app/models/group.rb2
-rw-r--r--app/models/member.rb35
-rw-r--r--app/models/members/group_member.rb9
-rw-r--r--app/models/members/project_member.rb4
-rw-r--r--app/models/project.rb4
-rw-r--r--app/models/project_team.rb18
-rw-r--r--app/services/notification_service.rb30
-rw-r--r--app/views/groups/group_members/index.html.haml2
-rw-r--r--app/views/groups/show.html.haml3
-rw-r--r--app/views/layouts/nav/_group_settings.html.haml17
-rw-r--r--app/views/layouts/nav/_project.html.haml36
-rw-r--r--app/views/notify/group_access_granted_email.html.haml3
-rw-r--r--app/views/notify/group_access_granted_email.text.erb3
-rw-r--r--app/views/notify/group_invite_accepted_email.html.haml6
-rw-r--r--app/views/notify/group_invite_accepted_email.text.erb3
-rw-r--r--app/views/notify/group_invite_declined_email.html.haml5
-rw-r--r--app/views/notify/group_invite_declined_email.text.erb3
-rw-r--r--app/views/notify/group_member_invited_email.html.haml14
-rw-r--r--app/views/notify/group_member_invited_email.text.erb4
-rw-r--r--app/views/notify/member_access_denied_email.html.haml2
-rw-r--r--app/views/notify/member_access_denied_email.text.erb4
-rw-r--r--app/views/notify/member_access_granted_email.html.haml2
-rw-r--r--app/views/notify/member_access_granted_email.text.erb4
-rw-r--r--app/views/notify/member_access_requested_email.html.haml4
-rw-r--r--app/views/notify/member_access_requested_email.text.erb4
-rw-r--r--app/views/notify/member_invite_accepted_email.html.haml2
-rw-r--r--app/views/notify/member_invite_accepted_email.text.erb4
-rw-r--r--app/views/notify/member_invite_declined_email.html.haml2
-rw-r--r--app/views/notify/member_invite_declined_email.text.erb4
-rw-r--r--app/views/notify/member_invited_email.html.haml8
-rw-r--r--app/views/notify/member_invited_email.text.erb2
-rw-r--r--app/views/projects/_home_panel.html.haml11
-rw-r--r--app/views/projects/buttons/_notifications.html.haml2
-rw-r--r--app/views/projects/notes/_note.html.haml2
-rw-r--r--app/views/projects/project_members/_group_members.html.haml10
-rw-r--r--app/views/projects/project_members/index.html.haml4
-rw-r--r--app/views/shared/_group_or_project_home_dropdown.html.haml30
-rw-r--r--app/views/shared/members/_access_request_buttons.html.haml12
-rw-r--r--app/views/shared/members/_member.html.haml28
-rw-r--r--app/views/shared/members/_requests.html.haml10
-rw-r--r--features/steps/dashboard/group.rb2
-rw-r--r--features/steps/group/members.rb10
-rw-r--r--features/steps/project/team_management.rb26
-rw-r--r--lib/api/entities.rb5
-rw-r--r--spec/controllers/groups/group_members_controller_spec.rb24
-rw-r--r--spec/controllers/projects/project_members_controller_spec.rb30
-rw-r--r--spec/features/groups/members/owner_manages_access_requests_spec.rb14
-rw-r--r--spec/features/groups/members/user_requests_access_spec.rb28
-rw-r--r--spec/features/projects/members/master_manages_access_requests_spec.rb14
-rw-r--r--spec/features/projects/members/user_requests_access_spec.rb32
-rw-r--r--spec/helpers/gitlab_routing_helper_spec.rb79
-rw-r--r--spec/helpers/members_helper_spec.rb111
-rw-r--r--spec/helpers/projects_helper_spec.rb19
-rw-r--r--spec/mailers/notify_spec.rb132
-rw-r--r--spec/models/concerns/access_requestable_spec.rb7
-rw-r--r--spec/models/group_spec.rb11
-rw-r--r--spec/models/member_spec.rb103
-rw-r--r--spec/models/members/group_member_spec.rb22
-rw-r--r--spec/models/members/project_member_spec.rb22
-rw-r--r--spec/models/project_spec.rb19
-rw-r--r--spec/models/project_team_spec.rb22
74 files changed, 775 insertions, 839 deletions
diff --git a/app/assets/stylesheets/pages/groups.scss b/app/assets/stylesheets/pages/groups.scss
index ec6c099df5b..ac7721cbe15 100644
--- a/app/assets/stylesheets/pages/groups.scss
+++ b/app/assets/stylesheets/pages/groups.scss
@@ -39,3 +39,20 @@
}
}
}
+
+.groups-cover-block {
+
+ .container-fluid {
+ position: relative;
+ }
+
+ .access-request-button {
+ @include btn-gray;
+ position: absolute;
+ right: 16px;
+ bottom: 32px;
+ padding: 3px 10px;
+ text-transform: none;
+ background-color: $background-color;
+ }
+}
diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss
index 2505deaf757..0e4cefc55c2 100644
--- a/app/assets/stylesheets/pages/projects.scss
+++ b/app/assets/stylesheets/pages/projects.scss
@@ -229,13 +229,20 @@
right: 16px;
bottom: 0;
- .btn {
- padding: 3px 10px;
- background-color: $background-color;
+ @media (max-width: $screen-lg-min) {
+ top: 0;
}
- @media (max-width: 1304px) {
- top: 0;
+ .access-request-button {
+ position: absolute;
+ right: 0;
+ bottom: 61px;
+
+ @media (max-width: $screen-lg-min) {
+ position: relative;
+ bottom: 0;
+ margin-right: 10px;
+ }
}
}
diff --git a/app/controllers/concerns/access_request_actions.rb b/app/controllers/concerns/access_request_actions.rb
deleted file mode 100644
index c4d22749d6a..00000000000
--- a/app/controllers/concerns/access_request_actions.rb
+++ /dev/null
@@ -1,38 +0,0 @@
-module AccessRequestActions
- extend ActiveSupport::Concern
-
- def request_access
- access_requestable_resource.request_access(current_user)
-
- redirect_to access_requestable_resource_path,
- notice: 'Your request for access has been queued for review.'
- end
-
- def approve_access_request
- @member = access_requestable_resource.public_send(member_entity_name.pluralize).request.find(params[:id])
-
- return render_403 unless can?(current_user, :"update_#{member_entity_name}", @member)
-
- @member.accept_request
-
- redirect_to access_requestable_resource_members_path
- end
-
- protected
-
- def access_requestable_resource
- raise NotImplementedError
- end
-
- def access_requestable_resource_path
- access_requestable_resource
- end
-
- def access_requestable_resource_members_path
- [access_requestable_resource, 'members']
- end
-
- def member_entity_name
- "#{access_requestable_resource.class.to_s.underscore}_member"
- end
-end
diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb
new file mode 100644
index 00000000000..a24273fad0b
--- /dev/null
+++ b/app/controllers/concerns/membership_actions.rb
@@ -0,0 +1,58 @@
+module MembershipActions
+ extend ActiveSupport::Concern
+ include MembersHelper
+
+ def request_access
+ membershipable.request_access(current_user)
+
+ redirect_to polymorphic_path(membershipable),
+ notice: 'Your request for access has been queued for review.'
+ end
+
+ def approve_access_request
+ @member = membershipable.members.request.find(params[:id])
+
+ return render_403 unless can?(current_user, action_member_permission(:update, @member), @member)
+
+ @member.accept_request
+
+ redirect_to polymorphic_url([membershipable, :members])
+ end
+
+ def leave
+ @member = membershipable.members.find_by(user_id: current_user)
+ return render_403 unless @member
+
+ source_type = @member.real_source_type.humanize(capitalize: false)
+
+ if can?(current_user, action_member_permission(:destroy, @member), @member)
+ notice =
+ if @member.request?
+ "Your access request to the #{source_type} has been withdrawn."
+ else
+ "You left the \"#{@member.source.human_name}\" #{source_type}."
+ end
+ @member.destroy
+
+ redirect_to [:dashboard, @member.real_source_type.tableize], notice: notice
+ else
+ if cannot_leave?
+ alert = "You can not leave the \"#{@member.source.human_name}\" #{source_type}."
+ alert << " Transfer or delete the #{source_type}."
+ redirect_to polymorphic_url(membershipable), alert: alert
+ else
+ render_403
+ end
+ end
+ end
+
+ protected
+
+ def membershipable
+ raise NotImplementedError
+ end
+
+ def cannot_leave?
+ raise NotImplementedError
+ end
+end
diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb
index a37129062f9..d0f2e2949f0 100644
--- a/app/controllers/groups/group_members_controller.rb
+++ b/app/controllers/groups/group_members_controller.rb
@@ -1,5 +1,5 @@
class Groups::GroupMembersController < Groups::ApplicationController
- include AccessRequestActions
+ include MembershipActions
# Authorize
before_action :authorize_admin_group_member!, except: [:index, :leave, :request_access]
@@ -38,7 +38,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
return render_403 unless can?(current_user, :destroy_group_member, @group_member)
- @group_member.request? ? @group_member.decline_request : @group_member.destroy
+ @group_member.destroy
respond_to do |format|
format.html { redirect_to group_group_members_path(@group), notice: 'User was successfully removed from group.' }
@@ -60,46 +60,16 @@ class Groups::GroupMembersController < Groups::ApplicationController
end
end
- def leave
- @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: 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.")
- else
- return render_403
- end
- end
- end
-
protected
def member_params
params.require(:group_member).permit(:access_level, :user_id)
end
- # AccessRequestActions concern
- def access_requestable_resource
- @group
- end
-
- def access_requestable_resource_path
- group_path(@group)
- end
+ # MembershipActions concern
+ alias_method :membershipable, :group
- def access_requestable_resource_members_path
- group_group_members_path(@group)
+ def cannot_leave?
+ @group.last_owner?(current_user)
end
end
diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb
index c61eda95bc7..35d067cd029 100644
--- a/app/controllers/projects/project_members_controller.rb
+++ b/app/controllers/projects/project_members_controller.rb
@@ -1,5 +1,5 @@
class Projects::ProjectMembersController < Projects::ApplicationController
- include AccessRequestActions
+ include MembershipActions
# Authorize
before_action :authorize_admin_project_member!, except: [:index, :leave, :request_access]
@@ -52,7 +52,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
return render_403 unless can?(current_user, :destroy_project_member, @project_member)
- @project_member.request? ? @project_member.decline_request : @project_member.destroy
+ @project_member.destroy
respond_to do |format|
format.html do
@@ -76,31 +76,6 @@ class Projects::ProjectMembersController < Projects::ApplicationController
end
end
- def leave
- @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
-
- 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.'
- redirect_back_or_default(default: { action: 'index' }, options: { alert: message })
- else
- render_403
- end
- end
- end
-
def apply_import
source_project = Project.find(params[:source_project_id])
@@ -121,16 +96,10 @@ class Projects::ProjectMembersController < Projects::ApplicationController
params.require(:project_member).permit(:user_id, :access_level)
end
- # AccessRequestActions concern
- def access_requestable_resource
- @project
- end
-
- def access_requestable_resource_path
- namespace_project_path(@project.namespace, @project)
- end
+ # MembershipActions concern
+ alias_method :membershipable, :project
- def access_requestable_resource_members_path
- namespace_project_project_members_path(@project.namespace, @project)
+ def cannot_leave?
+ current_user == @project.owner
end
end
diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb
index 2ce2d4e694f..3a43e936aee 100644
--- a/app/helpers/gitlab_routing_helper.rb
+++ b/app/helpers/gitlab_routing_helper.rb
@@ -13,10 +13,23 @@
# merge_request_path(merge_request)
#
module GitlabRoutingHelper
+ # Project
def project_path(project, *args)
namespace_project_path(project.namespace, project, *args)
end
+ def project_url(project, *args)
+ namespace_project_url(project.namespace, project, *args)
+ end
+
+ def edit_project_path(project, *args)
+ edit_namespace_project_path(project.namespace, project, *args)
+ end
+
+ def edit_project_url(project, *args)
+ edit_namespace_project_url(project.namespace, project, *args)
+ end
+
def project_files_path(project, *args)
namespace_project_tree_path(project.namespace, project, @ref || project.repository.root_ref)
end
@@ -41,10 +54,6 @@ module GitlabRoutingHelper
activity_namespace_project_path(project.namespace, project, *args)
end
- def edit_project_path(project, *args)
- edit_namespace_project_path(project.namespace, project, *args)
- end
-
def runners_path(project, *args)
namespace_project_runners_path(project.namespace, project, *args)
end
@@ -65,14 +74,6 @@ module GitlabRoutingHelper
namespace_project_milestone_path(entity.project.namespace, entity.project, entity, *args)
end
- def project_url(project, *args)
- namespace_project_url(project.namespace, project, *args)
- end
-
- def edit_project_url(project, *args)
- edit_namespace_project_url(project.namespace, project, *args)
- end
-
def issue_url(entity, *args)
namespace_project_issue_url(entity.project.namespace, entity.project, entity, *args)
end
@@ -92,4 +93,56 @@ module GitlabRoutingHelper
toggle_subscription_namespace_project_merge_request_path(entity.project.namespace, entity.project, entity)
end
end
+
+ ## Members
+ def project_members_url(project, *args)
+ namespace_project_project_members_url(project.namespace, project)
+ end
+
+ def project_member_path(project_member, *args)
+ namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member)
+ end
+
+ def request_access_project_members_path(project, *args)
+ request_access_namespace_project_project_members_path(project.namespace, project)
+ end
+
+ def leave_project_members_path(project, *args)
+ leave_namespace_project_project_members_path(project.namespace, project)
+ end
+
+ def approve_access_request_project_member_path(project_member, *args)
+ approve_access_request_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member)
+ end
+
+ def resend_invite_project_member_path(project_member, *args)
+ resend_invite_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member)
+ end
+
+ # Groups
+
+ ## Members
+ def group_members_url(group, *args)
+ group_group_members_url(group, *args)
+ end
+
+ def group_member_path(group_member, *args)
+ group_group_member_path(group_member.source, group_member)
+ end
+
+ def request_access_group_members_path(group, *args)
+ request_access_group_group_members_path(group)
+ end
+
+ def leave_group_members_path(group, *args)
+ leave_group_group_members_path(group)
+ end
+
+ def approve_access_request_group_member_path(group_member, *args)
+ approve_access_request_group_group_member_path(group_member.source, group_member)
+ end
+
+ def resend_invite_group_member_path(group_member, *args)
+ resend_invite_group_group_member_path(group_member.source, group_member)
+ end
end
diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb
index bd84b8b239f..a53828ef4e7 100644
--- a/app/helpers/members_helper.rb
+++ b/app/helpers/members_helper.rb
@@ -1,117 +1,45 @@
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
-
+ # Returns a `<action>_<source>_member` association, e.g.:
+ # - admin_project_member, update_project_member, destroy_project_member
+ # - admin_group_member, update_group_member, destroy_group_member
def action_member_permission(action, member)
- "#{action}_#{member.source.class.to_s.underscore}_member".to_sym
+ "#{action}_#{member.type.underscore}".to_sym
end
- def can_see_entity_roles?(user, entity)
+ def can_see_member_roles?(source:, user: nil)
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
+ user.is_admin? || source.members.exists?(user_id: user.id)
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 remove_member_message(member, user: nil)
+ user = current_user if defined?(current_user)
- 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_access_request_namespace_project_project_member_path(member.source.namespace, member.source, member)
- when Group
- approve_access_request_group_group_member_path(member.source, member)
- else
- raise ArgumentError.new('Unknown object class')
- end
- end
+ text = 'Are you sure you want to '
+ action =
+ if member.request?
+ if member.user == user
+ 'withdraw your access request for'
+ else
+ "deny #{member.user.name}'s request to join"
+ end
+ elsif member.invite?
+ "revoke the invitation for #{member.invite_email} to join"
+ else
+ "remove #{member.user.name} from"
+ 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
+ text << action << " the #{member.source.human_name} #{member.real_source_type.humanize(capitalize: false)}?"
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
+ text = " from #{member.real_source_type.humanize(capitalize: false)}"
- def entity_type(entity)
- entity.class.to_s.underscore
+ text.prepend(member.request? ? 'Deny access request' : 'Remove user')
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
+ def leave_confirmation_message(member_source)
+ "Are you sure you want to leave the " \
+ "\"#{member_source.human_name}\" #{member_source.class.to_s.humanize(capitalize: false)}?"
end
end
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 03941f87b13..d30dd66202b 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -1,8 +1,4 @@
module ProjectsHelper
- 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)
link_to [project.namespace.becomes(Namespace), project], title: h(project.name) do
title = content_tag(:span, project.name, class: 'project-name')
diff --git a/app/mailers/emails/members.rb b/app/mailers/emails/members.rb
index 5fd55c149df..6dde2e9847d 100644
--- a/app/mailers/emails/members.rb
+++ b/app/mailers/emails/members.rb
@@ -1,104 +1,81 @@
module Emails
module Members
extend ActiveSupport::Concern
+ include MembersHelper
included do
- attr_reader :member_target_type
- helper_method :member, :access_requester, :member_target_type, :member_target_name, :member_target_url
+ helper_method :member_source, :member
end
- def member_access_requested_email(member_target_type, member_id)
- @member_target_type = member_target_type
+ def member_access_requested_email(member_source_type, member_id)
+ @member_source_type = member_source_type
@member_id = member_id
- admins = User.where(id: target.public_send(members_association).admins.pluck(:user_id)).pluck(:notification_email)
+ admins = member_source.members.owners_and_masters.includes(:user).pluck(:notification_email)
mail(to: admins,
- subject: subject("Request to join the #{member_target_name} #{member_target_type}"))
+ subject: subject("Request to join the #{member_source.human_name} #{member_source.model_name.singular}"))
end
- def member_access_granted_email(member_target_type, member_id)
- @member_target_type = member_target_type
+ def member_access_granted_email(member_source_type, member_id)
+ @member_source_type = member_source_type
@member_id = member_id
mail(to: member.user.notification_email,
- subject: subject("Access to the #{member_target_name} #{member_target_type} was granted"))
+ subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was granted"))
end
- def member_access_denied_email(member_target_type, target_id, user_id)
- @member_target_type = member_target_type
- @target = target_class.find(target_id)
+ def member_access_denied_email(member_source_type, source_id, user_id)
+ @member_source_type = member_source_type
+ @member_source = member_source_class.find(source_id)
+ requester = User.find(user_id)
- mail(to: User.find(user_id).notification_email,
- subject: subject("Access to the #{member_target_name} #{member_target_type} was denied"))
+ mail(to: requester.notification_email,
+ subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was denied"))
end
- def member_invited_email(member_target_type, member_id, token)
- @member_target_type = member_target_type
+ def member_invited_email(member_source_type, member_id, token)
+ @member_source_type = member_source_type
@member_id = member_id
@token = token
mail(to: member.invite_email,
- subject: "Invitation to join the #{member_target_name} #{member_target_type}")
+ subject: "Invitation to join the #{member_source.human_name} #{member_source.model_name.singular}")
end
- def member_invite_accepted_email(member_target_type, member_id)
- @member_target_type = member_target_type
+ def member_invite_accepted_email(member_source_type, member_id)
+ @member_source_type = member_source_type
@member_id = member_id
- return if access_requester.nil?
+ return unless member.created_by
- mail(to: access_requester.notification_email,
+ mail(to: member.created_by.notification_email,
subject: subject('Invitation accepted'))
end
- def member_invite_declined_email(member_target_type, target_id, invite_email, created_by_id)
- return if created_by_id.nil?
+ def member_invite_declined_email(member_source_type, source_id, invite_email, created_by_id)
+ return unless created_by_id
- @member_target_type = member_target_type
- @target = target_class.find(target_id)
+ @member_source_type = member_source_type
+ @member_source = member_source_class.find(source_id)
@invite_email = invite_email
+ inviter = User.find(created_by_id)
- mail(to: User.find(created_by_id).notification_email,
+ mail(to: inviter.notification_email,
subject: subject('Invitation declined'))
end
def member
- @member ||= member_class.find(@member_id)
+ @member ||= Member.find(@member_id)
end
- def access_requester
- @access_requester ||= member.created_by
- end
-
- def member_target_name
- case member_target_type
- when 'project'
- target.name_with_namespace
- else
- target.name
- end
- end
-
- def member_target_url
- @member_target_url ||= target.web_url
+ def member_source
+ @member_source ||= member.source
end
private
- def target
- @target ||= member.public_send(member_target_type)
- end
-
- def target_class
- @target_class ||= member_target_type.classify.constantize
- end
-
- def member_class
- @member_class ||= "#{member_target_type.classify}Member".constantize
- end
-
- def members_association
- @members_association ||= member_class.to_s.tableize
+ def member_source_class
+ @member_source_type.classify.constantize
end
end
end
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index bd5c6788cce..0cc709f68e4 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -13,6 +13,8 @@ class Notify < BaseMailer
add_template_helper DiffHelper
add_template_helper BlobHelper
add_template_helper EmailsHelper
+ add_template_helper MembersHelper
+ add_template_helper GitlabRoutingHelper
def test_email(recipient_email, subject, body)
mail(to: recipient_email,
diff --git a/app/models/ability.rb b/app/models/ability.rb
index 90156bf130c..647a73aa1ce 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -460,8 +460,6 @@ 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
@@ -481,8 +479,6 @@ 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
index cf37284e31a..eedd32a729f 100644
--- a/app/models/concerns/access_requestable.rb
+++ b/app/models/concerns/access_requestable.rb
@@ -10,18 +10,7 @@ module AccessRequestable
def request_access(user)
members.create(
access_level: Gitlab::Access::DEVELOPER,
- created_by: user,
+ user: 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 520cbd0283c..b8dffe9f5b9 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -8,7 +8,7 @@ class Group < Namespace
has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember'
alias_method :members, :group_members
- has_many :users, through: :group_members
+ has_many :users, -> { where(members: { requested_at: nil }) }, through: :group_members
has_many :project_group_links, dependent: :destroy
has_many :shared_projects, through: :project_group_links, source: :project
has_many :notification_settings, dependent: :destroy, as: :source
diff --git a/app/models/member.rb b/app/models/member.rb
index 5c3a5eab406..cea6d259760 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: :pending?
+ validates :user, presence: true, unless: :invite?
validates :source, presence: true
validates :user_id, uniqueness: { scope: [:source_type, :source_id],
message: "already exists in source",
@@ -27,16 +27,17 @@ class Member < ActiveRecord::Base
}
scope :invite, -> { where.not(invite_token: nil) }
+ scope :non_invite, -> { where(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 :non_pending, -> { non_request.non_invite }
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]) }
+ scope :owners_and_masters, -> { where(access_level: [OWNER, MASTER]) }
before_validation :generate_invite_token, on: :create, if: -> (member) { member.invite_email.present? }
@@ -46,6 +47,7 @@ class Member < ActiveRecord::Base
after_create :post_create_hook, unless: :pending?
after_update :post_update_hook, unless: :pending?
after_destroy :post_destroy_hook, unless: :pending?
+ after_destroy :post_decline_request, if: :request?
delegate :name, :username, :email, to: :user, prefix: true
@@ -102,36 +104,31 @@ class Member < ActiveRecord::Base
end
end
- def pending?
- request? || invite?
+ def real_source_type
+ source_type
+ end
+
+ def invite?
+ self.invite_token.present?
end
def request?
- user.nil? && created_by.present? && requested_at.present?
+ requested_at.present?
end
- def invite?
- self.invite_token.present?
+ def pending?
+ invite? || request?
end
def accept_request
return false unless request?
- updated = self.update(user: created_by, requested_at: nil)
+ updated = self.update(requested_at: nil)
after_accept_request if updated
updated
end
- def decline_request
- return false unless request?
-
- self.destroy
- after_decline_request if destroyed?
-
- destroyed?
- end
-
def accept_invite!(new_user)
return false unless invite?
@@ -217,7 +214,7 @@ class Member < ActiveRecord::Base
post_create_hook
end
- def after_decline_request
+ def post_decline_request
# override in subclass
end
diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb
index 476b4816b90..363db877968 100644
--- a/app/models/members/group_member.rb
+++ b/app/models/members/group_member.rb
@@ -20,6 +20,11 @@ class GroupMember < Member
access_level
end
+ # Because source_type is `Namespace`...
+ def real_source_type
+ 'Group'
+ end
+
private
def send_invite
@@ -60,8 +65,8 @@ class GroupMember < Member
super
end
- def after_decline_request
- notification_service.decline_group_access_request(group, created_by)
+ def post_decline_request
+ notification_service.decline_group_access_request(self)
super
end
diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb
index c6fd1a5c3d1..250ee04fd1d 100644
--- a/app/models/members/project_member.rb
+++ b/app/models/members/project_member.rb
@@ -152,8 +152,8 @@ class ProjectMember < Member
super
end
- def after_decline_request
- notification_service.decline_project_access_request(project, created_by)
+ def post_decline_request
+ notification_service.decline_project_access_request(self)
super
end
diff --git a/app/models/project.rb b/app/models/project.rb
index ef665373495..0d2e612436a 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -104,7 +104,8 @@ class Project < ActiveRecord::Base
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 :users, through: :project_members
+ alias_method :members, :project_members
+ has_many :users, -> { where(members: { requested_at: nil }) }, through: :project_members
has_many :deploy_keys_projects, dependent: :destroy
has_many :deploy_keys, through: :deploy_keys_projects
has_many :users_star_projects, dependent: :destroy
@@ -690,6 +691,7 @@ class Project < ActiveRecord::Base
end
end
end
+ alias_method :human_name, :name_with_namespace
def path_with_namespace
if namespace
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index 7fb17df0e96..73e736820af 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -22,12 +22,12 @@ class ProjectTeam
end
def find_member(user_id)
- member = project.project_members.find_by(user_id: user_id)
+ member = project.members.non_request.find_by(user_id: user_id)
# If user is not in project members
# we should check for group membership
if group && !member
- member = group.group_members.find_by(user_id: user_id)
+ member = group.members.non_request.find_by(user_id: user_id)
end
member
@@ -128,12 +128,16 @@ 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)
access = []
- project.project_members.each do |member|
+ project.members.non_request.each do |member|
if member.user_id == user_id
access << member.access_field if member.access_field
break
@@ -141,7 +145,7 @@ class ProjectTeam
end
if group
- group.group_members.each do |member|
+ group.members.non_request.each do |member|
if member.user_id == user_id
access << member.access_field if member.access_field
break
@@ -174,14 +178,14 @@ class ProjectTeam
end
def fetch_members(level = nil)
- project_members = project.project_members
- group_members = group ? group.group_members : []
+ project_members = project.members.non_request
+ group_members = group ? group.members.non_request : []
invited_members = []
if project.invited_groups.any? && project.allowed_to_share_with_group?
project.project_group_links.each do |group_link|
invited_group = group_link.group
- im = invited_group.group_members
+ im = invited_group.members.non_request
if level
int_level = GroupMember.access_level_roles[level.to_s.singularize.titleize]
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 259199f6e2b..f804ac171c4 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -175,24 +175,24 @@ class NotificationService
# Project access request
def new_project_access_request(project_member)
- mailer.member_access_requested_email('project', project_member.id).deliver_later
+ mailer.member_access_requested_email(project_member.real_source_type, project_member.id).deliver_later
end
- def decline_project_access_request(project, user)
- mailer.member_access_denied_email('project', project.id, user.id).deliver_later
+ def decline_project_access_request(project_member)
+ mailer.member_access_denied_email(project_member.real_source_type, project_member.project.id, project_member.user.id).deliver_later
end
def invite_project_member(project_member, token)
- mailer.member_invited_email('project', project_member.id, token).deliver_later
+ mailer.member_invited_email(project_member.real_source_type, project_member.id, token).deliver_later
end
def accept_project_invite(project_member)
- mailer.member_invite_accepted_email('project', project_member.id).deliver_later
+ mailer.member_invite_accepted_email(project_member.real_source_type, project_member.id).deliver_later
end
def decline_project_invite(project_member)
mailer.member_invite_declined_email(
- 'project',
+ project_member.real_source_type,
project_member.project.id,
project_member.invite_email,
project_member.access_level,
@@ -201,24 +201,24 @@ class NotificationService
end
def new_project_member(project_member)
- mailer.member_access_granted_email('project', project_member.id).deliver_later
+ mailer.member_access_granted_email(project_member.real_source_type, project_member.id).deliver_later
end
def update_project_member(project_member)
- mailer.member_access_granted_email('project', project_member.id).deliver_later
+ mailer.member_access_granted_email(project_member.real_source_type, project_member.id).deliver_later
end
# Group access request
def new_group_access_request(group_member)
- mailer.member_access_requested_email('group', group_member.id).deliver_later
+ mailer.member_access_requested_email(group_member.real_source_type, group_member.id).deliver_later
end
- def decline_group_access_request(group, user)
- mailer.member_access_denied_email('group', group.id, user.id).deliver_later
+ def decline_group_access_request(group_member)
+ mailer.member_access_denied_email(group_member.real_source_type, group_member.group.id, group_member.user.id).deliver_later
end
def invite_group_member(group_member, token)
- mailer.member_invited_email('group', group_member.id, token).deliver_later
+ mailer.member_invited_email(group_member.real_source_type, group_member.id, token).deliver_later
end
def accept_group_invite(group_member)
@@ -227,7 +227,7 @@ class NotificationService
def decline_group_invite(group_member)
mailer.member_invite_declined_email(
- 'group',
+ group_member.real_source_type,
group_member.group.id,
group_member.invite_email,
group_member.access_level,
@@ -236,11 +236,11 @@ class NotificationService
end
def new_group_member(group_member)
- mailer.member_access_granted_email('group', group_member.id).deliver_later
+ mailer.member_access_granted_email(group_member.real_source_type, group_member.id).deliver_later
end
def update_group_member(group_member)
- mailer.member_access_granted_email('group', group_member.id).deliver_later
+ mailer.member_access_granted_email(group_member.real_source_type, group_member.id).deliver_later
end
def project_was_moved(project, old_path_with_namespace)
diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml
index a39d5d3d0f0..a36531e095a 100644
--- a/app/views/groups/group_members/index.html.haml
+++ b/app/views/groups/group_members/index.html.haml
@@ -11,7 +11,7 @@
.new-group-member-holder
= render "new_group_member"
- = render "shared/members/requests", entity: @group, members: @members
+ = render 'shared/members/requests', membership_source: @group, members: @members.request
.panel.panel-default
.panel-heading
diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml
index 85635bc4616..62ebd69485c 100644
--- a/app/views/groups/show.html.haml
+++ b/app/views/groups/show.html.haml
@@ -19,6 +19,9 @@
.cover-desc.description
= markdown(@group.description, pipeline: :description)
+ - if current_user
+ = render 'shared/members/access_request_buttons', source: @group
+
%div{ class: container_class }
.top-area
%ul.nav-links
diff --git a/app/views/layouts/nav/_group_settings.html.haml b/app/views/layouts/nav/_group_settings.html.haml
index b461772b87e..dac46648b9f 100644
--- a/app/views/layouts/nav/_group_settings.html.haml
+++ b/app/views/layouts/nav/_group_settings.html.haml
@@ -1,3 +1,16 @@
- if current_user
- .controls
- = render 'shared/group_or_project_home_dropdown', entity: @group
+ - 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
diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml
index 3398794302f..ad019710830 100644
--- a/app/views/layouts/nav/_project.html.haml
+++ b/app/views/layouts/nav/_project.html.haml
@@ -1,33 +1,25 @@
- if current_user
.controls
- - access = user_max_access_in_project(current_user.id, @project)
- - can_edit = can?(current_user, :admin_project, @project)
.dropdown.project-settings-dropdown
%a.dropdown-new.btn.btn-default#project-settings-button{href: '#', 'data-toggle' => 'dropdown'}
= icon('cog')
= icon('caret-down')
%ul.dropdown-menu.dropdown-menu-align-right
= render 'layouts/nav/project_settings'
- %li.divider
- - if can_edit
- %li
- = link_to edit_project_path(@project) do
- Edit Project
- - if access
- %li
- = 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_path(@project),
- class: 'btn btn-gray', style: 'margin-left: 10px', method: :post do
- Request Access
+
+ - access = @project.team.max_member_access(current_user.id)
+ - can_edit = can?(current_user, :admin_project, @project)
+ - if can_edit || access
+ %li.divider
+ - if can_edit
+ %li
+ = link_to edit_project_path(@project) do
+ Edit Project
+ - if access
+ %li
+ = link_to polymorphic_path([:leave, @project, :members]),
+ data: { confirm: leave_confirmation_message(@project) }, method: :delete do
+ Leave Project
%div{ class: nav_control_class }
%ul.nav-links.scrolling-tabs
diff --git a/app/views/notify/group_access_granted_email.html.haml b/app/views/notify/group_access_granted_email.html.haml
deleted file mode 100644
index 1283758c576..00000000000
--- a/app/views/notify/group_access_granted_email.html.haml
+++ /dev/null
@@ -1,3 +0,0 @@
-%p
- 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
deleted file mode 100644
index c7568350075..00000000000
--- a/app/views/notify/group_access_granted_email.text.erb
+++ /dev/null
@@ -1,3 +0,0 @@
-You have been granted <%= @group_member.human_access %> access to group <%= @group.name %>.
-
-<%= @target_url %>
diff --git a/app/views/notify/group_invite_accepted_email.html.haml b/app/views/notify/group_invite_accepted_email.html.haml
deleted file mode 100644
index 55efad384a7..00000000000
--- a/app/views/notify/group_invite_accepted_email.html.haml
+++ /dev/null
@@ -1,6 +0,0 @@
-%p
- #{@group_member.invite_email}, now known as
- #{link_to @group_member.user.name, user_url(@group_member.user)},
- has accepted your invitation to join group
- #{link_to @group.name, group_url(@group)}.
-
diff --git a/app/views/notify/group_invite_accepted_email.text.erb b/app/views/notify/group_invite_accepted_email.text.erb
deleted file mode 100644
index f8b70f7a5a6..00000000000
--- a/app/views/notify/group_invite_accepted_email.text.erb
+++ /dev/null
@@ -1,3 +0,0 @@
-<%= @group_member.invite_email %>, now known as <%= @group_member.user.name %>, has accepted your invitation to join group <%= @group.name %>.
-
-<%= group_url(@group) %>
diff --git a/app/views/notify/group_invite_declined_email.html.haml b/app/views/notify/group_invite_declined_email.html.haml
deleted file mode 100644
index f9525d84fac..00000000000
--- a/app/views/notify/group_invite_declined_email.html.haml
+++ /dev/null
@@ -1,5 +0,0 @@
-%p
- #{@invite_email}
- has declined your invitation to join group
- #{link_to @group.name, group_url(@group)}.
-
diff --git a/app/views/notify/group_invite_declined_email.text.erb b/app/views/notify/group_invite_declined_email.text.erb
deleted file mode 100644
index 6c19a288d15..00000000000
--- a/app/views/notify/group_invite_declined_email.text.erb
+++ /dev/null
@@ -1,3 +0,0 @@
-<%= @invite_email %> has declined your invitation to join group <%= @group.name %>.
-
-<%= group_url(@group) %>
diff --git a/app/views/notify/group_member_invited_email.html.haml b/app/views/notify/group_member_invited_email.html.haml
deleted file mode 100644
index 163e88bfea3..00000000000
--- a/app/views/notify/group_member_invited_email.html.haml
+++ /dev/null
@@ -1,14 +0,0 @@
-%p
- You have been invited
- - if inviter = @group_member.created_by
- by
- = link_to inviter.name, user_url(inviter)
- to join group
- = link_to @group.name, group_url(@group)
- as #{@group_member.human_access}.
-
-%p
- = link_to 'Accept invitation', invite_url(@token)
- or
- = link_to 'decline', decline_invite_url(@token)
-
diff --git a/app/views/notify/group_member_invited_email.text.erb b/app/views/notify/group_member_invited_email.text.erb
deleted file mode 100644
index 28ce4819b14..00000000000
--- a/app/views/notify/group_member_invited_email.text.erb
+++ /dev/null
@@ -1,4 +0,0 @@
-You have been invited <%= "by #{@group_member.created_by.name} " if @group_member.created_by %>to join group <%= @group.name %> as <%= @group_member.human_access %>.
-
-Accept invitation: <%= invite_url(@token) %>
-Decline invitation: <%= decline_invite_url(@token) %>
diff --git a/app/views/notify/member_access_denied_email.html.haml b/app/views/notify/member_access_denied_email.html.haml
index a25af24d783..71c9c50071a 100644
--- a/app/views/notify/member_access_denied_email.html.haml
+++ b/app/views/notify/member_access_denied_email.html.haml
@@ -1,4 +1,4 @@
%p
Your request to join the
- #{link_to member_target_name, member_target_url} #{member_target_type}
+ #{link_to member_source.human_name, member_source.web_url} #{member_source.model_name.singular}
has been denied.
diff --git a/app/views/notify/member_access_denied_email.text.erb b/app/views/notify/member_access_denied_email.text.erb
index eb204458d9d..87f2ef817ee 100644
--- a/app/views/notify/member_access_denied_email.text.erb
+++ b/app/views/notify/member_access_denied_email.text.erb
@@ -1,3 +1,3 @@
-Your request to join the <%= member_target_name %> <%= member_target_type %> has been denied.
+Your request to join the <%= member_source.human_name %> <%= member_source.model_name.singular %> has been denied.
-<%= member_target_url %>
+<%= member_source.web_url %>
diff --git a/app/views/notify/member_access_granted_email.html.haml b/app/views/notify/member_access_granted_email.html.haml
index 62837d74555..18dec806539 100644
--- a/app/views/notify/member_access_granted_email.html.haml
+++ b/app/views/notify/member_access_granted_email.html.haml
@@ -1,3 +1,3 @@
%p
You have been granted #{member.human_access} access to the
- #{link_to member_target_name, member_target_url} #{member_target_type}.
+ #{link_to member_source.human_name, member_source.web_url} #{member_source.model_name.singular}.
diff --git a/app/views/notify/member_access_granted_email.text.erb b/app/views/notify/member_access_granted_email.text.erb
index be9bb5ee948..a9fb3a589a5 100644
--- a/app/views/notify/member_access_granted_email.text.erb
+++ b/app/views/notify/member_access_granted_email.text.erb
@@ -1,3 +1,3 @@
-You have been granted <%= member.human_access %> access to the <%= member_target_name %> <%= member_target_type %>.
+You have been granted <%= member.human_access %> access to the <%= member_source.human_name %> <%= member_source.model_name.singular %>.
-<%= member_target_url %>
+<%= member_source.web_url %>
diff --git a/app/views/notify/member_access_requested_email.html.haml b/app/views/notify/member_access_requested_email.html.haml
index 96e92a069f2..76f1f08a0cb 100644
--- a/app/views/notify/member_access_requested_email.html.haml
+++ b/app/views/notify/member_access_requested_email.html.haml
@@ -1,3 +1,3 @@
%p
- #{link_to access_requester.name, access_requester} requested #{member.human_access}
- access to the #{link_to member_target_name, member_target_url} #{member_target_type}.
+ #{link_to member.user.name, member.user} requested #{member.human_access}
+ access to the #{link_to member_source.human_name, polymorphic_url([member_source, :members])} #{member_source.model_name.singular}.
diff --git a/app/views/notify/member_access_requested_email.text.erb b/app/views/notify/member_access_requested_email.text.erb
index 3b5de8c2abe..9c5ee0eaf26 100644
--- a/app/views/notify/member_access_requested_email.text.erb
+++ b/app/views/notify/member_access_requested_email.text.erb
@@ -1,3 +1,3 @@
-<%= access_requester.name %> (<%= user_url(access_requester) %>) requested <%= member.human_access %> access to the <%= member_target_name %> <%= member_target_type %>.
+<%= member.user.name %> (<%= user_url(member.user) %>) requested <%= member.human_access %> access to the <%= member_source.human_name %> <%= member_source.model_name.singular %>.
-<%= member_target_url %>
+<%= polymorphic_url([member_source, :members]) %>
diff --git a/app/views/notify/member_invite_accepted_email.html.haml b/app/views/notify/member_invite_accepted_email.html.haml
index c420a8a7b3c..2d1d40881eb 100644
--- a/app/views/notify/member_invite_accepted_email.html.haml
+++ b/app/views/notify/member_invite_accepted_email.html.haml
@@ -2,4 +2,4 @@
#{member.invite_email}, now known as
#{link_to member.user.name, user_url(member.user)},
has accepted your invitation to join the
- #{link_to member_target_name, member_target_url} #{member_target_type}.
+ #{link_to member_source.human_name, member_source.web_url} #{member_source.model_name.singular}.
diff --git a/app/views/notify/member_invite_accepted_email.text.erb b/app/views/notify/member_invite_accepted_email.text.erb
index a1616163ceb..cef87101427 100644
--- a/app/views/notify/member_invite_accepted_email.text.erb
+++ b/app/views/notify/member_invite_accepted_email.text.erb
@@ -1,3 +1,3 @@
-<%= member.invite_email %>, now known as <%= member.user.name %>, has accepted your invitation to join the <%= member_target_name %> <%= member_target_type %>.
+<%= member.invite_email %>, now known as <%= member.user.name %>, has accepted your invitation to join the <%= member_source.human_name %> <%= member_source.model_name.singular %>.
-<%= member_target_url %>
+<%= member_source.web_url %>
diff --git a/app/views/notify/member_invite_declined_email.html.haml b/app/views/notify/member_invite_declined_email.html.haml
index 5a30ac31b3c..aa1b373d1a6 100644
--- a/app/views/notify/member_invite_declined_email.html.haml
+++ b/app/views/notify/member_invite_declined_email.html.haml
@@ -1,4 +1,4 @@
%p
#{@invite_email}
has declined your invitation to join the
- #{link_to member_target_name, member_target_url} #{member_target_type}.
+ #{link_to member_source.human_name, member_source.web_url} #{member_source.model_name.singular}.
diff --git a/app/views/notify/member_invite_declined_email.text.erb b/app/views/notify/member_invite_declined_email.text.erb
index 301287946d4..8bc305910c4 100644
--- a/app/views/notify/member_invite_declined_email.text.erb
+++ b/app/views/notify/member_invite_declined_email.text.erb
@@ -1,3 +1,3 @@
-<%= @invite_email %> has declined your invitation to join the <%= member_target_name %> <%= member_target_type %>.
+<%= @invite_email %> has declined your invitation to join the <%= member_source.human_name %> <%= member_source.model_name.singular %>.
-<%= member_target_url %>
+<%= member_source.web_url %>
diff --git a/app/views/notify/member_invited_email.html.haml b/app/views/notify/member_invited_email.html.haml
index a8e58df9ac8..b8b75da3f2f 100644
--- a/app/views/notify/member_invited_email.html.haml
+++ b/app/views/notify/member_invited_email.html.haml
@@ -1,11 +1,11 @@
%p
You have been invited
- - if access_requester
+ - if member.created_by
by
- = link_to access_requester.name, user_url(access_requester)
+ = link_to member.created_by.name, user_url(member.created_by)
to join the
- = link_to member_target_name, member_target_url
- #{member_target_type} as #{member.human_access}.
+ = link_to member_source.human_name, member_source.web_url
+ #{member_source.model_name.singular} as #{member.human_access}.
%p
= link_to 'Accept invitation', invite_url(@token)
diff --git a/app/views/notify/member_invited_email.text.erb b/app/views/notify/member_invited_email.text.erb
index 1b6897ee2ec..0a6393355be 100644
--- a/app/views/notify/member_invited_email.text.erb
+++ b/app/views/notify/member_invited_email.text.erb
@@ -1,4 +1,4 @@
-You have been invited <%= "by #{access_requester.name} " if access_requester %>to join the <%= member_target_name %> <%= member_target_type %> as <%= member.human_access %>.
+You have been invited <%= "by #{member.created_by.name} " if member.created_by %>to join the <%= member_source.human_name %> <%= member_source.model_name.singular %> as <%= member.human_access %>.
Accept invitation: <%= invite_url(@token) %>
Decline invitation: <%= decline_invite_url(@token) %>
diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml
index f5bc1b4e409..2b19ee93eea 100644
--- a/app/views/projects/_home_panel.html.haml
+++ b/app/views/projects/_home_panel.html.haml
@@ -29,10 +29,13 @@
.project-clone-holder
= render "shared/clone_panel"
- .project-repo-buttons.btn-group.project-right-buttons
- = render "projects/buttons/download"
- = render 'projects/buttons/dropdown'
- = render 'projects/buttons/notifications'
+ .project-repo-buttons.project-right-buttons
+ - if current_user
+ = render 'shared/members/access_request_buttons', source: @project
+ .btn-group
+ = render "projects/buttons/download"
+ = render 'projects/buttons/dropdown'
+ = render 'projects/buttons/notifications'
:javascript
new Star();
diff --git a/app/views/projects/buttons/_notifications.html.haml b/app/views/projects/buttons/_notifications.html.haml
index 3b97dc9328f..a7a97181096 100644
--- a/app/views/projects/buttons/_notifications.html.haml
+++ b/app/views/projects/buttons/_notifications.html.haml
@@ -1,7 +1,7 @@
- if @notification_setting
= form_for @notification_setting, url: namespace_project_notification_setting_path(@project.namespace.becomes(Namespace), @project), method: :patch, remote: true, html: { class: 'inline', id: 'notification-form' } do |f|
= f.hidden_field :level
- .dropdown
+ .dropdown.hidden-sm
%button.btn.btn-default.notifications-btn#notifications-button{ data: { toggle: "dropdown" }, aria: { haspopup: "true", expanded: "false" } }
= icon('bell')
= notification_title(@notification_setting.level)
diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml
index 112a532f9d3..bcdbff08011 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 = max_access_level(note.project, note.author)
+ - access = note.project.team.human_max_access(note.author.id)
- 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 78c12d52a78..cb6136c215a 100644
--- a/app/views/projects/project_members/_group_members.html.haml
+++ b/app/views/projects/project_members/_group_members.html.haml
@@ -6,8 +6,9 @@
(#{members.count})
- if can?(current_user, :admin_group_member, @group)
.controls
- = link_to group_group_members_path(@group), class: 'btn' do
- Manage group members
+ = link_to 'Manage group members',
+ group_group_members_path(@group),
+ class: 'btn'
%ul.content-list
= render partial: 'shared/members/member',
collection: members.limit(20),
@@ -15,7 +16,4 @@
locals: { show_controls: false }
- if members.size > 20
%li
- and
- = members.size - 20
- more. For full list visit
- = link_to 'group members page', group_group_members_path(@group)
+ and #{members.count - 20} more. For full list visit #{link_to 'group members page', group_group_members_path(@group)}
diff --git a/app/views/projects/project_members/index.html.haml b/app/views/projects/project_members/index.html.haml
index 61a82724d69..357ccccaf1d 100644
--- a/app/views/projects/project_members/index.html.haml
+++ b/app/views/projects/project_members/index.html.haml
@@ -13,9 +13,9 @@
Users with access to this project are listed below.
= render "new_project_member"
- = render "shared/members/requests", entity: @project, members: @project_members
+ = render 'shared/members/requests', membership_source: @project, members: @project_members.request
- = render "team", members: @project_members.non_request
+ = render 'team', members: @project_members.non_request
- if @group
= render "group_members", members: @group_members
diff --git a/app/views/shared/_group_or_project_home_dropdown.html.haml b/app/views/shared/_group_or_project_home_dropdown.html.haml
deleted file mode 100644
index fb9e63f2bd4..00000000000
--- a/app/views/shared/_group_or_project_home_dropdown.html.haml
+++ /dev/null
@@ -1,30 +0,0 @@
-- 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/members/_access_request_buttons.html.haml b/app/views/shared/members/_access_request_buttons.html.haml
new file mode 100644
index 00000000000..ed0a6ebcf84
--- /dev/null
+++ b/app/views/shared/members/_access_request_buttons.html.haml
@@ -0,0 +1,12 @@
+- member = source.members.find_by(user_id: current_user.id)
+
+- if member
+ - if member.request?
+ = link_to 'Withdraw Access Request', polymorphic_path([:leave, source, :members]),
+ method: :delete,
+ data: { confirm: remove_member_message(member) },
+ class: 'btn access-request-button hidden-xs'
+- else
+ = link_to 'Request Access', polymorphic_path([:request_access, source, :members]),
+ method: :post,
+ class: 'btn access-request-button hidden-xs'
diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml
index 7e119155a6c..c69d4cbfbe3 100644
--- a/app/views/shared/members/_member.html.haml
+++ b/app/views/shared/members/_member.html.haml
@@ -1,6 +1,6 @@
- show_roles = local_assigns.fetch(:show_roles, true)
- show_controls = local_assigns.fetch(:show_controls, true)
-- user = member.request? ? member.created_by : member.user
+- user = member.user
%li.js-toggle-container{ class: dom_class(member), id: dom_id(member) }
%span{ class: ("list-item-name" if show_controls) }
@@ -18,25 +18,25 @@
%strong Blocked
- if member.request?
- %small
+ %span.cgray
– 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
+ – 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),
+ = link_to 'Resend invite', polymorphic_path([:resend_invite, member]),
method: :post,
class: 'btn-xs btn'
- - if show_roles && can_see_entity_roles?(current_user, member.source)
+ - if show_roles && can_see_member_roles?(source: member.source, user: current_user)
%span.pull-right
%strong= member.human_access
- if show_controls
@@ -48,30 +48,30 @@
- if member.request?
&nbsp;
- = link_to icon('check inverse'), approve_request_member_path(member),
+ = link_to icon('check inverse'), polymorphic_path([:approve_access_request, member]),
method: :post,
- type: 'button',
class: 'btn-xs btn btn-success',
title: 'Grant access'
- if can?(current_user, action_member_permission(:destroy, member), member)
&nbsp;
- 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),
+ = link_to icon('sign-out', text: 'Leave'), polymorphic_path([:leave, member.source, :members]),
method: :delete,
+ data: { confirm: leave_confirmation_message(member.source) },
+ class: 'btn-xs btn btn-remove'
+ - else
+ = link_to icon('trash'), member,
remote: true,
+ method: :delete,
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|
+ = form_for member, 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'
+ = f.select :access_level, options_for_select(member.class.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
index ffbb380f794..b5963876034 100644
--- a/app/views/shared/members/_requests.html.haml
+++ b/app/views/shared/members/_requests.html.haml
@@ -1,10 +1,8 @@
-- requesters = members.request
-
-- if requesters.any?
+- if members.any?
.panel.panel-default
.panel-heading
- %strong= entity.name
+ %strong= membership_source.name
access requests
- %small= "(#{requesters.size})"
+ %small= "(#{members.size})"
%ul.content-list
- = render partial: 'shared/members/member', collection: requesters, as: :member
+ = render partial: 'shared/members/member', collection: members, as: :member
diff --git a/features/steps/dashboard/group.rb b/features/steps/dashboard/group.rb
index 0c6a0ae3725..9b79a3be49b 100644
--- a/features/steps/dashboard/group.rb
+++ b/features/steps/dashboard/group.rb
@@ -62,6 +62,6 @@ class Spinach::Features::DashboardGroup < Spinach::FeatureSteps
end
step 'I should see the "Can not leave message"' do
- expect(page).to have_content "You can not leave Owned group because you're the last owner"
+ expect(page).to have_content "You can not leave the \"Owned\" group."
end
end
diff --git a/features/steps/group/members.rb b/features/steps/group/members.rb
index 9de82765df1..dfa2fa75def 100644
--- a/features/steps/group/members.rb
+++ b/features/steps/group/members.rb
@@ -53,7 +53,7 @@ class Spinach::Features::GroupMembers < Spinach::FeatureSteps
step 'I should see "sjobs@apple.com" in team list as invited "Reporter"' do
page.within '.content-list' do
expect(page).to have_content('sjobs@apple.com')
- expect(page).to have_content('invited')
+ expect(page).to have_content('Invited')
expect(page).to have_content('Reporter')
end
end
@@ -116,11 +116,9 @@ class Spinach::Features::GroupMembers < Spinach::FeatureSteps
member = mary_jane_member
page.within "#group_member_#{member.id}" do
- find(".js-toggle-button").click
- page.within "#edit_group_member_#{member.id}" do
- select 'Developer', from: 'group_member_access_level'
- click_on 'Save'
- end
+ click_button "Edit access level"
+ select 'Developer', from: 'group_member_access_level'
+ click_on 'Save'
end
end
diff --git a/features/steps/project/team_management.rb b/features/steps/project/team_management.rb
index c6ced747370..f32576d2cb1 100644
--- a/features/steps/project/team_management.rb
+++ b/features/steps/project/team_management.rb
@@ -26,8 +26,11 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps
end
step 'I should see "Mike" in team list as "Reporter"' do
- page.within ".access-reporter" do
+ user = User.find_by(name: 'Mike')
+ project_member = project.project_members.find_by(user_id: user.id)
+ page.within "#project_member_#{project_member.id}" do
expect(page).to have_content('Mike')
+ expect(page).to have_content('Reporter')
end
end
@@ -40,16 +43,20 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps
end
step 'I should see "sjobs@apple.com" in team list as invited "Reporter"' do
- page.within ".access-reporter" do
+ project_member = project.project_members.find_by(invite_email: 'sjobs@apple.com')
+ page.within "#project_member_#{project_member.id}" do
expect(page).to have_content('sjobs@apple.com')
- expect(page).to have_content('invited')
+ expect(page).to have_content('Invited')
expect(page).to have_content('Reporter')
end
end
step 'I should see "Dmitriy" in team list as "Developer"' do
- page.within ".access-developer" do
+ user = User.find_by(name: 'Dmitriy')
+ project_member = project.project_members.find_by(user_id: user.id)
+ page.within "#project_member_#{project_member.id}" do
expect(page).to have_content('Dmitriy')
+ expect(page).to have_content('Developer')
end
end
@@ -65,15 +72,14 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps
end
step 'I should see "Dmitriy" in team list as "Reporter"' do
- page.within ".access-reporter" do
+ user = User.find_by(name: 'Dmitriy')
+ project_member = project.project_members.find_by(user_id: user.id)
+ page.within "#project_member_#{project_member.id}" do
expect(page).to have_content('Dmitriy')
+ expect(page).to have_content('Reporter')
end
end
- step 'I click link "Remove from team"' do
- click_link "Remove from team"
- end
-
step 'I should not see "Dmitriy" in team list' do
user = User.find_by(name: "Dmitriy")
expect(page).not_to have_content(user.name)
@@ -120,7 +126,7 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps
user = User.find_by(name: 'Dmitriy')
project_member = project.project_members.find_by(user_id: user.id)
page.within "#project_member_#{project_member.id}" do
- click_link('Remove user from team')
+ click_link('Remove user from project')
end
end
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 14370ac218d..cc29c7ef428 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -88,10 +88,7 @@ module API
class Group < Grape::Entity
expose :id, :name, :path, :description, :visibility_level
expose :avatar_url
-
- expose :web_url do |group, options|
- Gitlab::Routing.url_helpers.group_url(group)
- end
+ expose :web_url
end
class GroupDetail < Group
diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb
index 0ca8a656f63..89c2c26a367 100644
--- a/spec/controllers/groups/group_members_controller_spec.rb
+++ b/spec/controllers/groups/group_members_controller_spec.rb
@@ -35,7 +35,7 @@ describe Groups::GroupMembersController do
let(:group_user) { create(:user) }
let(:member) do
group.add_developer(group_user)
- group.group_members.find_by(user_id: group_user.id)
+ group.members.find_by(user_id: group_user)
end
context 'when user does not have enough rights' do
@@ -103,7 +103,7 @@ describe Groups::GroupMembersController do
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 set_flash.to "You left the \"#{group.name}\" group."
expect(response).to redirect_to(dashboard_groups_path)
expect(group.users).not_to include user
end
@@ -118,8 +118,8 @@ describe Groups::GroupMembersController do
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(response).to redirect_to(group_path(group))
+ expect(response).to set_flash[:alert].to "You can not leave the \"#{group.name}\" group. Transfer or delete the group."
expect(group.users).to include user
end
end
@@ -133,9 +133,9 @@ describe Groups::GroupMembersController do
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 set_flash.to 'Your access request to the group has been withdrawn.'
expect(response).to redirect_to(dashboard_groups_path)
- expect(group.group_members.request).to be_empty
+ expect(group.members.request).to be_empty
expect(group.users).not_to include user
end
end
@@ -155,18 +155,18 @@ describe Groups::GroupMembersController do
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.members.request.exists?(user_id: user)).to be_truthy
expect(group.users).not_to include user
end
end
- describe '#approve' do
+ describe '#approve_access_request' do
let(:group) { create(:group, :public) }
context 'when member is not found' do
it 'returns 403' do
post :approve_access_request, group_id: group,
- id: 42
+ id: 42
expect(response.status).to eq(403)
end
@@ -177,7 +177,7 @@ describe Groups::GroupMembersController do
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)
+ group.members.request.find_by(user_id: group_requester)
end
context 'when user does not have enough rights' do
@@ -188,7 +188,7 @@ describe Groups::GroupMembersController do
it 'returns 403' do
post :approve_access_request, group_id: group,
- id: member
+ id: member
expect(response.status).to eq(403)
expect(group.users).not_to include group_requester
@@ -203,7 +203,7 @@ describe Groups::GroupMembersController do
it 'adds user to members' do
post :approve_access_request, group_id: group,
- id: member
+ id: member
expect(response).to redirect_to(group_group_members_path(group))
expect(group.users).to include group_requester
diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb
index d3bd2d0bbba..fc5f458e795 100644
--- a/spec/controllers/projects/project_members_controller_spec.rb
+++ b/spec/controllers/projects/project_members_controller_spec.rb
@@ -80,7 +80,7 @@ describe Projects::ProjectMembersController do
let(:team_user) { create(:user) }
let(:member) do
project.team << [team_user, :developer]
- project.project_members.find_by(user_id: team_user.id)
+ project.members.find_by(user_id: team_user.id)
end
context 'when user does not have enough rights' do
@@ -154,7 +154,7 @@ describe Projects::ProjectMembersController do
delete :leave, namespace_id: project.namespace,
project_id: project
- expect(response).to set_flash.to 'You left the project.'
+ expect(response).to set_flash.to "You left the \"#{project.human_name}\" project."
expect(response).to redirect_to(dashboard_projects_path)
expect(project.users).not_to include user
end
@@ -167,14 +167,14 @@ describe Projects::ProjectMembersController do
sign_in(user)
end
- it 'cannot removes himself from the project' do
+ it 'cannot remove 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)
+ namespace_project_path(project.namespace, project)
)
- expect(response).to set_flash[:alert].to 'You can not leave your own project. Transfer or delete the project.'
+ expect(response).to set_flash[:alert].to "You can not leave the \"#{project.human_name}\" project. Transfer or delete the project."
expect(project.users).to include user
end
end
@@ -189,9 +189,9 @@ describe Projects::ProjectMembersController 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 set_flash.to 'Your access request to the project has been withdrawn.'
expect(response).to redirect_to(dashboard_projects_path)
- expect(project.project_members.request).to be_empty
+ expect(project.members.request).to be_empty
expect(project.users).not_to include user
end
end
@@ -214,7 +214,7 @@ describe Projects::ProjectMembersController do
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.members.request.exists?(user_id: user)).to be_truthy
expect(project.users).not_to include user
end
end
@@ -225,8 +225,8 @@ describe Projects::ProjectMembersController do
context 'when member is not found' do
it 'returns 404' do
post :approve_access_request, namespace_id: project.namespace,
- project_id: project,
- id: 42
+ project_id: project,
+ id: 42
expect(response.status).to eq(404)
end
@@ -237,7 +237,7 @@ describe Projects::ProjectMembersController do
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)
+ project.members.request.find_by(user_id: team_requester.id)
end
context 'when user does not have enough rights' do
@@ -248,8 +248,8 @@ describe Projects::ProjectMembersController do
it 'returns 404' do
post :approve_access_request, namespace_id: project.namespace,
- project_id: project,
- id: member
+ project_id: project,
+ id: member
expect(response.status).to eq(404)
expect(project.users).not_to include team_requester
@@ -264,8 +264,8 @@ describe Projects::ProjectMembersController do
it 'adds user to members' do
post :approve_access_request, namespace_id: project.namespace,
- project_id: project,
- id: member
+ project_id: project,
+ id: member
expect(response).to redirect_to(
namespace_project_project_members_path(project.namespace, project)
diff --git a/spec/features/groups/members/owner_manages_access_requests_spec.rb b/spec/features/groups/members/owner_manages_access_requests_spec.rb
index d5b5e0e35ea..22525ce530b 100644
--- a/spec/features/groups/members/owner_manages_access_requests_spec.rb
+++ b/spec/features/groups/members/owner_manages_access_requests_spec.rb
@@ -22,12 +22,10 @@ feature 'Groups > Members > Owner manages access requests', feature: true do
expect_visible_access_request(group, user)
- perform_enqueued_jobs do
- click_on 'Grant access'
- end
+ perform_enqueued_jobs { click_on 'Grant access' }
expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
- expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{group.name} group was granted/
+ expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{group.name} group was granted"
end
scenario 'master can deny access' do
@@ -35,17 +33,15 @@ feature 'Groups > Members > Owner manages access requests', feature: true do
expect_visible_access_request(group, user)
- perform_enqueued_jobs do
- click_on 'Deny access'
- end
+ perform_enqueued_jobs { click_on 'Deny access' }
expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
- expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{group.name} group was denied/
+ expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{group.name} group was denied"
end
def expect_visible_access_request(group, user)
- expect(group.access_requested?(user)).to be_truthy
+ expect(group.members.request.exists?(user_id: user)).to be_truthy
expect(page).to have_content "#{group.name} access requests (1)"
expect(page).to have_content user.name
end
diff --git a/spec/features/groups/members/user_requests_access_spec.rb b/spec/features/groups/members/user_requests_access_spec.rb
index 9b8492807fa..a878a96b6ee 100644
--- a/spec/features/groups/members/user_requests_access_spec.rb
+++ b/spec/features/groups/members/user_requests_access_spec.rb
@@ -8,47 +8,41 @@ feature 'Groups > Members > User requests access', feature: true do
background do
group.add_owner(owner)
login_as(user)
+ visit group_path(group)
end
scenario 'user can request access to a group' do
- visit group_path(group)
-
- perform_enqueued_jobs do
- click_link 'Request Access'
- end
+ perform_enqueued_jobs { click_link 'Request Access' }
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(ActionMailer::Base.deliveries.last.subject).to match "Request to join the #{group.name} group"
- expect(group.access_requested?(user)).to be_truthy
+ expect(group.members.request.exists?(user_id: 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'
+
+ expect(page).to have_content 'Withdraw Access 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
+ expect(group.members.request.exists?(user_id: 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
+ expect(group.members.request.exists?(user_id: user)).to be_truthy
- click_link 'Withdraw Request'
+ click_link 'Withdraw Access Request'
- expect(group.access_requested?(user)).to be_falsey
- expect(page).to have_content 'You withdrawn your access request to the group.'
+ expect(group.members.request.exists?(user_id: user)).to be_falsey
+ expect(page).to have_content 'Your access request to the group has been withdrawn.'
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
index 1b5490ba97f..5fe4caa12f0 100644
--- a/spec/features/projects/members/master_manages_access_requests_spec.rb
+++ b/spec/features/projects/members/master_manages_access_requests_spec.rb
@@ -22,12 +22,10 @@ feature 'Projects > Members > Master manages access requests', feature: true do
expect_visible_access_request(project, user)
- perform_enqueued_jobs do
- click_on 'Grant access'
- end
+ perform_enqueued_jobs { click_on 'Grant access' }
expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
- expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{project.name_with_namespace} project was granted/
+ expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{project.name_with_namespace} project was granted"
end
scenario 'master can deny access' do
@@ -35,16 +33,14 @@ feature 'Projects > Members > Master manages access requests', feature: true do
expect_visible_access_request(project, user)
- perform_enqueued_jobs do
- click_on 'Deny access'
- end
+ perform_enqueued_jobs { click_on 'Deny access' }
expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
- expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{project.name_with_namespace} project was denied/
+ expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{project.name_with_namespace} project was denied"
end
def expect_visible_access_request(project, user)
- expect(project.access_requested?(user)).to be_truthy
+ expect(project.members.request.exists?(user_id: user)).to be_truthy
expect(page).to have_content "#{project.name} access requests (1)"
expect(page).to have_content user.name
end
diff --git a/spec/features/projects/members/user_requests_access_spec.rb b/spec/features/projects/members/user_requests_access_spec.rb
index 58a7ec1880d..fd92a3a2f0c 100644
--- a/spec/features/projects/members/user_requests_access_spec.rb
+++ b/spec/features/projects/members/user_requests_access_spec.rb
@@ -8,30 +8,27 @@ feature 'Projects > Members > User requests access', feature: true do
background do
project.team << [master, :master]
login_as(user)
+ visit namespace_project_path(project.namespace, project)
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
+ perform_enqueued_jobs { click_link 'Request Access' }
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(ActionMailer::Base.deliveries.last.subject).to eq "Request to join the #{project.name_with_namespace} project"
- expect(project.access_requested?(user)).to be_truthy
+ expect(project.members.request.exists?(user_id: 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'
+
+ expect(page).to have_content 'Withdraw Access 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
+ expect(project.members.request.exists?(user_id: user)).to be_truthy
+ open_project_settings_menu
click_link 'Members'
visit namespace_project_project_members_path(project.namespace, project)
@@ -41,14 +38,17 @@ feature 'Projects > Members > User requests access', feature: true do
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
+ expect(project.members.request.exists?(user_id: user)).to be_truthy
- click_link 'Withdraw Request'
+ click_link 'Withdraw Access Request'
+
+ expect(project.members.request.exists?(user_id: user)).to be_falsey
+ expect(page).to have_content 'Your access request to the project has been withdrawn.'
+ end
- expect(project.access_requested?(user)).to be_falsey
- expect(page).to have_content 'You withdrawn your access request to the project.'
+ def open_project_settings_menu
+ find('#project-settings-button').click
end
end
diff --git a/spec/helpers/gitlab_routing_helper_spec.rb b/spec/helpers/gitlab_routing_helper_spec.rb
new file mode 100644
index 00000000000..14847d0a49e
--- /dev/null
+++ b/spec/helpers/gitlab_routing_helper_spec.rb
@@ -0,0 +1,79 @@
+require 'spec_helper'
+
+describe GitlabRoutingHelper do
+ describe 'Project URL helpers' do
+ describe '#project_members_url' do
+ let(:project) { build_stubbed(:empty_project) }
+
+ it { expect(project_members_url(project)).to eq namespace_project_project_members_url(project.namespace, project) }
+ end
+
+ describe '#project_member_path' do
+ let(:project_member) { create(:project_member) }
+
+ it { expect(project_member_path(project_member)).to eq namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
+ end
+
+ describe '#request_access_project_members_path' do
+ let(:project) { build_stubbed(:empty_project) }
+
+ it { expect(request_access_project_members_path(project)).to eq request_access_namespace_project_project_members_path(project.namespace, project) }
+ end
+
+ describe '#leave_project_members_path' do
+ let(:project) { build_stubbed(:empty_project) }
+
+ it { expect(leave_project_members_path(project)).to eq leave_namespace_project_project_members_path(project.namespace, project) }
+ end
+
+ describe '#approve_access_request_project_member_path' do
+ let(:project_member) { create(:project_member) }
+
+ it { expect(approve_access_request_project_member_path(project_member)).to eq approve_access_request_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
+ end
+
+ describe '#resend_invite_project_member_path' do
+ let(:project_member) { create(:project_member) }
+
+ it { expect(resend_invite_project_member_path(project_member)).to eq resend_invite_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
+ end
+ end
+
+ describe 'Group URL helpers' do
+ describe '#group_members_url' do
+ let(:group) { build_stubbed(:group) }
+
+ it { expect(group_members_url(group)).to eq group_group_members_url(group) }
+ end
+
+ describe '#group_member_path' do
+ let(:group_member) { create(:group_member) }
+
+ it { expect(group_member_path(group_member)).to eq group_group_member_path(group_member.source, group_member) }
+ end
+
+ describe '#request_access_group_members_path' do
+ let(:group) { build_stubbed(:group) }
+
+ it { expect(request_access_group_members_path(group)).to eq request_access_group_group_members_path(group) }
+ end
+
+ describe '#leave_group_members_path' do
+ let(:group) { build_stubbed(:group) }
+
+ it { expect(leave_group_members_path(group)).to eq leave_group_group_members_path(group) }
+ end
+
+ describe '#approve_access_request_group_member_path' do
+ let(:group_member) { create(:group_member) }
+
+ it { expect(approve_access_request_group_member_path(group_member)).to eq approve_access_request_group_group_member_path(group_member.source, group_member) }
+ end
+
+ describe '#resend_invite_group_member_path' do
+ let(:group_member) { create(:group_member) }
+
+ it { expect(resend_invite_group_member_path(group_member)).to eq resend_invite_group_group_member_path(group_member.source, group_member) }
+ end
+ end
+end
diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb
index c2f10e1db75..0b1a76156e0 100644
--- a/spec/helpers/members_helper_spec.rb
+++ b/spec/helpers/members_helper_spec.rb
@@ -1,22 +1,6 @@
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) }
@@ -25,73 +9,20 @@ describe MembersHelper do
it { expect(action_member_permission(:admin, group_member)).to eq :admin_group_member }
end
- describe '#can_see_entity_roles?' do
- let(:project) { create(:project) }
+ describe '#can_see_member_roles?' do
+ let(:project) { create(:empty_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_access_request_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_access_request_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?" }
+ it { expect(can_see_member_roles?(source: project, user: nil)).to be_falsy }
+ it { expect(can_see_member_roles?(source: group, user: nil)).to be_falsy }
+ it { expect(can_see_member_roles?(source: project, user: admin)).to be_truthy }
+ it { expect(can_see_member_roles?(source: group, user: admin)).to be_truthy }
+ it { expect(can_see_member_roles?(source: project, user: project_member.user)).to be_truthy }
+ it { expect(can_see_member_roles?(source: group, user: group_member.user)).to be_truthy }
end
describe '#remove_member_message' do
@@ -105,12 +36,14 @@ describe MembersHelper do
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?" }
+ it { expect(remove_member_message(project_member)).to eq "Are you sure you want to remove #{project_member.user.name} from the #{project.name_with_namespace} project?" }
+ it { expect(remove_member_message(project_member_invite)).to eq "Are you sure you want to revoke the invitation for #{project_member_invite.invite_email} to join the #{project.name_with_namespace} project?" }
+ it { expect(remove_member_message(project_member_request)).to eq "Are you sure you want to deny #{requester.name}'s request to join the #{project.name_with_namespace} project?" }
+ it { expect(remove_member_message(project_member_request, user: requester)).to eq "Are you sure you want to withdraw your access request for the #{project.name_with_namespace} project?" }
+ it { expect(remove_member_message(group_member)).to eq "Are you sure you want to remove #{group_member.user.name} from the #{group.name} group?" }
+ it { expect(remove_member_message(group_member_invite)).to eq "Are you sure you want to revoke the invitation for #{group_member_invite.invite_email} to join the #{group.name} group?" }
+ it { expect(remove_member_message(group_member_request)).to eq "Are you sure you want to deny #{requester.name}'s request to join the #{group.name} group?" }
+ it { expect(remove_member_message(group_member_request, user: requester)).to eq "Are you sure you want to withdraw your access request for the #{group.name} group?" }
end
describe '#remove_member_title' do
@@ -122,10 +55,10 @@ describe MembersHelper do
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' }
+ it { expect(remove_member_title(project_member)).to eq 'Remove user from project' }
+ it { expect(remove_member_title(project_member_request)).to eq 'Deny access request from project' }
+ it { expect(remove_member_title(group_member)).to eq 'Remove user from group' }
+ it { expect(remove_member_title(group_member_request)).to eq 'Deny access request from group' }
end
describe '#leave_confirmation_message' do
@@ -133,7 +66,7 @@ describe MembersHelper do
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?" }
+ it { expect(leave_confirmation_message(project)).to eq "Are you sure you want to leave the \"#{project.name_with_namespace}\" project?" }
+ it { expect(leave_confirmation_message(group)).to eq "Are you sure you want to leave the \"#{group.name}\" group?" }
end
end
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index fa81c28849e..09e0bbfd00b 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -1,25 +1,6 @@
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")
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index a86ec865b5d..1e6eb20ab39 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -405,7 +405,7 @@ describe Notify do
let(:user) { create(:user) }
let(:project_member) do
project.request_access(user)
- project.project_members.find_by(created_by_id: user.id)
+ project.members.request.find_by(user_id: user.id)
end
subject { Notify.member_access_requested_email('project', project_member.id) }
@@ -413,10 +413,12 @@ describe Notify do
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
- it { is_expected.to have_subject "Request to join the #{project.name_with_namespace} project" }
- it { is_expected.to have_body_text /#{project.name_with_namespace}/ }
- it { is_expected.to have_body_text /#{project.web_url}/ }
- it { is_expected.to have_body_text /#{project_member.human_access}/ }
+ it 'contains all the useful information' do
+ is_expected.to have_subject "Request to join the #{project.name_with_namespace} project"
+ is_expected.to have_body_text /#{project.name_with_namespace}/
+ is_expected.to have_body_text /#{namespace_project_project_members_url(project.namespace, project)}/
+ is_expected.to have_body_text /#{project_member.human_access}/
+ end
end
describe 'project access denied' do
@@ -424,7 +426,7 @@ describe Notify do
let(:user) { create(:user) }
let(:project_member) do
project.request_access(user)
- project.project_members.find_by(created_by_id: user.id)
+ project.members.request.find_by(user_id: user.id)
end
subject { Notify.member_access_denied_email('project', project.id, user.id) }
@@ -432,9 +434,11 @@ describe Notify do
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
- it { is_expected.to have_subject "Access to the #{project.name_with_namespace} project was denied" }
- it { is_expected.to have_body_text /#{project.name_with_namespace}/ }
- it { is_expected.to have_body_text /#{project.web_url}/ }
+ it 'contains all the useful information' do
+ is_expected.to have_subject "Access to the #{project.name_with_namespace} project was denied"
+ is_expected.to have_body_text /#{project.name_with_namespace}/
+ is_expected.to have_body_text /#{project.web_url}/
+ end
end
describe 'project access changed' do
@@ -447,10 +451,12 @@ describe Notify do
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
- it { is_expected.to have_subject "Access to the #{project.name_with_namespace} project was granted" }
- it { is_expected.to have_body_text /#{project.name_with_namespace}/ }
- it { is_expected.to have_body_text /#{project.web_url}/ }
- it { is_expected.to have_body_text /#{project_member.human_access}/ }
+ it 'contains all the useful information' do
+ is_expected.to have_subject "Access to the #{project.name_with_namespace} project was granted"
+ is_expected.to have_body_text /#{project.name_with_namespace}/
+ is_expected.to have_body_text /#{project.web_url}/
+ is_expected.to have_body_text /#{project_member.human_access}/
+ end
end
def invite_to_project(project:, email:, inviter:)
@@ -470,11 +476,13 @@ describe Notify do
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
- it { is_expected.to have_subject "Invitation to join the #{project.name_with_namespace} project" }
- it { is_expected.to have_body_text /#{project.name_with_namespace}/ }
- it { is_expected.to have_body_text /#{project.web_url}/ }
- it { is_expected.to have_body_text /#{project_member.human_access}/ }
- it { is_expected.to have_body_text /#{project_member.invite_token}/ }
+ it 'contains all the useful information' do
+ is_expected.to have_subject "Invitation to join the #{project.name_with_namespace} project"
+ is_expected.to have_body_text /#{project.name_with_namespace}/
+ is_expected.to have_body_text /#{project.web_url}/
+ is_expected.to have_body_text /#{project_member.human_access}/
+ is_expected.to have_body_text /#{project_member.invite_token}/
+ end
end
describe 'project invitation accepted' do
@@ -493,11 +501,13 @@ describe Notify do
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
- it { is_expected.to have_subject 'Invitation accepted' }
- it { is_expected.to have_body_text /#{project.name_with_namespace}/ }
- it { is_expected.to have_body_text /#{project.web_url}/ }
- it { is_expected.to have_body_text /#{project_member.invite_email}/ }
- it { is_expected.to have_body_text /#{invited_user.name}/ }
+ it 'contains all the useful information' do
+ is_expected.to have_subject 'Invitation accepted'
+ is_expected.to have_body_text /#{project.name_with_namespace}/
+ is_expected.to have_body_text /#{project.web_url}/
+ is_expected.to have_body_text /#{project_member.invite_email}/
+ is_expected.to have_body_text /#{invited_user.name}/
+ end
end
describe 'project invitation declined' do
@@ -515,10 +525,12 @@ describe Notify do
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
- it { is_expected.to have_subject 'Invitation declined' }
- it { is_expected.to have_body_text /#{project.name_with_namespace}/ }
- it { is_expected.to have_body_text /#{project.web_url}/ }
- it { is_expected.to have_body_text /#{project_member.invite_email}/ }
+ it 'contains all the useful information' do
+ is_expected.to have_subject 'Invitation declined'
+ is_expected.to have_body_text /#{project.name_with_namespace}/
+ is_expected.to have_body_text /#{project.web_url}/
+ is_expected.to have_body_text /#{project_member.invite_email}/
+ end
end
context 'items that are noteable, the email for a note' do
@@ -639,7 +651,7 @@ describe Notify do
let(:user) { create(:user) }
let(:group_member) do
group.request_access(user)
- group.group_members.find_by(created_by_id: user.id)
+ group.members.request.find_by(user_id: user.id)
end
subject { Notify.member_access_requested_email('group', group_member.id) }
@@ -647,10 +659,12 @@ describe Notify do
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
- it { is_expected.to have_subject "Request to join the #{group.name} group" }
- it { is_expected.to have_body_text /#{group.name}/ }
- it { is_expected.to have_body_text /#{group.web_url}/ }
- it { is_expected.to have_body_text /#{group_member.human_access}/ }
+ it 'contains all the useful information' do
+ is_expected.to have_subject "Request to join the #{group.name} group"
+ is_expected.to have_body_text /#{group.name}/
+ is_expected.to have_body_text /#{group_group_members_url(group)}/
+ is_expected.to have_body_text /#{group_member.human_access}/
+ end
end
describe 'group access denied' do
@@ -658,7 +672,7 @@ describe Notify do
let(:user) { create(:user) }
let(:group_member) do
group.request_access(user)
- group.group_members.find_by(created_by_id: user.id)
+ group.members.request.find_by(user_id: user.id)
end
subject { Notify.member_access_denied_email('group', group.id, user.id) }
@@ -666,9 +680,11 @@ describe Notify do
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
- it { is_expected.to have_subject "Access to the #{group.name} group was denied" }
- it { is_expected.to have_body_text /#{group.name}/ }
- it { is_expected.to have_body_text /#{group.web_url}/ }
+ it 'contains all the useful information' do
+ is_expected.to have_subject "Access to the #{group.name} group was denied"
+ is_expected.to have_body_text /#{group.name}/
+ is_expected.to have_body_text /#{group.web_url}/
+ end
end
describe 'group access changed' do
@@ -682,10 +698,12 @@ describe Notify do
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
- it { is_expected.to have_subject "Access to the #{group.name} group was granted" }
- it { is_expected.to have_body_text /#{group.name}/ }
- it { is_expected.to have_body_text /#{group.web_url}/ }
- it { is_expected.to have_body_text /#{group_member.human_access}/ }
+ it 'contains all the useful information' do
+ is_expected.to have_subject "Access to the #{group.name} group was granted"
+ is_expected.to have_body_text /#{group.name}/
+ is_expected.to have_body_text /#{group.web_url}/
+ is_expected.to have_body_text /#{group_member.human_access}/
+ end
end
def invite_to_group(group:, email:, inviter:)
@@ -705,11 +723,13 @@ describe Notify do
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
- it { is_expected.to have_subject "Invitation to join the #{group.name} group" }
- it { is_expected.to have_body_text /#{group.name}/ }
- it { is_expected.to have_body_text /#{group.web_url}/ }
- it { is_expected.to have_body_text /#{group_member.human_access}/ }
- it { is_expected.to have_body_text /#{group_member.invite_token}/ }
+ it 'contains all the useful information' do
+ is_expected.to have_subject "Invitation to join the #{group.name} group"
+ is_expected.to have_body_text /#{group.name}/
+ is_expected.to have_body_text /#{group.web_url}/
+ is_expected.to have_body_text /#{group_member.human_access}/
+ is_expected.to have_body_text /#{group_member.invite_token}/
+ end
end
describe 'group invitation accepted' do
@@ -728,11 +748,13 @@ describe Notify do
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
- it { is_expected.to have_subject 'Invitation accepted' }
- it { is_expected.to have_body_text /#{group.name}/ }
- it { is_expected.to have_body_text /#{group.web_url}/ }
- it { is_expected.to have_body_text /#{group_member.invite_email}/ }
- it { is_expected.to have_body_text /#{invited_user.name}/ }
+ it 'contains all the useful information' do
+ is_expected.to have_subject 'Invitation accepted'
+ is_expected.to have_body_text /#{group.name}/
+ is_expected.to have_body_text /#{group.web_url}/
+ is_expected.to have_body_text /#{group_member.invite_email}/
+ is_expected.to have_body_text /#{invited_user.name}/
+ end
end
describe 'group invitation declined' do
@@ -750,10 +772,12 @@ describe Notify do
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
- it { is_expected.to have_subject 'Invitation declined' }
- it { is_expected.to have_body_text /#{group.name}/ }
- it { is_expected.to have_body_text /#{group.web_url}/ }
- it { is_expected.to have_body_text /#{group_member.invite_email}/ }
+ it 'contains all the useful information' do
+ is_expected.to have_subject 'Invitation declined'
+ is_expected.to have_body_text /#{group.name}/
+ is_expected.to have_body_text /#{group.web_url}/
+ is_expected.to have_body_text /#{group_member.invite_email}/
+ end
end
end
diff --git a/spec/models/concerns/access_requestable_spec.rb b/spec/models/concerns/access_requestable_spec.rb
index 2dfed1eb4c4..98307876962 100644
--- a/spec/models/concerns/access_requestable_spec.rb
+++ b/spec/models/concerns/access_requestable_spec.rb
@@ -7,8 +7,7 @@ describe AccessRequestable do
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) }
+ it { expect(group.request_access(user).user).to eq(user) }
end
describe '#access_requested?' do
@@ -17,7 +16,7 @@ describe AccessRequestable do
before { group.request_access(user) }
- it { expect(group.access_requested?(user)).to be_truthy }
+ it { expect(group.members.request.exists?(user_id: user)).to be_truthy }
end
end
@@ -35,7 +34,7 @@ describe AccessRequestable do
before { project.request_access(user) }
- it { expect(project.access_requested?(user)).to be_truthy }
+ it { expect(project.members.request.exists?(user_id: user)).to be_truthy }
end
end
end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 52f9d57bc0a..ccdcb29f773 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -10,17 +10,6 @@ describe Group, models: true do
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
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index a3d525d8d56..3ed3202ac6c 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -55,45 +55,78 @@ describe Member, models: true do
end
end
- describe 'Scopes' do
+ describe 'Scopes & finders' 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)) }
+ group = create(:group)
+ @owner_user = create(:user).tap { |u| group.add_owner(u) }
+ @owner = group.members.find_by(user_id: @owner_user.id)
+
+ @master_user = create(:user).tap { |u| project.team << [u, :master] }
+ @master = project.members.find_by(user_id: @master_user.id)
+
+ ProjectMember.add_user(project.members, 'toto1@example.com', Gitlab::Access::DEVELOPER, @master_user)
+ @invited_member = project.members.invite.find_by_invite_email('toto1@example.com')
+
+ accepted_invite_user = build(:user)
+ ProjectMember.add_user(project.members, 'toto2@example.com', Gitlab::Access::DEVELOPER, @master_user)
+ @accepted_invite_member = project.members.invite.find_by_invite_email('toto2@example.com').tap { |u| u.accept_invite!(accepted_invite_user) }
requested_user = create(:user).tap { |u| project.request_access(u) }
- @requested_member = project.project_members.find_by(created_by_id: requested_user.id)
+ @requested_member = project.members.request.find_by(user_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 }
+ @accepted_request_member = project.members.request.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request }
end
- describe '#invite' do
+ describe '.invite' do
+ it { expect(described_class.invite).not_to include @master }
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
+ describe '.non_invite' do
+ it { expect(described_class.non_invite).to include @master }
+ it { expect(described_class.non_invite).not_to include @invited_member }
+ it { expect(described_class.non_invite).to include @accepted_invite_member }
+ it { expect(described_class.non_invite).to include @requested_member }
+ it { expect(described_class.non_invite).to include @accepted_request_member }
+ end
+
+ describe '.request' do
+ it { expect(described_class.request).not_to include @master }
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
+ describe '.non_request' do
+ it { expect(described_class.non_request).to include @master }
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
+ describe '.non_pending' do
+ it { expect(described_class.non_pending).to include @master }
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
+
+ describe '.owners_and_masters' do
+ it { expect(described_class.owners_and_masters).to include @owner }
+ it { expect(described_class.owners_and_masters).to include @master }
+ it { expect(described_class.owners_and_masters).not_to include @invited_member }
+ it { expect(described_class.owners_and_masters).not_to include @accepted_invite_member }
+ it { expect(described_class.owners_and_masters).not_to include @requested_member }
+ it { expect(described_class.owners_and_masters).not_to include @accepted_request_member }
+ end
end
describe "Delegate methods" do
@@ -101,6 +134,18 @@ describe Member, models: true do
it { is_expected.to respond_to(:user_email) }
end
+ describe 'Callbacks' do
+ describe 'after_destroy :post_decline_request, if: :request?' do
+ let(:member) { create(:project_member, requested_at: Time.now.utc) }
+
+ it 'calls #post_decline_request' do
+ expect(member).to receive(:post_decline_request)
+
+ member.destroy
+ end
+ end
+ end
+
describe ".add_user" do
let!(:user) { create(:user) }
let(:project) { create(:project) }
@@ -139,18 +184,9 @@ describe Member, models: true do
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
+ let(:member) { create(:project_member, requested_at: Time.now.utc) }
- it 'sets the user' do
- member.accept_request
-
- expect(member.user).to eq(user)
- end
+ it { expect(member.accept_request).to be_truthy }
it 'clears requested_at' do
member.accept_request
@@ -165,25 +201,24 @@ describe Member, models: true do
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) }
+ describe '#invite?' do
+ subject { create(:project_member, invite_email: "user@example.com", user: nil) }
- it 'returns true' do
- expect(member.decline_request).to be_truthy
- end
+ it { is_expected.to be_invite }
+ end
- it 'destroys the member' do
- member.decline_request
+ describe '#request?' do
+ subject { create(:project_member, requested_at: Time.now.utc) }
- expect(member).to be_destroyed
- end
+ it { is_expected.to be_request }
+ end
- it 'calls #after_decline_request' do
- expect(member).to receive(:after_decline_request)
+ describe '#pending?' do
+ let(:invited_member) { create(:project_member, invite_email: "user@example.com", user: nil) }
+ let(:requester) { create(:project_member, requested_at: Time.now.utc) }
- member.decline_request
- end
+ it { expect(invited_member).to be_invite }
+ it { expect(requester).to be_pending }
end
describe "#accept_invite!" do
diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb
index c3070d4cb78..eeb74a462ac 100644
--- a/spec/models/members/group_member_spec.rb
+++ b/spec/models/members/group_member_spec.rb
@@ -51,24 +51,30 @@ describe GroupMember, models: true do
end
end
- describe 'after accept_request' do
- let(:member) { create(:group_member, user: nil, created_by: build_stubbed(:user), requested_at: Time.now) }
+ describe '#after_accept_request' do
+ it 'calls NotificationService.accept_group_access_request' do
+ member = create(:group_member, user: 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
+ member.__send__(:after_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) }
+ describe '#post_decline_request' do
+ it 'calls NotificationService.decline_group_access_request' do
+ member = create(:group_member, user: 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
+ member.__send__(:post_decline_request)
end
end
+
+ describe '#real_source_type' do
+ subject { create(:group_member).real_source_type }
+
+ it { is_expected.to eq 'Group' }
+ end
end
end
diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb
index 99b3c77c6cd..1e466f9c620 100644
--- a/spec/models/members/project_member_spec.rb
+++ b/spec/models/members/project_member_spec.rb
@@ -33,6 +33,12 @@ describe ProjectMember, models: true do
it { is_expected.to include_module(Gitlab::ShellAdapter) }
end
+ describe '#real_source_type' do
+ subject { create(:project_member).real_source_type }
+
+ it { is_expected.to eq 'Project' }
+ end
+
describe "#destroy" do
let(:owner) { create(:project_member, access_level: ProjectMember::OWNER) }
let(:project) { owner.project }
@@ -137,23 +143,23 @@ describe ProjectMember, models: true do
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) }
+ describe '#after_accept_request' do
+ it 'calls NotificationService.new_project_member' do
+ member = create(:project_member, user: 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
+ member.__send__(:after_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) }
+ describe '#post_decline_request' do
+ it 'calls NotificationService.decline_project_access_request' do
+ member = create(:project_member, user: 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
+ member.__send__(:post_decline_request)
end
end
end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index d5a4b73affd..30aa2b70c8d 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -29,17 +29,6 @@ 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
@@ -100,11 +89,17 @@ describe Project, models: true do
it { is_expected.to respond_to(:repo_exists?) }
it { is_expected.to respond_to(:update_merge_requests) }
it { is_expected.to respond_to(:execute_hooks) }
- it { is_expected.to respond_to(:name_with_namespace) }
it { is_expected.to respond_to(:owner) }
it { is_expected.to respond_to(:path_with_namespace) }
end
+ describe '#name_with_namespace' do
+ let(:project) { build_stubbed(:empty_project) }
+
+ it { expect(project.name_with_namespace).to eq "#{project.namespace.human_name} / #{project.name}" }
+ it { expect(project.human_name).to eq project.name_with_namespace }
+ end
+
describe '#to_reference' do
let(:project) { create(:empty_project) }
diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb
index 36b1f439955..9262aeb6ed8 100644
--- a/spec/models/project_team_spec.rb
+++ b/spec/models/project_team_spec.rb
@@ -112,6 +112,28 @@ describe ProjectTeam, models: true do
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'
+ 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'
+ end
+ end
+
describe '#max_member_access' do
let(:requester) { create(:user) }