From 868cb4307f93b2f8fa0d25d9e47e632d0855881e Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Fri, 23 Feb 2018 12:10:57 +0000 Subject: Fix subgroup issue and MR pages empty states and counts Previously, these wouldn't count issues or MRs in subgroups - meaning that if _this_ group had no issues or MRs, we'd show the empty state, which was wrong. --- app/controllers/groups/application_controller.rb | 4 ---- app/controllers/groups_controller.rb | 1 - 2 files changed, 5 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index 4a2bfc1f887..9f3bb60b4cc 100644 --- a/app/controllers/groups/application_controller.rb +++ b/app/controllers/groups/application_controller.rb @@ -18,10 +18,6 @@ class Groups::ApplicationController < ApplicationController @projects ||= GroupProjectsFinder.new(group: group, current_user: current_user).execute end - def group_merge_requests - @group_merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id).execute - end - def authorize_admin_group! unless can?(current_user, :admin_group, group) return render_404 diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 14b9d6c22bd..283c3e5f1e0 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -14,7 +14,6 @@ class GroupsController < Groups::ApplicationController before_action :authorize_create_group!, only: [:new] before_action :group_projects, only: [:projects, :activity, :issues, :merge_requests] - before_action :group_merge_requests, only: [:merge_requests] before_action :event_filter, only: [:activity] before_action :user_actions, only: [:show, :subgroups] -- cgit v1.2.1 From bf41063679b25371b2e64542f2f469b38502edf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 11 Oct 2017 16:47:08 +0200 Subject: Remove explicit audit event log in MembershipActions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move it to Members::ApproveAccessRequestService. Also, note that there was a double audit event log for access request destruction. Signed-off-by: Rémy Coutable --- app/controllers/concerns/membership_actions.rb | 13 +++++++++---- app/controllers/groups/group_members_controller.rb | 9 ++++----- app/controllers/projects/project_members_controller.rb | 9 ++++----- 3 files changed, 17 insertions(+), 14 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index c6b1e443de6..a6f1509b451 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -15,8 +15,9 @@ module MembershipActions end def destroy + member = membershipable.members_and_requesters.find(params[:id]) Members::DestroyService.new(membershipable, current_user, params) - .execute(:all) + .execute(member) respond_to do |format| format.html do @@ -36,14 +37,18 @@ module MembershipActions end def approve_access_request - Members::ApproveAccessRequestService.new(membershipable, current_user, params).execute + access_requester = membershipable.requesters.find(params[:id]) + Members::ApproveAccessRequestService + .new(membershipable, current_user, params) + .execute(access_requester) redirect_to members_page_url end def leave - member = Members::DestroyService.new(membershipable, current_user, user_id: current_user.id) - .execute(:all) + member = membershipable.members_and_requesters.find_by!(user_id: current_user.id) + Members::DestroyService.new(membershipable, current_user) + .execute(member) notice = if member.request? diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 2c371e76313..1efd07835e2 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -28,12 +28,11 @@ class Groups::GroupMembersController < Groups::ApplicationController end def update - @group_member = @group.members_and_requesters.find(params[:id]) + member = @group.members_and_requesters.find(params[:id]) + @group_member = Members::UpdateService + .new(@group, current_user, member_params) + .execute(member) .present(current_user: current_user) - - return render_403 unless can?(current_user, :update_group_member, @group_member) - - @group_member.update_attributes(member_params) end def resend_invite diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index d7372beb9d3..06388055d52 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -27,12 +27,11 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def update - @project_member = @project.members_and_requesters.find(params[:id]) + member = @project.members_and_requesters.find(params[:id]) + @project_member = Members::UpdateService + .new(@project, current_user, member_params) + .execute(member) .present(current_user: current_user) - - return render_403 unless can?(current_user, :update_project_member, @project_member) - - @project_member.update_attributes(member_params) end def resend_invite -- cgit v1.2.1 From e82f629be4b9c91e2611095cd4296e487ed137ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 16 Feb 2018 12:00:25 +0100 Subject: Move the #update action from Project/Member controllers to the MembershipActions concern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/controllers/concerns/membership_actions.rb | 12 ++++++++++++ app/controllers/groups/group_members_controller.rb | 8 -------- app/controllers/projects/project_members_controller.rb | 8 -------- 3 files changed, 12 insertions(+), 16 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index a6f1509b451..82fdb797d2a 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -14,6 +14,18 @@ module MembershipActions end end + def update + member = membershipable.members_and_requesters.find(params[:id]) + @member = Members::UpdateService + .new(membershipable, current_user, member_params) + .execute(member) + .present(current_user: current_user) + + respond_to do |format| + format.js { render 'shared/members/update' } + end + end + def destroy member = membershipable.members_and_requesters.find(params[:id]) Members::DestroyService.new(membershipable, current_user, params) diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 1efd07835e2..23ade14edfd 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -27,14 +27,6 @@ class Groups::GroupMembersController < Groups::ApplicationController @group_member = @group.group_members.new end - def update - member = @group.members_and_requesters.find(params[:id]) - @group_member = Members::UpdateService - .new(@group, current_user, member_params) - .execute(member) - .present(current_user: current_user) - end - def resend_invite redirect_path = group_group_members_path(@group) diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 06388055d52..006d5df767c 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -26,14 +26,6 @@ class Projects::ProjectMembersController < Projects::ApplicationController @project_member = @project.project_members.new end - def update - member = @project.members_and_requesters.find(params[:id]) - @project_member = Members::UpdateService - .new(@project, current_user, member_params) - .execute(member) - .present(current_user: current_user) - end - def resend_invite redirect_path = project_project_members_path(@project) -- cgit v1.2.1 From 1c88d92b3fe174a56080575a14d6b473f17f7d8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 16 Feb 2018 15:10:22 +0100 Subject: Improve Member services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/controllers/admin/groups_controller.rb | 2 +- app/controllers/concerns/membership_actions.rb | 53 ++++++++++++++++------ app/controllers/groups/group_members_controller.rb | 20 -------- .../projects/project_members_controller.rb | 20 -------- 4 files changed, 39 insertions(+), 56 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index a94726887d9..cc38608eda5 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -48,7 +48,7 @@ class Admin::GroupsController < Admin::ApplicationController def members_update member_params = params.permit(:user_ids, :access_level, :expires_at) - result = Members::CreateService.new(@group, current_user, member_params.merge(limit: -1)).execute + result = Members::CreateService.new(current_user, member_params.merge(limit: -1)).execute(@group) if result[:status] == :success redirect_to [:admin, @group], notice: 'Users were successfully added.' diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index 82fdb797d2a..7a6a00b8e13 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -3,33 +3,31 @@ module MembershipActions def create create_params = params.permit(:user_ids, :access_level, :expires_at) - result = Members::CreateService.new(membershipable, current_user, create_params).execute - - redirect_url = members_page_url + result = Members::CreateService.new(current_user, create_params).execute(membershipable) if result[:status] == :success - redirect_to redirect_url, notice: 'Users were successfully added.' + redirect_to members_page_url, notice: 'Users were successfully added.' else - redirect_to redirect_url, alert: result[:message] + redirect_to members_page_url, alert: result[:message] end end def update + update_params = params.require(root_params_key).permit(:access_level, :expires_at) member = membershipable.members_and_requesters.find(params[:id]) - @member = Members::UpdateService - .new(membershipable, current_user, member_params) + member = Members::UpdateService + .new(current_user, update_params) .execute(member) .present(current_user: current_user) respond_to do |format| - format.js { render 'shared/members/update' } + format.js { render 'shared/members/update', locals: { member: member } } end end def destroy member = membershipable.members_and_requesters.find(params[:id]) - Members::DestroyService.new(membershipable, current_user, params) - .execute(member) + Members::DestroyService.new(current_user).execute(member) respond_to do |format| format.html do @@ -51,7 +49,7 @@ module MembershipActions def approve_access_request access_requester = membershipable.requesters.find(params[:id]) Members::ApproveAccessRequestService - .new(membershipable, current_user, params) + .new(current_user, params) .execute(access_requester) redirect_to members_page_url @@ -59,8 +57,7 @@ module MembershipActions def leave member = membershipable.members_and_requesters.find_by!(user_id: current_user.id) - Members::DestroyService.new(membershipable, current_user) - .execute(member) + Members::DestroyService.new(current_user).execute(member) notice = if member.request? @@ -79,17 +76,43 @@ module MembershipActions end end + def resend_invite + member = membershipable.members.find(params[:id]) + + if member.invite? + member.resend_invite + + redirect_to members_page_url, notice: 'The invitation was successfully resent.' + else + redirect_to members_page_url, alert: 'The invitation has already been accepted.' + end + end + protected def membershipable raise NotImplementedError end + def root_params_key + case membershipable + when Namespace + :group_member + when Project + :project_member + else + raise "Unknown membershipable type: #{membershipable}!" + end + end + def members_page_url - if membershipable.is_a?(Project) + case membershipable + when Namespace + polymorphic_url([membershipable, :members]) + when Project project_project_members_path(membershipable) else - polymorphic_url([membershipable, :members]) + raise "Unknown membershipable type: #{membershipable}!" end end diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 23ade14edfd..f210434b2d7 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -27,26 +27,6 @@ class Groups::GroupMembersController < Groups::ApplicationController @group_member = @group.group_members.new end - def resend_invite - redirect_path = group_group_members_path(@group) - - @group_member = @group.group_members.find(params[:id]) - - if @group_member.invite? - @group_member.resend_invite - - redirect_to redirect_path, notice: 'The invitation was successfully resent.' - else - redirect_to redirect_path, alert: 'The invitation has already been accepted.' - end - end - - protected - - def member_params - params.require(:group_member).permit(:access_level, :user_id, :expires_at) - end - # MembershipActions concern alias_method :membershipable, :group end diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 006d5df767c..e9b4679f94c 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -26,20 +26,6 @@ class Projects::ProjectMembersController < Projects::ApplicationController @project_member = @project.project_members.new end - def resend_invite - redirect_path = project_project_members_path(@project) - - @project_member = @project.project_members.find(params[:id]) - - if @project_member.invite? - @project_member.resend_invite - - redirect_to redirect_path, notice: 'The invitation was successfully resent.' - else - redirect_to redirect_path, alert: 'The invitation has already been accepted.' - end - end - def import @projects = current_user.authorized_projects.order_id_desc end @@ -58,12 +44,6 @@ class Projects::ProjectMembersController < Projects::ApplicationController notice: notice) end - protected - - def member_params - params.require(:project_member).permit(:user_id, :access_level, :expires_at) - end - # MembershipActions concern alias_method :membershipable, :project end -- cgit v1.2.1