Skip to content
Snippets Groups Projects
Verified Commit aa792806 authored by Dylan Griffith's avatar Dylan Griffith Committed by GitLab
Browse files

Merge branch 'revert_async_members_destroy' into 'master'

Revert "Merge branch 'vij-async-billable-destroy' into 'master' "

See merge request !163700



Merged-by: default avatarDylan Griffith <dyl.griffith@gmail.com>
Approved-by: default avatarDylan Griffith <dyl.griffith@gmail.com>
Approved-by: Sylvester Chin's avatarSylvester Chin <schin@gitlab.com>
Co-authored-by: Thong Kuah's avatarThong Kuah <tkuah@gitlab.com>
parents 40582d63 9744e900
No related branches found
No related tags found
2 merge requests!164749Enable parallel in test-on-omnibus,!163700Revert "Merge branch 'vij-async-billable-destroy' into 'master' "
Pipeline #1424264238 failed
Pipeline: devkit

#1424271220

    Pipeline: E2E GDK

    #1424271212

      Pipeline: E2E Omnibus GitLab EE

      #1424268750

        +31
        ......@@ -545,7 +545,6 @@ DELETE /groups/:id/billable_members/:user_id
        | --------- | ---- | -------- | ----------- |
        | `id` | integer/string | yes | The ID or [URL-encoded path of the group](rest/index.md#namespaced-path-encoding) owned by the authenticated user |
        | `user_id` | integer | yes | The user ID of the member |
        | `async` | boolean | no | Remove the billable member asynchronously. **Status:** Beta |
        ```shell
        curl --request DELETE --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/groups/:id/billable_members/:user_id"
        ......
        ......@@ -14,11 +14,10 @@ class DestroyService
        InvalidGroupError = Class.new(StandardError)
        InvalidMemberError = Class.new(StandardError)
        def initialize(group, user_id:, current_user:, async: false)
        def initialize(group, user_id:, current_user:)
        @group = group
        @user_id = user_id
        @current_user = current_user
        @async = async
        end
        def execute
        ......@@ -34,7 +33,7 @@ def execute
        private
        attr_reader :group, :user_id, :current_user, :async
        attr_reader :group, :user_id, :current_user
        # rubocop: disable CodeReuse/ActiveRecord
        def remove_user_from_resources
        ......@@ -43,12 +42,7 @@ def remove_user_from_resources
        memberships.find_each do |member|
        memberships_found = true
        if async
        ::Members::DestroyWorker.perform_async(member.id, current_user.id, skip_subresources: true)
        else
        ::Members::DestroyService.new(current_user).execute(member, skip_subresources: true)
        end
        ::Members::DestroyService.new(current_user).execute(member, skip_subresources: true)
        end
        raise InvalidMemberError, 'No member found for the given user_id' unless memberships_found
        ......
        ......@@ -220,12 +220,11 @@ module Members
        desc 'Removes a billable member from a group or project.'
        params do
        requires :user_id, type: Integer, desc: 'The user ID of the member'
        optional :async, type: Grape::API::Boolean, desc: 'Remove the billable member asynchronously. **Status:** Beta'
        end
        delete ":id/billable_members/:user_id", feature_category: :seat_cost_management do
        group = find_group!(params[:id])
        result = ::BillableMembers::DestroyService.new(group, user_id: params[:user_id], current_user: current_user, async: params[:async]).execute
        result = ::BillableMembers::DestroyService.new(group, user_id: params[:user_id], current_user: current_user).execute
        if result[:status] == :success
        no_content!
        ......
        ......@@ -2,16 +2,16 @@
        require 'spec_helper'
        RSpec.describe BillableMembers::DestroyService, feature_category: :seat_cost_management do
        RSpec.describe BillableMembers::DestroyService do
        describe '#execute' do
        let_it_be(:current_user) { create(:user) }
        let_it_be(:root_group) { create(:group) }
        let(:group) { root_group }
        let(:user_id) { nil }
        let(:async) { false }
        subject(:execute) { described_class.new(group, user_id: user_id, current_user: current_user, async: async).execute }
        subject(:execute) { described_class.new(group, user_id: user_id, current_user: current_user).execute }
        context 'when unauthorized' do
        it 'raises an access error' do
        ......@@ -46,16 +46,6 @@
        end
        end
        shared_examples 'supports async removal' do
        let(:async) { true }
        it 'enqueues the destroy worker' do
        expect(::Members::DestroyWorker).to receive(:perform_async).once
        subject
        end
        end
        context 'when removing a group member' do
        let(:user_id) { group_member.user_id }
        ......@@ -64,8 +54,6 @@
        expect(root_group.members).not_to include(group_member)
        end
        it_behaves_like 'supports async removal'
        end
        context 'when removing a subgroup member' do
        ......@@ -76,8 +64,6 @@
        expect(subgroup.members).not_to include(subgroup_member)
        end
        it_behaves_like 'supports async removal'
        end
        context 'when removing a project member' do
        ......@@ -88,35 +74,21 @@
        expect(project_1.members).not_to include(project_member)
        end
        it_behaves_like 'supports async removal'
        end
        context 'when the user is a direct member of multiple projects' do
        let(:multi_project_user) { create(:user) }
        let(:user_id) { multi_project_user.id }
        before do
        it 'removes the user from all the projects' do
        project_1.add_developer(multi_project_user)
        project_2.add_developer(multi_project_user)
        end
        it 'removes the user from all the projects' do
        execute
        expect(multi_project_user.projects).not_to include(project_1)
        expect(multi_project_user.projects).not_to include(project_2)
        end
        context 'when processing asynchronously' do
        let(:async) { true }
        it 'enqueues a job per membership' do
        expect(::Members::DestroyWorker).to receive(:perform_async).twice
        execute
        end
        end
        end
        context 'when the user has no Member record' do
        ......
        0% Loading or .
        You are about to add 0 people to the discussion. Proceed with caution.
        Finish editing this message first!
        Please register or to comment