diff options
Diffstat (limited to 'spec')
13 files changed, 224 insertions, 137 deletions
diff --git a/spec/factories/events.rb b/spec/factories/events.rb index 768ce30b694..a4f06a48621 100644 --- a/spec/factories/events.rb +++ b/spec/factories/events.rb @@ -54,6 +54,16 @@ FactoryBot.define do target { note } end + trait :for_issue do + target { association(:issue, issue_type: :issue) } + target_type { 'Issue' } + end + + trait :for_work_item do + target { association(:work_item, :task) } + target_type { 'WorkItem' } + end + factory :design_event, traits: [:has_design] do action { :created } target { design } diff --git a/spec/frontend/admin/broadcast_messages/components/base_spec.js b/spec/frontend/admin/broadcast_messages/components/base_spec.js index 65f828ff6b4..020e1c1d7c1 100644 --- a/spec/frontend/admin/broadcast_messages/components/base_spec.js +++ b/spec/frontend/admin/broadcast_messages/components/base_spec.js @@ -1,35 +1,112 @@ import { shallowMount } from '@vue/test-utils'; +import { GlPagination } from '@gitlab/ui'; +import AxiosMockAdapter from 'axios-mock-adapter'; +import { TEST_HOST } from 'helpers/test_constants'; +import waitForPromises from 'helpers/wait_for_promises'; +import { useMockLocationHelper } from 'helpers/mock_window_location_helper'; +import { createAlert } from '~/flash'; +import axios from '~/lib/utils/axios_utils'; +import { redirectTo } from '~/lib/utils/url_utility'; import BroadcastMessagesBase from '~/admin/broadcast_messages/components/base.vue'; import MessagesTable from '~/admin/broadcast_messages/components/messages_table.vue'; -import { MOCK_MESSAGES } from '../mock_data'; +import { generateMockMessages, MOCK_MESSAGES } from '../mock_data'; + +jest.mock('~/flash'); +jest.mock('~/lib/utils/url_utility'); describe('BroadcastMessagesBase', () => { let wrapper; + let axiosMock; + + useMockLocationHelper(); const findTable = () => wrapper.findComponent(MessagesTable); + const findPagination = () => wrapper.findComponent(GlPagination); function createComponent(props = {}) { wrapper = shallowMount(BroadcastMessagesBase, { propsData: { + page: 1, + messagesCount: MOCK_MESSAGES.length, messages: MOCK_MESSAGES, ...props, }, }); } + beforeEach(() => { + axiosMock = new AxiosMockAdapter(axios); + }); + afterEach(() => { + axiosMock.restore(); wrapper.destroy(); }); - it('renders the table when there are existing messages', () => { + it('renders the table and pagination when there are existing messages', () => { createComponent(); expect(findTable().exists()).toBe(true); + expect(findPagination().exists()).toBe(true); }); - it('does not render the table when there are no existing messages', () => { + it('does not render the table when there are no visible messages', () => { createComponent({ messages: [] }); expect(findTable().exists()).toBe(false); + expect(findPagination().exists()).toBe(true); + }); + + it('does not remove a deleted message if it was not in visibleMessages', async () => { + createComponent(); + + findTable().vm.$emit('delete-message', -1); + await waitForPromises(); + + expect(axiosMock.history.delete).toHaveLength(0); + expect(wrapper.vm.visibleMessages.length).toBe(MOCK_MESSAGES.length); + }); + + it('does not remove a deleted message if the request fails', async () => { + createComponent(); + const { id, delete_path } = MOCK_MESSAGES[0]; + axiosMock.onDelete(delete_path).replyOnce(500); + + findTable().vm.$emit('delete-message', id); + await waitForPromises(); + + expect(wrapper.vm.visibleMessages.find((m) => m.id === id)).not.toBeUndefined(); + expect(createAlert).toHaveBeenCalledWith( + expect.objectContaining({ + message: BroadcastMessagesBase.i18n.deleteError, + }), + ); + }); + + it('removes a deleted message from visibleMessages on success', async () => { + createComponent(); + const { id, delete_path } = MOCK_MESSAGES[0]; + axiosMock.onDelete(delete_path).replyOnce(200); + + findTable().vm.$emit('delete-message', id); + await waitForPromises(); + + expect(wrapper.vm.visibleMessages.find((m) => m.id === id)).toBeUndefined(); + expect(wrapper.vm.totalMessages).toBe(MOCK_MESSAGES.length - 1); + }); + + it('redirects to the first page when totalMessages changes from 21 to 20', async () => { + window.location.pathname = `${TEST_HOST}/admin/broadcast_messages`; + + const messages = generateMockMessages(21); + const { id, delete_path } = messages[0]; + createComponent({ messages, messagesCount: messages.length }); + + axiosMock.onDelete(delete_path).replyOnce(200); + + findTable().vm.$emit('delete-message', id); + await waitForPromises(); + + expect(redirectTo).toHaveBeenCalledWith(`${TEST_HOST}/admin/broadcast_messages?page=1`); }); }); diff --git a/spec/frontend/admin/broadcast_messages/components/messages_table_row_spec.js b/spec/frontend/admin/broadcast_messages/components/messages_table_row_spec.js deleted file mode 100644 index 0b3e2e2a75a..00000000000 --- a/spec/frontend/admin/broadcast_messages/components/messages_table_row_spec.js +++ /dev/null @@ -1,26 +0,0 @@ -import { shallowMount } from '@vue/test-utils'; -import MessagesTableRow from '~/admin/broadcast_messages/components/messages_table_row.vue'; -import { MOCK_MESSAGE } from '../mock_data'; - -describe('MessagesTableRow', () => { - let wrapper; - - function createComponent(props = {}) { - wrapper = shallowMount(MessagesTableRow, { - propsData: { - message: MOCK_MESSAGE, - ...props, - }, - }); - } - - afterEach(() => { - wrapper.destroy(); - }); - - it('renders the message ID', () => { - createComponent(); - - expect(wrapper.text()).toBe(`${MOCK_MESSAGE.id}`); - }); -}); diff --git a/spec/frontend/admin/broadcast_messages/components/messages_table_spec.js b/spec/frontend/admin/broadcast_messages/components/messages_table_spec.js index 0699fa34024..349fab03853 100644 --- a/spec/frontend/admin/broadcast_messages/components/messages_table_spec.js +++ b/spec/frontend/admin/broadcast_messages/components/messages_table_spec.js @@ -1,15 +1,19 @@ -import { shallowMount } from '@vue/test-utils'; +import { mount } from '@vue/test-utils'; import MessagesTable from '~/admin/broadcast_messages/components/messages_table.vue'; -import MessagesTableRow from '~/admin/broadcast_messages/components/messages_table_row.vue'; import { MOCK_MESSAGES } from '../mock_data'; describe('MessagesTable', () => { let wrapper; - const findRows = () => wrapper.findAllComponents(MessagesTableRow); + const findRows = () => wrapper.findAll('[data-testid="message-row"]'); + const findTargetRoles = () => wrapper.find('[data-testid="target-roles-th"]'); + const findDeleteButton = (id) => wrapper.find(`[data-testid="delete-message-${id}"]`); - function createComponent(props = {}) { - wrapper = shallowMount(MessagesTable, { + function createComponent(props = {}, glFeatures = {}) { + wrapper = mount(MessagesTable, { + provide: { + glFeatures, + }, propsData: { messages: MOCK_MESSAGES, ...props, @@ -26,4 +30,22 @@ describe('MessagesTable', () => { expect(findRows()).toHaveLength(MOCK_MESSAGES.length); }); + + it('renders the "Target Roles" column when roleTargetedBroadcastMessages is enabled', () => { + createComponent({}, { roleTargetedBroadcastMessages: true }); + expect(findTargetRoles().exists()).toBe(true); + }); + + it('does not render the "Target Roles" column when roleTargetedBroadcastMessages is disabled', () => { + createComponent(); + expect(findTargetRoles().exists()).toBe(false); + }); + + it('emits a delete-message event when a delete button is clicked', () => { + const { id } = MOCK_MESSAGES[0]; + createComponent(); + findDeleteButton(id).element.click(); + expect(wrapper.emitted('delete-message')).toHaveLength(1); + expect(wrapper.emitted('delete-message')[0]).toEqual([id]); + }); }); diff --git a/spec/frontend/admin/broadcast_messages/mock_data.js b/spec/frontend/admin/broadcast_messages/mock_data.js index f176e43a535..8dd98c2319d 100644 --- a/spec/frontend/admin/broadcast_messages/mock_data.js +++ b/spec/frontend/admin/broadcast_messages/mock_data.js @@ -1,5 +1,17 @@ -export const MOCK_MESSAGE = { - id: 100, -}; +const generateMockMessage = (id) => ({ + id, + delete_path: `/admin/broadcast_messages/${id}.js`, + edit_path: `/admin/broadcast_messages/${id}/edit`, + starts_at: new Date().toISOString(), + ends_at: new Date().toISOString(), + preview: '<div>YEET</div>', + status: 'Expired', + target_path: '*/welcome', + target_roles: 'Maintainer, Owner', + type: 'Banner', +}); -export const MOCK_MESSAGES = [MOCK_MESSAGE, { id: 200 }, { id: 300 }]; +export const generateMockMessages = (n) => + [...Array(n).keys()].map((id) => generateMockMessage(id + 1)); + +export const MOCK_MESSAGES = generateMockMessages(5).map((id) => generateMockMessage(id)); diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index 517897311e1..4944142de5f 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -314,15 +314,15 @@ RSpec.describe GitlabSchema do end describe '.parse_gids' do - let_it_be(:global_ids) { %w[gid://gitlab/TestOne/123 gid://gitlab/TestOne/456] } + let_it_be(:global_ids) { %w[gid://gitlab/TestOne/123 gid://gitlab/TestTwo/456] } - subject(:parse_gids) { described_class.parse_gids(global_ids, expected_type: TestOne) } + subject(:parse_gids) { described_class.parse_gids(global_ids, expected_type: [TestOne, TestTwo]) } it 'parses the gids' do - expect(described_class).to receive(:parse_gid).with('gid://gitlab/TestOne/123', expected_type: TestOne).and_call_original - expect(described_class).to receive(:parse_gid).with('gid://gitlab/TestOne/456', expected_type: TestOne).and_call_original + expect(described_class).to receive(:parse_gid).with('gid://gitlab/TestOne/123', expected_type: [TestOne, TestTwo]).and_call_original + expect(described_class).to receive(:parse_gid).with('gid://gitlab/TestTwo/456', expected_type: [TestOne, TestTwo]).and_call_original expect(parse_gids.map(&:model_id)).to eq %w[123 456] - expect(parse_gids.map(&:model_class)).to match_array [TestOne, TestOne] + expect(parse_gids.map(&:model_class)).to eq [TestOne, TestTwo] end end end diff --git a/spec/presenters/event_presenter_spec.rb b/spec/presenters/event_presenter_spec.rb index 5a67fd92c9d..9093791421d 100644 --- a/spec/presenters/event_presenter_spec.rb +++ b/spec/presenters/event_presenter_spec.rb @@ -51,6 +51,14 @@ RSpec.describe EventPresenter do it 'returns milestone for a milestone event' do expect(group_event.present).to have_attributes(target_type_name: 'milestone') end + + it 'returns the issue_type for issue events' do + expect(build(:event, :for_issue, :created).present).to have_attributes(target_type_name: 'issue') + end + + it 'returns the issue_type for work item events' do + expect(build(:event, :for_work_item, :created).present).to have_attributes(target_type_name: 'task') + end end describe '#note_target_type_name' do diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index b7c73467e8e..d771c1e2dcc 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -438,21 +438,7 @@ RSpec.describe API::MavenPackages do it_behaves_like 'processing HEAD requests', instance_level: true end - context 'with check_maven_path_first enabled' do - before do - stub_feature_flags(check_maven_path_first: true) - end - - it_behaves_like 'handling groups, subgroups and user namespaces for', 'heading a file' - end - - context 'with check_maven_path_first disabled' do - before do - stub_feature_flags(check_maven_path_first: false) - end - - it_behaves_like 'handling groups, subgroups and user namespaces for', 'heading a file' - end + it_behaves_like 'handling groups, subgroups and user namespaces for', 'heading a file' end describe 'GET /api/v4/groups/:id/-/packages/maven/*path/:file_name' do @@ -668,21 +654,7 @@ RSpec.describe API::MavenPackages do let(:path) { package.maven_metadatum.path } let(:url) { "/groups/#{group.id}/-/packages/maven/#{path}/#{package_file.file_name}" } - context 'with check_maven_path_first enabled' do - before do - stub_feature_flags(check_maven_path_first: true) - end - - it_behaves_like 'handling groups and subgroups for', 'processing HEAD requests' - end - - context 'with check_maven_path_first disabled' do - before do - stub_feature_flags(check_maven_path_first: false) - end - - it_behaves_like 'handling groups and subgroups for', 'processing HEAD requests' - end + it_behaves_like 'handling groups and subgroups for', 'processing HEAD requests' end describe 'GET /api/v4/projects/:id/packages/maven/*path/:file_name' do @@ -774,21 +746,7 @@ RSpec.describe API::MavenPackages do let(:path) { package.maven_metadatum.path } let(:url) { "/projects/#{project.id}/packages/maven/#{path}/#{package_file.file_name}" } - context 'with check_maven_path_first enabled' do - before do - stub_feature_flags(check_maven_path_first: true) - end - - it_behaves_like 'processing HEAD requests' - end - - context 'with check_maven_path_first disabled' do - before do - stub_feature_flags(check_maven_path_first: false) - end - - it_behaves_like 'processing HEAD requests' - end + it_behaves_like 'processing HEAD requests' end describe 'PUT /api/v4/projects/:id/packages/maven/*path/:file_name/authorize' do diff --git a/spec/rubocop/cop/rspec/factory_bot/avoid_create_spec.rb b/spec/rubocop/cop/rspec/factory_bot/avoid_create_spec.rb new file mode 100644 index 00000000000..7f45661c13d --- /dev/null +++ b/spec/rubocop/cop/rspec/factory_bot/avoid_create_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'rubocop_spec_helper' + +require_relative '../../../../../rubocop/cop/rspec/factory_bot/avoid_create' + +RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AvoidCreate do + shared_examples 'an offensive factory call' do |namespace| + %i[create create_list].each do |forbidden_method| + namespaced_forbidden_method = "#{namespace}#{forbidden_method}(:user)" + + it "registers an offense for #{namespaced_forbidden_method}" do + expect_offense(<<-RUBY) + describe 'foo' do + let(:user) { #{namespaced_forbidden_method} } + #{'^' * namespaced_forbidden_method.size} Prefer using `build_stubbed` or similar over `#{forbidden_method}`. See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#optimize-factory-usage + end + RUBY + end + end + end + + it_behaves_like 'an offensive factory call', '' + it_behaves_like 'an offensive factory call', 'FactoryBot.' +end diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index 8f448184b45..b3c4ed4c544 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -9,6 +9,7 @@ RSpec.describe MergeRequests::CloseService do let(:merge_request) { create(:merge_request, assignees: [user2], author: create(:user)) } let(:project) { merge_request.project } let!(:todo) { create(:todo, :assigned, user: user, project: project, target: merge_request, author: user2) } + let(:service) { described_class.new(project: project, current_user: user) } before do project.add_maintainer(user) @@ -16,18 +17,20 @@ RSpec.describe MergeRequests::CloseService do project.add_guest(guest) end + def execute + service.execute(merge_request) + end + describe '#execute' do it_behaves_like 'cache counters invalidator' it_behaves_like 'merge request reviewers cache counters invalidator' context 'valid params' do - let(:service) { described_class.new(project: project, current_user: user) } - before do allow(service).to receive(:execute_hooks) perform_enqueued_jobs do - @merge_request = service.execute(merge_request) + @merge_request = execute end end @@ -73,7 +76,7 @@ RSpec.describe MergeRequests::CloseService do expect(metrics_service).to receive(:close) - described_class.new(project: project, current_user: user).execute(merge_request) + execute end it 'calls the merge request activity counter' do @@ -81,13 +84,11 @@ RSpec.describe MergeRequests::CloseService do .to receive(:track_close_mr_action) .with(user: user) - described_class.new(project: project, current_user: user).execute(merge_request) + execute end it 'refreshes the number of open merge requests for a valid MR', :use_clean_rails_memory_store_caching do - service = described_class.new(project: project, current_user: user) - - expect { service.execute(merge_request) } + expect { execute } .to change { project.open_merge_requests_count }.from(1).to(0) end @@ -96,25 +97,39 @@ RSpec.describe MergeRequests::CloseService do expect(service).to receive(:execute_for_merge_request_pipeline).with(merge_request) end - described_class.new(project: project, current_user: user).execute(merge_request) + execute end it 'schedules CleanupRefsService' do expect(MergeRequests::CleanupRefsService).to receive(:schedule).with(merge_request) - described_class.new(project: project, current_user: user).execute(merge_request) + execute + end + + it 'triggers GraphQL subscription mergeRequestMergeStatusUpdated' do + expect(GraphqlTriggers).to receive(:merge_request_merge_status_updated).with(merge_request) + + execute end context 'current user is not authorized to close merge request' do + let(:user) { guest } + before do perform_enqueued_jobs do - @merge_request = described_class.new(project: project, current_user: guest).execute(merge_request) + @merge_request = execute end end it 'does not close the merge request' do expect(@merge_request).to be_open end + + it 'does not trigger GraphQL subscription mergeRequestMergeStatusUpdated' do + expect(GraphqlTriggers).not_to receive(:merge_request_merge_status_updated) + + execute + end end end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 709b5c20c92..1d67574b06d 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -425,16 +425,10 @@ RSpec.describe MergeRequests::UpdateService, :mailer do create(:merge_request, :simple, source_project: project, reviewer_ids: [user2.id]) end - context 'when merge_request_reviewer feature is enabled' do - before do - stub_feature_flags(merge_request_reviewer: true) - end + let(:opts) { { reviewer_ids: [IssuableFinder::Params::NONE] } } - let(:opts) { { reviewer_ids: [IssuableFinder::Params::NONE] } } - - it 'removes reviewers' do - expect(update_merge_request(opts).reviewers).to eq [] - end + it 'removes reviewers' do + expect(update_merge_request(opts).reviewers).to eq [] end end end diff --git a/spec/support/shared_examples/services/merge_request_shared_examples.rb b/spec/support/shared_examples/services/merge_request_shared_examples.rb index b3ba0a1be93..cfd75d3cfcd 100644 --- a/spec/support/shared_examples/services/merge_request_shared_examples.rb +++ b/spec/support/shared_examples/services/merge_request_shared_examples.rb @@ -19,29 +19,13 @@ RSpec.shared_examples 'reviewer_ids filter' do let(:reviewer2) { create(:user) } context 'when the current user can admin the merge_request' do - context 'when merge_request_reviewer feature is enabled' do + context 'with a reviewer who can read the merge_request' do before do - stub_feature_flags(merge_request_reviewer: true) + project.add_developer(reviewer1) end - context 'with a reviewer who can read the merge_request' do - before do - project.add_developer(reviewer1) - end - - it 'contains reviewers who can read the merge_request' do - expect(execute.reviewers).to contain_exactly(reviewer1) - end - end - end - - context 'when merge_request_reviewer feature is disabled' do - before do - stub_feature_flags(merge_request_reviewer: false) - end - - it 'contains no reviewers' do - expect(execute.reviewers).to eq [] + it 'contains reviewers who can read the merge_request' do + expect(execute.reviewers).to contain_exactly(reviewer1) end end end diff --git a/spec/views/events/event/_common.html.haml_spec.rb b/spec/views/events/event/_common.html.haml_spec.rb index 0de84e2fdb8..ad8e5c2ef77 100644 --- a/spec/views/events/event/_common.html.haml_spec.rb +++ b/spec/views/events/event/_common.html.haml_spec.rb @@ -7,33 +7,41 @@ RSpec.describe 'events/event/_common.html.haml' do let_it_be(:issue) { create(:issue, project: project) } let_it_be(:user) { create(:user) } + before do + render partial: 'events/event/common', locals: { event: event.present } + end + context 'when it is a work item event' do - let(:work_item) { create(:work_item, project: project) } + let_it_be(:work_item) { create(:work_item, :task, project: project) } - let(:event) do + let_it_be(:event) do create(:event, :created, project: project, target: work_item, target_type: 'WorkItem', author: user) end it 'renders the correct url' do - render partial: 'events/event/common', locals: { event: event.present } - expect(rendered).to have_link( work_item.reference_link_text, href: "/#{project.full_path}/-/work_items/#{work_item.id}" ) end + + it 'uses issue_type for the target_name' do + expect(rendered).to have_content("#{s_('Event|opened')} task #{work_item.to_reference}") + end end - context 'when it is an isssue event' do - let(:issue) { create(:issue, project: project) } + context 'when it is an issue event' do + let_it_be(:issue) { create(:issue, project: project) } - let(:event) do + let_it_be(:event) do create(:event, :created, project: project, target: issue, author: user) end it 'renders the correct url' do - render partial: 'events/event/common', locals: { event: event.present } - expect(rendered).to have_link(issue.reference_link_text, href: "/#{project.full_path}/-/issues/#{issue.iid}") end + + it 'uses issue_type for the target_name' do + expect(rendered).to have_content("#{s_('Event|opened')} issue #{issue.to_reference}") + end end end |