diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-08-20 21:42:06 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-08-20 21:42:06 +0300 |
commit | 6e4e1050d9dba2b7b2523fdd1768823ab85feef4 (patch) | |
tree | 78be5963ec075d80116a932011d695dd33910b4e /spec/services/design_management | |
parent | 1ce776de4ae122aba3f349c02c17cebeaa8ecf07 (diff) |
Add latest changes from gitlab-org/gitlab@13-3-stable-ee
Diffstat (limited to 'spec/services/design_management')
-rw-r--r-- | spec/services/design_management/move_designs_service_spec.rb | 147 | ||||
-rw-r--r-- | spec/services/design_management/save_designs_service_spec.rb | 171 |
2 files changed, 213 insertions, 105 deletions
diff --git a/spec/services/design_management/move_designs_service_spec.rb b/spec/services/design_management/move_designs_service_spec.rb new file mode 100644 index 00000000000..a05518dc28d --- /dev/null +++ b/spec/services/design_management/move_designs_service_spec.rb @@ -0,0 +1,147 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe DesignManagement::MoveDesignsService do + include DesignManagementTestHelpers + + let_it_be(:issue) { create(:issue) } + let_it_be(:developer) { create(:user, developer_projects: [issue.project]) } + let_it_be(:designs) { create_list(:design, 3, :with_relative_position, issue: issue) } + + let(:project) { issue.project } + + let(:service) { described_class.new(current_user, params) } + + let(:params) do + { + current_design: current_design, + previous_design: previous_design, + next_design: next_design + } + end + + let(:current_user) { developer } + let(:current_design) { nil } + let(:previous_design) { nil } + let(:next_design) { nil } + + before do + enable_design_management + end + + describe '#execute' do + subject { service.execute } + + context 'the feature is unavailable' do + let(:current_design) { designs.first } + let(:previous_design) { designs.second } + let(:next_design) { designs.third } + + before do + stub_feature_flags(reorder_designs: false) + end + + it 'raises cannot_move' do + expect(subject).to be_error.and(have_attributes(message: :cannot_move)) + end + + context 'but it is available on the current project' do + before do + stub_feature_flags(reorder_designs: issue.project) + end + + it 'is successful' do + expect(subject).to be_success + end + end + end + + context 'the user cannot move designs' do + let(:current_design) { designs.first } + let(:current_user) { build_stubbed(:user) } + + it 'raises cannot_move' do + expect(subject).to be_error.and(have_attributes(message: :cannot_move)) + end + end + + context 'the designs are not distinct' do + let(:current_design) { designs.first } + let(:previous_design) { designs.first } + + it 'raises not_distinct' do + expect(subject).to be_error.and(have_attributes(message: :not_distinct)) + end + end + + context 'the designs are not on the same issue' do + let(:current_design) { designs.first } + let(:previous_design) { create(:design) } + + it 'raises not_same_issue' do + expect(subject).to be_error.and(have_attributes(message: :not_same_issue)) + end + end + + context 'no focus is passed' do + let(:previous_design) { designs.second } + let(:next_design) { designs.third } + + it 'raises no_focus' do + expect(subject).to be_error.and(have_attributes(message: :no_focus)) + end + end + + context 'no neighbours are passed' do + let(:current_design) { designs.first } + + it 'raises no_neighbors' do + expect(subject).to be_error.and(have_attributes(message: :no_neighbors)) + end + end + + context 'the designs are not adjacent' do + let(:current_design) { designs.first } + let(:previous_design) { designs.second } + let(:next_design) { designs.third } + + it 'raises not_adjacent' do + create(:design, issue: issue, relative_position: next_design.relative_position - 1) + + expect(subject).to be_error.and(have_attributes(message: :not_adjacent)) + end + end + + context 'moving a design with neighbours' do + let(:current_design) { designs.first } + let(:previous_design) { designs.second } + let(:next_design) { designs.third } + + it 'repositions existing designs and correctly places the given design' do + other_design1 = create(:design, issue: issue, relative_position: 10) + other_design2 = create(:design, issue: issue, relative_position: 20) + other_design3, other_design4 = create_list(:design, 2, issue: issue) + + expect(subject).to be_success + + expect(issue.designs.ordered(issue.project)).to eq([ + # Existing designs which already had a relative_position set. + # These should stay at the beginning, in the same order. + other_design1, + other_design2, + + # The designs we're passing into the service. + # These should be placed between the existing designs, in the correct order. + previous_design, + current_design, + next_design, + + # Existing designs which didn't have a relative_position set. + # These should be placed at the end, in the order of their IDs. + other_design3, + other_design4 + ]) + end + end + end +end diff --git a/spec/services/design_management/save_designs_service_spec.rb b/spec/services/design_management/save_designs_service_spec.rb index 24639632566..abba5de2c27 100644 --- a/spec/services/design_management/save_designs_service_spec.rb +++ b/spec/services/design_management/save_designs_service_spec.rb @@ -5,9 +5,9 @@ RSpec.describe DesignManagement::SaveDesignsService do include DesignManagementTestHelpers include ConcurrentHelpers - let_it_be(:developer) { create(:user) } + let_it_be_with_reload(:issue) { create(:issue) } + let_it_be(:developer) { create(:user, developer_projects: [issue.project]) } let(:project) { issue.project } - let(:issue) { create(:issue) } let(:user) { developer } let(:files) { [rails_sample] } let(:design_repository) { ::Gitlab::GlRepository::DESIGN.repository_resolver.call(project) } @@ -19,8 +19,20 @@ RSpec.describe DesignManagement::SaveDesignsService do fixture_file_upload("spec/fixtures/#{filename}") end + def commit_count + design_repository.expire_statistics_caches + design_repository.expire_root_ref_cache + design_repository.commit_count + end + before do - project.add_developer(developer) + if issue.design_collection.repository.exists? + issue.design_collection.repository.expire_all_method_caches + issue.design_collection.repository.raw.delete_all_refs_except([Gitlab::Git::BLANK_SHA]) + end + + allow(::DesignManagement::NewVersionWorker) + .to receive(:perform_async).with(Integer).and_return(nil) end def run_service(files_to_upload = nil) @@ -83,24 +95,20 @@ RSpec.describe DesignManagement::SaveDesignsService do design_repository.exists? end - it 'creates a design repository when it did not exist' do - expect { run_service }.to change { repository_exists }.from(false).to(true) + it 'is ensured when the service runs' do + run_service + + expect(repository_exists).to be true end end - it 'updates the creation count' do + it 'creates a commit, an event in the activity stream and updates the creation count' do counter = Gitlab::UsageDataCounters::DesignsCounter - expect { run_service }.to change { counter.read(:create) }.by(1) - end - it 'creates an event in the activity stream' do expect { run_service } .to change { Event.count }.by(1) .and change { Event.for_design.created_action.count }.by(1) - end - - it 'creates a commit in the repository' do - run_service + .and change { counter.read(:create) }.by(1) expect(design_repository.commit).to have_attributes( author: user, @@ -109,35 +117,26 @@ RSpec.describe DesignManagement::SaveDesignsService do end it 'can run the same command in parallel' do - blocks = Array.new(10).map do - unique_files = %w(rails_sample.jpg dk.png) - .map { |name| RenameableUpload.unique_file(name) } + parellism = 4 + + blocks = Array.new(parellism).map do + unique_files = [RenameableUpload.unique_file('rails_sample.jpg')] -> { run_service(unique_files) } end - expect { run_parallel(blocks) }.to change(DesignManagement::Version, :count).by(10) + expect { run_parallel(blocks) }.to change(DesignManagement::Version, :count).by(parellism) end - it 'causes diff_refs not to be nil' do - expect(response).to include( - designs: all(have_attributes(diff_refs: be_present)) - ) - end - - it 'creates a design & a version for the filename if it did not exist' do - expect(issue.designs.size).to eq(0) - - updated_designs = response[:designs] - - expect(updated_designs.size).to eq(1) - expect(updated_designs.first.versions.size).to eq(1) - end - - it 'saves the user as the author' do - updated_designs = response[:designs] + describe 'the response' do + it 'includes designs with the expected properties' do + updated_designs = response[:designs] - expect(updated_designs.first.versions.first.author).to eq(user) + expect(updated_designs).to all(have_attributes(diff_refs: be_present)) + expect(updated_designs.size).to eq(1) + expect(updated_designs.first.versions.size).to eq(1) + expect(updated_designs.first.versions.first.author).to eq(user) + end end describe 'saving the file to LFS' do @@ -147,14 +146,10 @@ RSpec.describe DesignManagement::SaveDesignsService do end end - it 'saves the design to LFS' do - expect { run_service }.to change { LfsObject.count }.by(1) - end - - it 'saves the repository_type of the LfsObjectsProject as design' do - expect do - run_service - end.to change { project.lfs_objects_projects.count }.from(0).to(1) + it 'saves the design to LFS and saves the repository_type of the LfsObjectsProject as design' do + expect { run_service } + .to change { LfsObject.count }.by(1) + .and change { project.lfs_objects_projects.count }.from(0).to(1) expect(project.lfs_objects_projects.first.repository_type).to eq('design') end @@ -202,12 +197,10 @@ RSpec.describe DesignManagement::SaveDesignsService do run_service end - it 'does not create a new version' do - expect { run_service }.not_to change { issue.design_versions.count } - end + it 'does not create a new version, and returns the design in `skipped_designs`' do + response = nil - it 'returns the design in `skipped_designs` instead of `designs`' do - response = run_service + expect { response = run_service }.not_to change { issue.design_versions.count } expect(response[:designs]).to be_empty expect(response[:skipped_designs].size).to eq(1) @@ -223,35 +216,20 @@ RSpec.describe DesignManagement::SaveDesignsService do touch_files([files.first]) end - it 'counts one creation and one update' do + it 'has the correct side-effects' do counter = Gitlab::UsageDataCounters::DesignsCounter - expect { run_service } - .to change { counter.read(:create) }.by(1) - .and change { counter.read(:update) }.by(1) - end - it 'creates the correct activity stream events' do + expect(::DesignManagement::NewVersionWorker) + .to receive(:perform_async).once.with(Integer).and_return(nil) + expect { run_service } .to change { Event.count }.by(2) .and change { Event.for_design.count }.by(2) .and change { Event.created_action.count }.by(1) .and change { Event.updated_action.count }.by(1) - end - - it 'creates a single commit' do - commit_count = -> do - design_repository.expire_all_method_caches - design_repository.commit_count - end - - expect { run_service }.to change { commit_count.call }.by(1) - end - - it 'enqueues just one new version worker' do - expect(::DesignManagement::NewVersionWorker) - .to receive(:perform_async).once.with(Integer) - - run_service + .and change { counter.read(:create) }.by(1) + .and change { counter.read(:update) }.by(1) + .and change { commit_count }.by(1) end end @@ -262,45 +240,28 @@ RSpec.describe DesignManagement::SaveDesignsService do expect(response).to include(designs: have_attributes(size: 2), status: :success) end - it 'creates 2 designs with a single version' do - expect { run_service }.to change { issue.designs.count }.from(0).to(2) - - expect(DesignManagement::Version.for_designs(issue.designs).size).to eq(1) - end - - it 'increments the creation count by 2' do + it 'has the correct side-effects', :request_store do counter = Gitlab::UsageDataCounters::DesignsCounter - expect { run_service }.to change { counter.read(:create) }.by 2 - end - - it 'enqueues a new version worker' do - expect(::DesignManagement::NewVersionWorker) - .to receive(:perform_async).once.with(Integer) - - run_service - end - - it 'creates a single commit' do - commit_count = -> do - design_repository.expire_all_method_caches - design_repository.commit_count - end - - expect { run_service }.to change { commit_count.call }.by(1) - end - - it 'only does 5 gitaly calls', :request_store, :sidekiq_might_not_need_inline do - allow(::DesignManagement::NewVersionWorker).to receive(:perform_async).with(Integer) service = described_class.new(project, user, issue: issue, files: files) + # Some unrelated calls that are usually cached or happen only once - service.__send__(:repository).create_if_not_exists - service.__send__(:repository).has_visible_content? + # We expect: + # - An exists? + # - a check for existing blobs + # - default branch + # - an after_commit callback on LfsObjectsProject + design_repository.create_if_not_exists + design_repository.has_visible_content? - request_count = -> { Gitlab::GitalyClient.get_request_count } + expect(::DesignManagement::NewVersionWorker) + .to receive(:perform_async).once.with(Integer).and_return(nil) - # An exists?, a check for existing blobs, default branch, an after_commit - # callback on LfsObjectsProject - expect { service.execute }.to change(&request_count).by(4) + expect { service.execute } + .to change { issue.designs.count }.from(0).to(2) + .and change { DesignManagement::Version.count }.by(1) + .and change { counter.read(:create) }.by(2) + .and change { Gitlab::GitalyClient.get_request_count }.by(3) + .and change { commit_count }.by(1) end context 'when uploading too many files' do @@ -313,7 +274,7 @@ RSpec.describe DesignManagement::SaveDesignsService do end context 'when the user is not allowed to upload designs' do - let(:user) { create(:user) } + let(:user) { build_stubbed(:user) } it_behaves_like 'a service error' end |