diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index 9b978483cc6..d153ff21a35 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -238,7 +238,7 @@ export default { : ''; }, statusIcon() { - return this.isClosed ? 'mobile-issue-close' : 'issue-open-m'; + return this.isClosed ? 'issue-close' : 'issue-open-m'; }, statusText() { return IssuableStatusText[this.issuableStatus]; diff --git a/app/assets/javascripts/vue_shared/components/deprecated_modal.vue b/app/assets/javascripts/vue_shared/components/deprecated_modal.vue deleted file mode 100644 index 3f55f43edbb..00000000000 --- a/app/assets/javascripts/vue_shared/components/deprecated_modal.vue +++ /dev/null @@ -1,146 +0,0 @@ - - - diff --git a/app/assets/javascripts/vue_shared/components/recaptcha_eventhub.js b/app/assets/javascripts/vue_shared/components/recaptcha_eventhub.js deleted file mode 100644 index e193883b6e9..00000000000 --- a/app/assets/javascripts/vue_shared/components/recaptcha_eventhub.js +++ /dev/null @@ -1,21 +0,0 @@ -import createEventHub from '~/helpers/event_hub_factory'; - -// see recaptcha_tags in app/views/shared/_recaptcha_form.html.haml -export const callbackName = 'recaptchaDialogCallback'; - -export const eventHub = createEventHub(); - -const throwDuplicateCallbackError = () => { - throw new Error(`${callbackName} is already defined!`); -}; - -if (window[callbackName]) { - throwDuplicateCallbackError(); -} - -const callback = () => eventHub.$emit('submit'); - -Object.defineProperty(window, callbackName, { - get: () => callback, - set: throwDuplicateCallbackError, -}); diff --git a/app/assets/javascripts/vue_shared/components/recaptcha_modal.vue b/app/assets/javascripts/vue_shared/components/recaptcha_modal.vue deleted file mode 100644 index fc1f3675a3d..00000000000 --- a/app/assets/javascripts/vue_shared/components/recaptcha_modal.vue +++ /dev/null @@ -1,90 +0,0 @@ - - - diff --git a/app/assets/javascripts/vue_shared/mixins/recaptcha_modal_implementor.js b/app/assets/javascripts/vue_shared/mixins/recaptcha_modal_implementor.js deleted file mode 100644 index ff1f565e79a..00000000000 --- a/app/assets/javascripts/vue_shared/mixins/recaptcha_modal_implementor.js +++ /dev/null @@ -1,36 +0,0 @@ -import recaptchaModal from '../components/recaptcha_modal.vue'; - -export default { - data() { - return { - showRecaptcha: false, - recaptchaHTML: '', - }; - }, - - components: { - recaptchaModal, - }, - - methods: { - openRecaptcha() { - this.showRecaptcha = true; - }, - - closeRecaptcha() { - this.showRecaptcha = false; - }, - - checkForSpam(data) { - if (!data.recaptcha_html) return data; - - this.recaptchaHTML = data.recaptcha_html; - - const spamError = new Error(data.error_message); - spamError.name = 'SpamError'; - spamError.message = 'SpamError'; - - throw spamError; - }, - }, -}; diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb index eb083950fff..5db24d07e4b 100644 --- a/app/graphql/gitlab_schema.rb +++ b/app/graphql/gitlab_schema.rb @@ -124,7 +124,7 @@ class GitlabSchema < GraphQL::Schema raise Gitlab::Graphql::Errors::ArgumentError, "#{global_id} is not a valid GitLab ID." unless gid - if expected_type && !gid.model_class.ancestors.include?(expected_type) + if expected_type && gid.model_class.ancestors.exclude?(expected_type) vars = { global_id: global_id, expected_type: expected_type } msg = _('%{global_id} is not a valid ID for %{expected_type}.') % vars raise Gitlab::Graphql::Errors::ArgumentError, msg diff --git a/app/graphql/resolvers/base_resolver.rb b/app/graphql/resolvers/base_resolver.rb index 7c408382cbe..48563633d11 100644 --- a/app/graphql/resolvers/base_resolver.rb +++ b/app/graphql/resolvers/base_resolver.rb @@ -39,9 +39,7 @@ module Resolvers as_single << block # Have we been called after defining the single version of this resolver? - if @single.present? - @single.instance_exec(&block) - end + @single.instance_exec(&block) if @single.present? end def self.as_single @@ -90,7 +88,7 @@ module Resolvers def self.last parent = self - @last ||= Class.new(self.single) do + @last ||= Class.new(single) do type parent.singular_type, null: true def select_result(results) diff --git a/app/graphql/resolvers/merge_request_resolver.rb b/app/graphql/resolvers/merge_request_resolver.rb index 1f7a4b48aae..c431d079beb 100644 --- a/app/graphql/resolvers/merge_request_resolver.rb +++ b/app/graphql/resolvers/merge_request_resolver.rb @@ -9,9 +9,9 @@ module Resolvers type ::Types::MergeRequestType, null: true argument :iid, GraphQL::STRING_TYPE, - required: true, - as: :iids, - description: 'IID of the merge request, for example `1`.' + required: true, + as: :iids, + description: 'IID of the merge request, for example `1`.' def no_results_possible?(args) project.nil? diff --git a/app/graphql/resolvers/merge_requests_resolver.rb b/app/graphql/resolvers/merge_requests_resolver.rb index 5994dc449f6..52f7fa64864 100644 --- a/app/graphql/resolvers/merge_requests_resolver.rb +++ b/app/graphql/resolvers/merge_requests_resolver.rb @@ -10,35 +10,41 @@ module Resolvers def self.accept_assignee argument :assignee_username, GraphQL::STRING_TYPE, - required: false, - description: 'Username of the assignee.' + required: false, + description: 'Username of the assignee.' end def self.accept_author argument :author_username, GraphQL::STRING_TYPE, - required: false, - description: 'Username of the author.' + required: false, + description: 'Username of the author.' end def self.accept_reviewer argument :reviewer_username, GraphQL::STRING_TYPE, - required: false, - description: 'Username of the reviewer.' + required: false, + description: 'Username of the reviewer.' end argument :iids, [GraphQL::STRING_TYPE], - required: false, - description: 'Array of IIDs of merge requests, for example `[1, 2]`.' + required: false, + description: 'Array of IIDs of merge requests, for example `[1, 2]`.' argument :source_branches, [GraphQL::STRING_TYPE], required: false, as: :source_branch, - description: 'Array of source branch names. All resolved merge requests will have one of these branches as their source.' + description: <<~DESC + Array of source branch names. + All resolved merge requests will have one of these branches as their source. + DESC argument :target_branches, [GraphQL::STRING_TYPE], required: false, as: :target_branch, - description: 'Array of target branch names. All resolved merge requests will have one of these branches as their target.' + description: <<~DESC + Array of target branch names. + All resolved merge requests will have one of these branches as their target. + DESC argument :state, ::Types::MergeRequestStateEnum, required: false, diff --git a/app/graphql/resolvers/user_merge_requests_resolver_base.rb b/app/graphql/resolvers/user_merge_requests_resolver_base.rb index 0b39d3945f6..221a43f691d 100644 --- a/app/graphql/resolvers/user_merge_requests_resolver_base.rb +++ b/app/graphql/resolvers/user_merge_requests_resolver_base.rb @@ -4,13 +4,21 @@ module Resolvers class UserMergeRequestsResolverBase < MergeRequestsResolver include ResolvesProject - argument :project_path, GraphQL::STRING_TYPE, - required: false, - description: 'The full-path of the project the authored merge requests should be in. Incompatible with projectId.' + argument :project_path, + type: GraphQL::STRING_TYPE, + required: false, + description: <<~DESC + The full-path of the project the authored merge requests should be in. + Incompatible with projectId. + DESC - argument :project_id, ::Types::GlobalIDType[::Project], - required: false, - description: 'The global ID of the project the authored merge requests should be in. Incompatible with projectPath.' + argument :project_id, + type: ::Types::GlobalIDType[::Project], + required: false, + description: <<~DESC + The global ID of the project the authored merge requests should be in. + Incompatible with projectPath. + DESC attr_reader :project alias_method :user, :object @@ -22,8 +30,7 @@ module Resolvers load_project(project_path, project_id) return early_return unless can_read_project? elsif args[:iids].present? - raise ::Gitlab::Graphql::Errors::ArgumentError, - 'iids requires projectPath or projectId' + raise ::Gitlab::Graphql::Errors::ArgumentError, 'iids requires projectPath or projectId' end super(**args) diff --git a/app/graphql/types/base_enum.rb b/app/graphql/types/base_enum.rb index a1adc70d64f..f3c5f58fee8 100644 --- a/app/graphql/types/base_enum.rb +++ b/app/graphql/types/base_enum.rb @@ -21,7 +21,10 @@ module Types graphql_name(enum_mod.name) if use_name description(enum_mod.description) if use_description - enum_mod.definition.each { |key, content| value(key.to_s.upcase, **content) } + enum_mod.definition.each do |key, content| + desc = content.delete(:description) + value(key.to_s.upcase, description: desc, **content) + end end def value(*args, **kwargs, &block) diff --git a/app/views/projects/diffs/_file_header.html.haml b/app/views/projects/diffs/_file_header.html.haml index 4a00e0af9d9..d1792826522 100644 --- a/app/views/projects/diffs/_file_header.html.haml +++ b/app/views/projects/diffs/_file_header.html.haml @@ -23,7 +23,7 @@ %strong.file-title-name.has-tooltip.gl-word-break-all{ data: { title: diff_file.new_path, container: 'body' } } = new_path - else - %strong.file-title-name.has-tooltip.gl-word-break-all{ data: { title: diff_file.file_path, container: 'body' } } + %strong.file-title-name.has-tooltip.gl-word-break-all{ data: { title: diff_file.file_path, container: 'body', qa_selector: 'file_name_content' } } = diff_file.file_path - if diff_file.deleted_file? @@ -37,3 +37,4 @@ - if diff_file.stored_externally? && diff_file.external_storage == :lfs %span.badge.label-lfs.gl-mr-2 LFS + diff --git a/app/views/projects/merge_requests/creations/_new_submit.html.haml b/app/views/projects/merge_requests/creations/_new_submit.html.haml index 79781e4a311..3631ddd5c3e 100644 --- a/app/views/projects/merge_requests/creations/_new_submit.html.haml +++ b/app/views/projects/merge_requests/creations/_new_submit.html.haml @@ -33,7 +33,7 @@ Pipelines %span.badge.badge-pill= @pipelines.size %li.diffs-tab - = link_to url_for(safe_params.merge(action: 'diffs')), data: {target: 'div#diffs', action: 'diffs', toggle: 'tabvue'} do + = link_to url_for(safe_params.merge(action: 'diffs')), data: {target: 'div#diffs', action: 'diffs', toggle: 'tabvue', qa_selector: 'diffs_tab'} do Changes %span.badge.badge-pill= @merge_request.diff_size diff --git a/app/views/shared/form_elements/_description.html.haml b/app/views/shared/form_elements/_description.html.haml index 7f4aed5d1f7..77a304d9565 100644 --- a/app/views/shared/form_elements/_description.html.haml +++ b/app/views/shared/form_elements/_description.html.haml @@ -19,9 +19,10 @@ = render layout: 'shared/md_preview', locals: { url: preview_url, referenced_users: true } do = render 'shared/zen', f: form, attr: :description, - classes: 'note-textarea qa-issuable-form-description rspec-issuable-form-description', + classes: 'note-textarea rspec-issuable-form-description', placeholder: placeholder, - supports_quick_actions: supports_quick_actions + supports_quick_actions: supports_quick_actions, + qa_selector: 'issuable_form_description' = render 'shared/notes/hints', supports_quick_actions: supports_quick_actions .clearfix .error-alert diff --git a/app/views/shared/issue_type/_details_header.html.haml b/app/views/shared/issue_type/_details_header.html.haml index 7e150c544bd..0c5ccee1ef6 100644 --- a/app/views/shared/issue_type/_details_header.html.haml +++ b/app/views/shared/issue_type/_details_header.html.haml @@ -1,7 +1,7 @@ .detail-page-header .detail-page-header-body .issuable-status-box.status-box.status-box-issue-closed{ class: issue_status_visibility(issuable, status_box: :closed) } - = sprite_icon('mobile-issue-close', css_class: 'gl-display-block gl-sm-display-none!') + = sprite_icon('issue-close', css_class: 'gl-display-block gl-sm-display-none!') .gl-display-none.gl-sm-display-block! = issue_closed_text(issuable, current_user) .issuable-status-box.status-box.status-box-open{ class: issue_status_visibility(issuable, status_box: :open) } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ef5e374c265..b3cf6a8d5f9 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -22993,9 +22993,6 @@ msgstr "" msgid "Please solve the captcha" msgstr "" -msgid "Please solve the reCAPTCHA" -msgstr "" - msgid "Please try again" msgstr "" diff --git a/qa/qa/page/component/issue_board/show.rb b/qa/qa/page/component/issue_board/show.rb index d7dfb0757bc..0488c7af9cb 100644 --- a/qa/qa/page/component/issue_board/show.rb +++ b/qa/qa/page/component/issue_board/show.rb @@ -11,6 +11,7 @@ module QA view 'app/assets/javascripts/boards/components/board_form.vue' do element :board_name_field + element :save_changes_button end view 'app/assets/javascripts/boards/components/board_list.vue' do @@ -23,10 +24,6 @@ module QA element :create_new_board_button end - view 'app/assets/javascripts/vue_shared/components/deprecated_modal.vue' do - element :save_changes_button - end - view 'app/assets/javascripts/vue_shared/components/sidebar/labels_select/base.vue' do element :labels_dropdown_content end diff --git a/qa/qa/page/merge_request/new.rb b/qa/qa/page/merge_request/new.rb index 8d0914bac4c..46b7bbeed84 100644 --- a/qa/qa/page/merge_request/new.rb +++ b/qa/qa/page/merge_request/new.rb @@ -8,8 +8,33 @@ module QA element :issuable_create_button, required: true end + view 'app/views/shared/form_elements/_description.html.haml' do + element :issuable_form_description + end + + view 'app/views/projects/merge_requests/show.html.haml' do + element :diffs_tab + end + + view 'app/assets/javascripts/diffs/components/diff_file_header.vue' do + element :file_name_content + end + def create_merge_request - click_element :issuable_create_button, Page::MergeRequest::Show + click_element(:issuable_create_button, Page::MergeRequest::Show) + end + + def has_description?(description) + has_element?(:issuable_form_description, text: description) + end + + def click_diffs_tab + click_element(:diffs_tab) + click_element(:dismiss_popover_button) if has_element?(:dismiss_popover_button, wait: 1) + end + + def has_file?(file_name) + has_element?(:file_name_content, text: file_name) end end end diff --git a/spec/frontend/vue_shared/components/deprecated_modal_spec.js b/spec/frontend/vue_shared/components/deprecated_modal_spec.js deleted file mode 100644 index b9793ce2d80..00000000000 --- a/spec/frontend/vue_shared/components/deprecated_modal_spec.js +++ /dev/null @@ -1,73 +0,0 @@ -import Vue from 'vue'; -import mountComponent from 'helpers/vue_mount_component_helper'; -import DeprecatedModal from '~/vue_shared/components/deprecated_modal.vue'; - -const modalComponent = Vue.extend(DeprecatedModal); - -describe('DeprecatedModal', () => { - let vm; - - afterEach(() => { - vm.$destroy(); - }); - - describe('props', () => { - describe('without primaryButtonLabel', () => { - beforeEach(() => { - vm = mountComponent(modalComponent, { - primaryButtonLabel: null, - }); - }); - - it('does not render a primary button', () => { - expect(vm.$el.querySelector('.js-primary-button')).toBeNull(); - }); - }); - - describe('with id', () => { - describe('does not render a primary button', () => { - beforeEach(() => { - vm = mountComponent(modalComponent, { - id: 'my-modal', - }); - }); - - it('assigns the id to the modal', () => { - expect(vm.$el.querySelector('#my-modal.modal')).not.toBeNull(); - }); - - it('does not show the modal immediately', () => { - expect(vm.$el.querySelector('#my-modal.modal')).not.toHaveClass('show'); - }); - - it('does not show a backdrop', () => { - expect(vm.$el.querySelector('modal-backdrop')).toBeNull(); - }); - }); - }); - - it('works with data-toggle="modal"', () => { - setFixtures(` - - - `); - - const modalContainer = document.getElementById('modal-container'); - const modalButton = document.getElementById('modal-button'); - vm = mountComponent( - modalComponent, - { - id: 'my-modal', - }, - modalContainer, - ); - const modalElement = vm.$el.querySelector('#my-modal'); - - expect(modalElement).not.toHaveClass('show'); - - modalButton.click(); - - expect(modalElement).toHaveClass('show'); - }); - }); -}); diff --git a/spec/frontend/vue_shared/components/recaptcha_eventhub_spec.js b/spec/frontend/vue_shared/components/recaptcha_eventhub_spec.js deleted file mode 100644 index d86d627886f..00000000000 --- a/spec/frontend/vue_shared/components/recaptcha_eventhub_spec.js +++ /dev/null @@ -1,21 +0,0 @@ -import { eventHub, callbackName } from '~/vue_shared/components/recaptcha_eventhub'; - -describe('reCAPTCHA event hub', () => { - // the following test case currently crashes - // see https://gitlab.com/gitlab-org/gitlab/issues/29192#note_217840035 - // eslint-disable-next-line jest/no-disabled-tests - it.skip('throws an error for overriding the callback', () => { - expect(() => { - window[callbackName] = 'something'; - }).toThrow(); - }); - - it('triggering callback emits a submit event', () => { - const eventHandler = jest.fn(); - eventHub.$once('submit', eventHandler); - - window[callbackName](); - - expect(eventHandler).toHaveBeenCalled(); - }); -}); diff --git a/spec/frontend/vue_shared/components/recaptcha_modal_spec.js b/spec/frontend/vue_shared/components/recaptcha_modal_spec.js deleted file mode 100644 index 8ab65efd388..00000000000 --- a/spec/frontend/vue_shared/components/recaptcha_modal_spec.js +++ /dev/null @@ -1,35 +0,0 @@ -import { shallowMount } from '@vue/test-utils'; - -import { eventHub } from '~/vue_shared/components/recaptcha_eventhub'; - -import RecaptchaModal from '~/vue_shared/components/recaptcha_modal.vue'; - -describe('RecaptchaModal', () => { - const recaptchaFormId = 'recaptcha-form'; - const recaptchaHtml = `
`; - - let wrapper; - - const findRecaptchaForm = () => wrapper.find(`#${recaptchaFormId}`).element; - - beforeEach(() => { - wrapper = shallowMount(RecaptchaModal, { - propsData: { - html: recaptchaHtml, - }, - }); - }); - - afterEach(() => { - wrapper.destroy(); - }); - - it('submits the form if event hub emits submit event', () => { - const form = findRecaptchaForm(); - jest.spyOn(form, 'submit').mockImplementation(); - - eventHub.$emit('submit'); - - expect(form.submit).toHaveBeenCalled(); - }); -}); diff --git a/spec/graphql/features/authorization_spec.rb b/spec/graphql/features/authorization_spec.rb index 7ab5adadd1f..d2a6b91d1c2 100644 --- a/spec/graphql/features/authorization_spec.rb +++ b/spec/graphql/features/authorization_spec.rb @@ -84,9 +84,9 @@ RSpec.describe 'DeclarativePolicy authorization in GraphQL ' do permissions = permission_collection query_factory do |qt| qt.field :item, type, - null: true, - resolver: new_resolver(test_object), - authorize: permissions + null: true, + resolver: new_resolver(test_object), + authorize: permissions end end @@ -123,8 +123,9 @@ RSpec.describe 'DeclarativePolicy authorization in GraphQL ' do let(:type) do permissions = permission_collection type_factory do |type| - type.field :name, GraphQL::STRING_TYPE, null: true, - authorize: permissions + type.field :name, GraphQL::STRING_TYPE, + null: true, + authorize: permissions end end @@ -201,9 +202,10 @@ RSpec.describe 'DeclarativePolicy authorization in GraphQL ' do let(:query_type) do query_factory do |query| - query.field :item, type, null: true, - resolver: resolver, - authorize: permission_2 + query.field :item, type, + null: true, + resolver: resolver, + authorize: permission_2 end end @@ -288,8 +290,12 @@ RSpec.describe 'DeclarativePolicy authorization in GraphQL ' do let(:query_string) { '{ item(first: 1) { edges { node { name } } } }' } it 'only checks permissions for the first object' do - expect(Ability).to receive(:allowed?).with(user, permission_single, test_object) { true } - expect(Ability).not_to receive(:allowed?).with(user, permission_single, second_test_object) + expect(Ability) + .to receive(:allowed?) + .with(user, permission_single, test_object) + .and_return(true) + expect(Ability) + .not_to receive(:allowed?).with(user, permission_single, second_test_object) expect(subject.size).to eq(1) end @@ -330,10 +336,12 @@ RSpec.describe 'DeclarativePolicy authorization in GraphQL ' do end let(:project_type) do |type| + issues = Issue.where(project: [visible_project, other_project]).order(id: :asc) type_factory do |type| type.graphql_name 'FakeProjectType' - type.field :test_issues, issue_type.connection_type, null: false, - resolver: new_resolver(Issue.where(project: [visible_project, other_project]).order(id: :asc)) + type.field :test_issues, issue_type.connection_type, + null: false, + resolver: new_resolver(issues) end end diff --git a/spec/graphql/mutations/design_management/upload_spec.rb b/spec/graphql/mutations/design_management/upload_spec.rb index e92000194b1..ada88b7652c 100644 --- a/spec/graphql/mutations/design_management/upload_spec.rb +++ b/spec/graphql/mutations/design_management/upload_spec.rb @@ -56,10 +56,10 @@ RSpec.describe Mutations::DesignManagement::Upload do .map { |f| RenameableUpload.unique_file(f) } end - def creates_designs + def creates_designs(&block) prior_count = DesignManagement::Design.count - expect { yield }.not_to raise_error + expect(&block).not_to raise_error expect(DesignManagement::Design.count).to eq(prior_count + files.size) end diff --git a/spec/graphql/resolvers/merge_requests_resolver_spec.rb b/spec/graphql/resolvers/merge_requests_resolver_spec.rb index 5e2a075c5b3..09f5181107d 100644 --- a/spec/graphql/resolvers/merge_requests_resolver_spec.rb +++ b/spec/graphql/resolvers/merge_requests_resolver_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do include SortingHelper let_it_be(:project) { create(:project, :repository) } + let_it_be(:other_project) { create(:project, :repository) } let_it_be(:milestone) { create(:milestone, project: project) } let_it_be(:current_user) { create(:user) } let_it_be(:other_user) { create(:user) } @@ -16,10 +17,17 @@ RSpec.describe Resolvers::MergeRequestsResolver do let_it_be(:merge_request_3) { create(:merge_request, :unique_branches, **common_attrs) } let_it_be(:merge_request_4) { create(:merge_request, :unique_branches, :locked, **common_attrs) } let_it_be(:merge_request_5) { create(:merge_request, :simple, :locked, **common_attrs) } - let_it_be(:merge_request_6) { create(:labeled_merge_request, :unique_branches, labels: create_list(:label, 2, project: project), **common_attrs) } - let_it_be(:merge_request_with_milestone) { create(:merge_request, :unique_branches, **common_attrs, milestone: milestone) } - let_it_be(:other_project) { create(:project, :repository) } - let_it_be(:other_merge_request) { create(:merge_request, source_project: other_project, target_project: other_project) } + let_it_be(:merge_request_6) do + create(:labeled_merge_request, :unique_branches, **common_attrs, labels: create_list(:label, 2, project: project)) + end + + let_it_be(:merge_request_with_milestone) do + create(:merge_request, :unique_branches, **common_attrs, milestone: milestone) + end + + let_it_be(:other_merge_request) do + create(:merge_request, source_project: other_project, target_project: other_project) + end let(:iid_1) { merge_request_1.iid } let(:iid_2) { merge_request_2.iid } @@ -43,11 +51,14 @@ RSpec.describe Resolvers::MergeRequestsResolver do # SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = 2 let(:queries_per_project) { 4 } - context 'no arguments' do + context 'without arguments' do it 'returns all merge requests' do result = resolve_mr(project) - expect(result).to contain_exactly(merge_request_1, merge_request_2, merge_request_3, merge_request_4, merge_request_5, merge_request_6, merge_request_with_milestone) + expect(result).to contain_exactly( + merge_request_1, merge_request_2, merge_request_3, merge_request_4, merge_request_5, + merge_request_6, merge_request_with_milestone + ) end it 'returns only merge requests that the current user can see' do @@ -57,7 +68,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do end end - context 'by iid alone' do + context 'with iid alone' do it 'batch-resolves by target project full path and individual IID', :request_store do # 1 query for project_authorizations, and 1 for merge_requests result = batch_sync(max_queries: queries_per_project) do @@ -83,7 +94,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do expect(result).to contain_exactly(merge_request_1, merge_request_2, merge_request_3) end - it 'can batch-resolve merge requests from different projects', :request_store, :use_clean_rails_memory_store_caching do + it 'can batch-resolve merge requests from different projects', :request_store do # 2 queries for project_authorizations, and 2 for merge_requests results = batch_sync(max_queries: queries_per_project * 2) do a = resolve_mr(project, iids: [iid_1]) @@ -121,7 +132,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do end end - context 'by source branches' do + context 'with source branches argument' do it 'takes one argument' do result = resolve_mr(project, source_branches: [merge_request_3.source_branch]) @@ -131,13 +142,13 @@ RSpec.describe Resolvers::MergeRequestsResolver do it 'takes more than one argument' do mrs = [merge_request_3, merge_request_4] branches = mrs.map(&:source_branch) - result = resolve_mr(project, source_branches: branches ) + result = resolve_mr(project, source_branches: branches) expect(result).to match_array(mrs) end end - context 'by target branches' do + context 'with target branches argument' do it 'takes one argument' do result = resolve_mr(project, target_branches: [merge_request_3.target_branch]) @@ -153,7 +164,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do end end - context 'by state' do + context 'with state argument' do it 'takes one argument' do result = resolve_mr(project, state: 'locked') @@ -161,7 +172,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do end end - context 'by label' do + context 'with label argument' do let_it_be(:label) { merge_request_6.labels.first } let_it_be(:with_label) { create(:labeled_merge_request, :closed, labels: [label], **common_attrs) } @@ -178,7 +189,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do end end - context 'by merged_after and merged_before' do + context 'with merged_after and merged_before arguments' do before do merge_request_1.metrics.update!(merged_at: 10.days.ago) end @@ -196,7 +207,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do end end - context 'by milestone' do + context 'with milestone argument' do it 'filters merge requests by milestone title' do result = resolve_mr(project, milestone_title: milestone.title) @@ -212,7 +223,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do describe 'combinations' do it 'requires all filters' do - create(:merge_request, :closed, source_project: project, target_project: project, source_branch: merge_request_4.source_branch) + create(:merge_request, :closed, **common_attrs, source_branch: merge_request_4.source_branch) result = resolve_mr(project, source_branches: [merge_request_4.source_branch], state: 'locked') diff --git a/spec/graphql/types/alert_management/prometheus_integration_type_spec.rb b/spec/graphql/types/alert_management/prometheus_integration_type_spec.rb index 7e4110af9a4..d057afb331c 100644 --- a/spec/graphql/types/alert_management/prometheus_integration_type_spec.rb +++ b/spec/graphql/types/alert_management/prometheus_integration_type_spec.rb @@ -48,14 +48,14 @@ RSpec.describe GitlabSchema.types['AlertManagementPrometheusIntegration'] do end end - context 'group integration' do + describe 'a group integration' do let_it_be(:group) { create(:group) } let_it_be(:integration) { create(:prometheus_service, project: nil, group: group) } # Since it is impossible to authorize the parent here, given that the # project is nil, all fields should be redacted: - described_class.fields.keys.each do |field_name| + described_class.fields.each_key do |field_name| context "field: #{field_name}" do it 'is redacted' do expect do diff --git a/spec/graphql/types/base_object_spec.rb b/spec/graphql/types/base_object_spec.rb index d144c1f6456..d8f2ef58ea5 100644 --- a/spec/graphql/types/base_object_spec.rb +++ b/spec/graphql/types/base_object_spec.rb @@ -196,20 +196,20 @@ RSpec.describe Types::BaseObject do end # For example a batchloaded association - context 'a lazy list' do + describe 'a lazy list' do it_behaves_like 'array member redaction', %w[lazyListOfYs] end # For example using a batchloader to map over a set of IDs - context 'a list of lazy items' do + describe 'a list of lazy items' do it_behaves_like 'array member redaction', %w[listOfLazyYs] end - context 'an array connection of items' do + describe 'an array connection of items' do it_behaves_like 'array member redaction', %w[arrayYsConn nodes] end - context 'an array connection of items, selecting edges' do + describe 'an array connection of items, selecting edges' do it_behaves_like 'array member redaction', %w[arrayYsConn edges node] end @@ -222,7 +222,7 @@ RSpec.describe Types::BaseObject do } } - doc = ->(after) do + doc = lambda do |after| GraphQL.parse(<<~GQL) query { x { @@ -238,9 +238,7 @@ RSpec.describe Types::BaseObject do } GQL end - returned_items = ->(ids) do - ids.to_a.map { |id| eq({ 'id' => id }) } - end + returned_items = ->(ids) { ids.to_a.map { |id| eq({ 'id' => id }) } } query = GraphQL::Query.new(test_schema, document: doc[nil], context: data) result = query.result.to_h diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index 9579ef8b99b..8499df6234d 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -106,7 +106,8 @@ RSpec.describe GitlabSchema.types['Project'] do expect(secure_analyzers_prefix['type']).to eq('string') expect(secure_analyzers_prefix['field']).to eq('SECURE_ANALYZERS_PREFIX') expect(secure_analyzers_prefix['label']).to eq('Image prefix') - expect(secure_analyzers_prefix['defaultValue']).to eq('registry.gitlab.com/gitlab-org/security-products/analyzers') + expect(secure_analyzers_prefix['defaultValue']) + .to eq('registry.gitlab.com/gitlab-org/security-products/analyzers') expect(secure_analyzers_prefix['value']).to eq('registry.gitlab.com/gitlab-org/security-products/analyzers') expect(secure_analyzers_prefix['size']).to eq('LARGE') expect(secure_analyzers_prefix['options']).to be_nil @@ -184,9 +185,11 @@ RSpec.describe GitlabSchema.types['Project'] do context 'when repository is accessible only by team members' do it "returns no configuration" do - project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED, - builds_access_level: ProjectFeature::DISABLED, - repository_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!( + merge_requests_access_level: ProjectFeature::DISABLED, + builds_access_level: ProjectFeature::DISABLED, + repository_access_level: ProjectFeature::PRIVATE + ) secure_analyzers_prefix = subject.dig('data', 'project', 'sastCiConfiguration') expect(secure_analyzers_prefix).to be_nil @@ -342,8 +345,13 @@ RSpec.describe GitlabSchema.types['Project'] do let_it_be(:project) { create(:project, :public) } context 'when project has Jira imports' do - let_it_be(:jira_import1) { create(:jira_import_state, :finished, project: project, jira_project_key: 'AA', created_at: 2.days.ago) } - let_it_be(:jira_import2) { create(:jira_import_state, :finished, project: project, jira_project_key: 'BB', created_at: 5.days.ago) } + let_it_be(:jira_import1) do + create(:jira_import_state, :finished, project: project, jira_project_key: 'AA', created_at: 2.days.ago) + end + + let_it_be(:jira_import2) do + create(:jira_import_state, :finished, project: project, jira_project_key: 'BB', created_at: 5.days.ago) + end it 'retrieves the imports' do expect(subject).to contain_exactly(jira_import1, jira_import2) diff --git a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb index fb48d820057..0c548e1ce32 100644 --- a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb +++ b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb @@ -12,7 +12,8 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do authorize :read_the_thing def initialize(user, found_object) - @user, @found_object = user, found_object + @user = user + @found_object = found_object end def find_object @@ -40,16 +41,12 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do before do # don't allow anything by default - allow(Ability).to receive(:allowed?) do - false - end + allow(Ability).to receive(:allowed?).and_return(false) end context 'when the user is allowed to perform the action' do before do - allow(Ability).to receive(:allowed?).with(user, :read_the_thing, project) do - true - end + allow(Ability).to receive(:allowed?).with(user, :read_the_thing, project).and_return(true) end describe '#authorized_find!' do diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb index ca21e56dca3..b1158d937c5 100644 --- a/spec/requests/api/graphql_spec.rb +++ b/spec/requests/api/graphql_spec.rb @@ -3,10 +3,11 @@ require 'spec_helper' RSpec.describe 'GraphQL' do include GraphqlHelpers + include AfterNextHelpers - let(:query) { graphql_query_for('echo', text: 'Hello world' ) } + let(:query) { graphql_query_for('echo', text: 'Hello world') } - context 'logging' do + describe 'logging' do shared_examples 'logging a graphql query' do let(:expected_params) do { @@ -51,19 +52,25 @@ RSpec.describe 'GraphQL' do context 'when there is an error in the logger' do before do - allow_any_instance_of(Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer).to receive(:process_variables).and_raise(StandardError.new("oh noes!")) + logger_analyzer = GitlabSchema.query_analyzers.find do |qa| + qa.is_a? Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer + end + allow(logger_analyzer).to receive(:process_variables) + .and_raise(StandardError.new("oh noes!")) end it 'logs the exception in Sentry and continues with the request' do - expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(:once) - expect(Gitlab::GraphqlLogger).to receive(:info) + expect(Gitlab::ErrorTracking) + .to receive(:track_and_raise_for_dev_exception).at_least(:once) + expect(Gitlab::GraphqlLogger) + .to receive(:info) post_graphql(query, variables: {}) end end end - context 'invalid variables' do + context 'with invalid variables' do it 'returns an error' do post_graphql(query, variables: "This is not JSON") @@ -72,7 +79,7 @@ RSpec.describe 'GraphQL' do end end - context 'authentication', :allow_forgery_protection do + describe 'authentication', :allow_forgery_protection do let(:user) { create(:user) } it 'allows access to public data without authentication' do @@ -99,7 +106,7 @@ RSpec.describe 'GraphQL' do expect(graphql_data['echo']).to eq("\"#{user.username}\" says: Hello world") end - context 'token authentication' do + context 'with token authentication' do let(:token) { create(:personal_access_token) } before do @@ -119,7 +126,7 @@ RSpec.describe 'GraphQL' do context 'when the personal access token has no api scope' do it 'does not log the user in' do - token.update(scopes: [:read_user]) + token.update!(scopes: [:read_user]) post_graphql(query, headers: { 'PRIVATE-TOKEN' => token.token }) @@ -136,7 +143,11 @@ RSpec.describe 'GraphQL' do let(:user) { create(:user) } let(:query) do - graphql_query_for('project', { 'fullPath' => project.full_path }, %w(id)) + graphql_query_for( + :project, + { full_path: project.full_path }, + 'id' + ) end before do @@ -202,8 +213,8 @@ RSpec.describe 'GraphQL' do let_it_be(:issues) { create_list(:issue, 10, project: project, created_at: Time.now.change(usec: 200)) } let(:page_size) { 6 } - let(:issues_edges) { %w(data project issues edges) } - let(:end_cursor) { %w(data project issues pageInfo endCursor) } + let(:issues_edges) { %w[project issues edges] } + let(:end_cursor) { %w[project issues pageInfo endCursor] } let(:query) do <<~GRAPHQL query project($fullPath: ID!, $first: Int, $after: String) { @@ -217,16 +228,10 @@ RSpec.describe 'GraphQL' do GRAPHQL end - # TODO: Switch this to use `post_graphql` - # This is not performing an actual GraphQL request because the - # variables end up being strings when passed through the `post_graphql` - # helper. - # - # https://gitlab.com/gitlab-org/gitlab/-/issues/222432 def execute_query(after: nil) - GitlabSchema.execute( + post_graphql( query, - context: { current_user: nil }, + current_user: nil, variables: { fullPath: project.full_path, first: page_size, @@ -240,14 +245,16 @@ RSpec.describe 'GraphQL' do expect(Gitlab::Graphql::Pagination::Keyset::QueryBuilder) .to receive(:new).with(anything, anything, hash_including('created_at'), anything).and_call_original - first_page = execute_query + execute_query + first_page = graphql_data edges = first_page.dig(*issues_edges) cursor = first_page.dig(*end_cursor) expect(edges.count).to eq(6) expect(edges.last['node']['iid']).to eq(issues[4].iid.to_s) - second_page = execute_query(after: cursor) + execute_query(after: cursor) + second_page = graphql_data edges = second_page.dig(*issues_edges) expect(edges.count).to eq(4)