diff options
24 files changed, 368 insertions, 81 deletions
diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js index bac124be34c..63658d49a05 100644 --- a/app/assets/javascripts/notes/stores/actions.js +++ b/app/assets/javascripts/notes/stores/actions.js @@ -268,11 +268,20 @@ export const saveNote = ({ commit, dispatch }, noteData) => { const { errors } = res; const commandsChanges = res.commands_changes; - if (hasQuickActions && errors && Object.keys(errors).length) { - eTagPoll.makeRequest(); + if (errors && Object.keys(errors).length) { + /* + The following reply means that quick actions have been successfully applied: - $('.js-gfm-input').trigger('clear-commands-cache.atwho'); - Flash(__('Commands applied'), 'notice', noteData.flashContainer); + {"commands_changes":{},"valid":false,"errors":{"commands_only":["Commands applied"]}} + */ + if (hasQuickActions) { + eTagPoll.makeRequest(); + + $('.js-gfm-input').trigger('clear-commands-cache.atwho'); + Flash(__('Commands applied'), 'notice', noteData.flashContainer); + } else { + throw new Error(__('Failed to save comment!')); + } } if (commandsChanges) { diff --git a/app/assets/javascripts/notes/stores/utils.js b/app/assets/javascripts/notes/stores/utils.js index 029fde348fb..ed4cef4a917 100644 --- a/app/assets/javascripts/notes/stores/utils.js +++ b/app/assets/javascripts/notes/stores/utils.js @@ -2,7 +2,8 @@ import AjaxCache from '~/lib/utils/ajax_cache'; import { trimFirstCharOfLineContent } from '~/diffs/store/utils'; import { sprintf, __ } from '~/locale'; -const REGEX_QUICK_ACTIONS = /^\/\w+.*$/gm; +// factory function because global flag makes RegExp stateful +const createQuickActionsRegex = () => /^\/\w+.*$/gm; export const findNoteObjectById = (notes, id) => notes.filter(n => n.id === id)[0]; @@ -27,9 +28,9 @@ export const getQuickActionText = note => { return text; }; -export const hasQuickActions = note => REGEX_QUICK_ACTIONS.test(note); +export const hasQuickActions = note => createQuickActionsRegex().test(note); -export const stripQuickActions = note => note.replace(REGEX_QUICK_ACTIONS, '').trim(); +export const stripQuickActions = note => note.replace(createQuickActionsRegex(), '').trim(); export const prepareDiffLines = diffLines => diffLines.map(line => ({ ...trimFirstCharOfLineContent(line) })); diff --git a/app/assets/stylesheets/framework/ci_variable_list.scss b/app/assets/stylesheets/framework/ci_variable_list.scss index d9b0e4558ad..28d7492b99c 100644 --- a/app/assets/stylesheets/framework/ci_variable_list.scss +++ b/app/assets/stylesheets/framework/ci_variable_list.scss @@ -47,6 +47,7 @@ display: flex; align-items: flex-start; width: 100%; + padding-bottom: $gl-padding; @include media-breakpoint-down(xs) { display: block; diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb index a12568d5d31..897e12c1b56 100644 --- a/app/graphql/gitlab_schema.rb +++ b/app/graphql/gitlab_schema.rb @@ -7,6 +7,9 @@ class GitlabSchema < GraphQL::Schema AUTHENTICATED_COMPLEXITY = 250 ADMIN_COMPLEXITY = 300 + ANONYMOUS_MAX_DEPTH = 10 + AUTHENTICATED_MAX_DEPTH = 15 + use BatchLoader::GraphQL use Gitlab::Graphql::Authorize use Gitlab::Graphql::Present @@ -23,21 +26,36 @@ class GitlabSchema < GraphQL::Schema mutation(Types::MutationType) - def self.execute(query_str = nil, **kwargs) - kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context]) + class << self + def execute(query_str = nil, **kwargs) + kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context]) + kwargs[:max_depth] ||= max_query_depth(kwargs[:context]) - super(query_str, **kwargs) - end + super(query_str, **kwargs) + end + + private + + def max_query_complexity(ctx) + current_user = ctx&.fetch(:current_user, nil) + + if current_user&.admin + ADMIN_COMPLEXITY + elsif current_user + AUTHENTICATED_COMPLEXITY + else + DEFAULT_MAX_COMPLEXITY + end + end - def self.max_query_complexity(ctx) - current_user = ctx&.fetch(:current_user, nil) + def max_query_depth(ctx) + current_user = ctx&.fetch(:current_user, nil) - if current_user&.admin - ADMIN_COMPLEXITY - elsif current_user - AUTHENTICATED_COMPLEXITY - else - DEFAULT_MAX_COMPLEXITY + if current_user + AUTHENTICATED_MAX_DEPTH + else + ANONYMOUS_MAX_DEPTH + end end end end diff --git a/app/views/ci/variables/_index.html.haml b/app/views/ci/variables/_index.html.haml index dc9ccb6cc39..464b9faf282 100644 --- a/app/views/ci/variables/_index.html.haml +++ b/app/views/ci/variables/_index.html.haml @@ -10,6 +10,7 @@ .hide.alert.alert-danger.js-ci-variable-error-box %ul.ci-variable-list + = render 'ci/variables/variable_header' - @variables.each.each do |variable| = render 'ci/variables/variable_row', form_field: 'variables', variable: variable = render 'ci/variables/variable_row', form_field: 'variables' diff --git a/app/views/ci/variables/_variable_header.html.haml b/app/views/ci/variables/_variable_header.html.haml new file mode 100644 index 00000000000..d3b7a5ae883 --- /dev/null +++ b/app/views/ci/variables/_variable_header.html.haml @@ -0,0 +1,16 @@ +- only_key_value = local_assigns.fetch(:only_key_value, false) + +%li.ci-variable-row.m-0.d-none.d-sm-block + .d-flex.w-100.align-items-center.pb-2 + .bold.table-section.section-15.append-right-10 + = s_('CiVariables|Type') + .bold.table-section.section-15.append-right-10 + = s_('CiVariables|Key') + .bold.table-section.section-15.append-right-10 + = s_('CiVariables|Value') + - unless only_key_value + .bold.table-section.section-20 + = s_('CiVariables|State') + .bold.table-section.section-20 + = s_('CiVariables|Masked') + = render_if_exists 'ci/variables/environment_scope_header' diff --git a/app/views/ci/variables/_variable_row.html.haml b/app/views/ci/variables/_variable_row.html.haml index 37257b3aa1c..b4930b41c09 100644 --- a/app/views/ci/variables/_variable_row.html.haml +++ b/app/views/ci/variables/_variable_row.html.haml @@ -20,18 +20,18 @@ - masked_input_name = "#{form_field}[variables_attributes][][masked]" %li.js-row.ci-variable-row{ data: { is_persisted: "#{!id.nil?}" } } - .ci-variable-row-body + .ci-variable-row-body.border-bottom %input.js-ci-variable-input-id{ type: "hidden", name: id_input_name, value: id } %input.js-ci-variable-input-destroy{ type: "hidden", name: destroy_input_name } - %select.js-ci-variable-input-variable-type.ci-variable-body-item.form-control.select-control{ name: variable_type_input_name } + %select.js-ci-variable-input-variable-type.ci-variable-body-item.form-control.select-control.table-section.section-15{ name: variable_type_input_name } = options_for_select(ci_variable_type_options, variable_type) - %input.js-ci-variable-input-key.ci-variable-body-item.qa-ci-variable-input-key.form-control{ type: "text", + %input.js-ci-variable-input-key.ci-variable-body-item.qa-ci-variable-input-key.form-control.table-section.section-15{ type: "text", name: key_input_name, value: key, placeholder: s_('CiVariables|Input variable key') } - .ci-variable-body-item.gl-show-field-errors + .ci-variable-body-item.gl-show-field-errors.table-section.section-15.border-top-0.p-0 .form-control.js-secret-value-placeholder.qa-ci-variable-input-value{ class: ('hide' unless id) } - = '*' * 20 + = '*' * 17 %textarea.js-ci-variable-input-value.js-secret-value.qa-ci-variable-input-value.form-control{ class: ('hide' if id), rows: 1, name: value_input_name, @@ -41,7 +41,7 @@ = s_("CiVariables|Cannot use Masked Variable with current value") = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'masked-variables'), target: '_blank', rel: 'noopener noreferrer' - unless only_key_value - .ci-variable-body-item.ci-variable-protected-item + .ci-variable-body-item.ci-variable-protected-item.table-section.section-20.mr-0.border-top-0 .append-right-default = s_("CiVariable|Protected") %button{ type: 'button', @@ -55,7 +55,7 @@ %span.toggle-icon = sprite_icon('status_success_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-checked') = sprite_icon('status_failed_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-unchecked') - .ci-variable-body-item.ci-variable-masked-item + .ci-variable-body-item.ci-variable-masked-item.table-section.section-20.mr-0.border-top-0 .append-right-default = s_("CiVariable|Masked") %button{ type: 'button', @@ -70,5 +70,5 @@ = sprite_icon('status_success_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-checked') = sprite_icon('status_failed_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-unchecked') = render_if_exists 'ci/variables/environment_scope', form_field: form_field, variable: variable - %button.js-row-remove-button.ci-variable-row-remove-button{ type: 'button', 'aria-label': s_('CiVariables|Remove variable row') } - = icon('minus-circle') + %button.js-row-remove-button.ci-variable-row-remove-button.table-section.section-5.border-top-0{ type: 'button', 'aria-label': s_('CiVariables|Remove variable row') } + = icon('minus-circle') diff --git a/app/views/repository_check_mailer/notify.text.haml b/app/views/repository_check_mailer/notify.text.haml index 6b64b337b0e..a2e04fa710f 100644 --- a/app/views/repository_check_mailer/notify.text.haml +++ b/app/views/repository_check_mailer/notify.text.haml @@ -3,3 +3,5 @@ = _("View details: %{details_url}") % { details_url: admin_projects_url(last_repository_check_failed: 1) } = _("You are receiving this message because you are a GitLab administrator for %{url}.") % { url: Gitlab.config.gitlab.url } + += render_if_exists 'repository_check_mailer/email_additional_text' diff --git a/changelogs/unreleased/27987-use-findorcreateservice-to-create-labels.yml b/changelogs/unreleased/27987-use-findorcreateservice-to-create-labels.yml new file mode 100644 index 00000000000..8d3501e0171 --- /dev/null +++ b/changelogs/unreleased/27987-use-findorcreateservice-to-create-labels.yml @@ -0,0 +1,5 @@ +--- +title: Use FindOrCreateService to create labels and check for existing ones +merge_request: 27987 +author: Matt Duren +type: fixed diff --git a/changelogs/unreleased/58404-set-default-max-depth-for-GraphQL.yml b/changelogs/unreleased/58404-set-default-max-depth-for-GraphQL.yml new file mode 100644 index 00000000000..7e95158a0e0 --- /dev/null +++ b/changelogs/unreleased/58404-set-default-max-depth-for-GraphQL.yml @@ -0,0 +1,5 @@ +--- +title: 58404 - setup max depth for GraphQL +merge_request: 25737 +author: Ken Ding +type: added diff --git a/changelogs/unreleased/winh-notes-error-handling.yml b/changelogs/unreleased/winh-notes-error-handling.yml new file mode 100644 index 00000000000..6f23dd459d4 --- /dev/null +++ b/changelogs/unreleased/winh-notes-error-handling.yml @@ -0,0 +1,5 @@ +--- +title: Handle errors in successful notes reply +merge_request: 28082 +author: +type: fixed diff --git a/db/migrate/20190412155659_add_merge_request_blocks.rb b/db/migrate/20190412155659_add_merge_request_blocks.rb new file mode 100644 index 00000000000..9e7f370d1cf --- /dev/null +++ b/db/migrate/20190412155659_add_merge_request_blocks.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class AddMergeRequestBlocks < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :merge_request_blocks, id: :bigserial do |t| + t.references :blocking_merge_request, + index: false, null: false, + foreign_key: { to_table: :merge_requests, on_delete: :cascade } + + t.references :blocked_merge_request, + index: true, null: false, + foreign_key: { to_table: :merge_requests, on_delete: :cascade } + + t.index [:blocking_merge_request_id, :blocked_merge_request_id], + unique: true, + name: 'index_mr_blocks_on_blocking_and_blocked_mr_ids' + + t.timestamps_with_timezone + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 1be3afd5282..159e7e03cf4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1227,6 +1227,15 @@ ActiveRecord::Schema.define(version: 20190506135400) do t.index ["user_id"], name: "index_merge_request_assignees_on_user_id", using: :btree end + create_table "merge_request_blocks", force: :cascade do |t| + t.integer "blocking_merge_request_id", null: false + t.integer "blocked_merge_request_id", null: false + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false + t.index ["blocked_merge_request_id"], name: "index_merge_request_blocks_on_blocked_merge_request_id", using: :btree + t.index ["blocking_merge_request_id", "blocked_merge_request_id"], name: "index_mr_blocks_on_blocking_and_blocked_mr_ids", unique: true, using: :btree + end + create_table "merge_request_diff_commits", id: false, force: :cascade do |t| t.datetime_with_timezone "authored_date" t.datetime_with_timezone "committed_date" @@ -2514,6 +2523,8 @@ ActiveRecord::Schema.define(version: 20190506135400) do add_foreign_key "members", "users", name: "fk_2e88fb7ce9", on_delete: :cascade add_foreign_key "merge_request_assignees", "merge_requests", on_delete: :cascade add_foreign_key "merge_request_assignees", "users", on_delete: :cascade + add_foreign_key "merge_request_blocks", "merge_requests", column: "blocked_merge_request_id", on_delete: :cascade + add_foreign_key "merge_request_blocks", "merge_requests", column: "blocking_merge_request_id", on_delete: :cascade add_foreign_key "merge_request_diff_commits", "merge_request_diffs", on_delete: :cascade add_foreign_key "merge_request_diff_files", "merge_request_diffs", on_delete: :cascade add_foreign_key "merge_request_diffs", "merge_requests", name: "fk_8483f3258f", on_delete: :cascade diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index 769d3279f91..c9f0ed66a54 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -135,7 +135,7 @@ module Gitlab def create_labels LABELS.each do |label_params| - label = ::Labels::CreateService.new(label_params).execute(project: project) + label = ::Labels::FindOrCreateService.new(nil, project, label_params).execute(skip_authorization: true) if label.valid? @labels[label_params[:title]] = label else diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b9fc3e00cff..6a3184175f4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1894,9 +1894,24 @@ msgstr "" msgid "CiVariables|Input variable value" msgstr "" +msgid "CiVariables|Key" +msgstr "" + +msgid "CiVariables|Masked" +msgstr "" + msgid "CiVariables|Remove variable row" msgstr "" +msgid "CiVariables|State" +msgstr "" + +msgid "CiVariables|Type" +msgstr "" + +msgid "CiVariables|Value" +msgstr "" + msgid "CiVariable|* (All environments)" msgstr "" @@ -4196,6 +4211,9 @@ msgstr "" msgid "Failed to remove user key." msgstr "" +msgid "Failed to save comment!" +msgstr "" + msgid "Failed to save merge conflicts resolutions. Please try again!" msgstr "" diff --git a/spec/features/issuables/markdown_references/jira_spec.rb b/spec/features/issuables/markdown_references/jira_spec.rb index cc04798248c..8eaccfc0949 100644 --- a/spec/features/issuables/markdown_references/jira_spec.rb +++ b/spec/features/issuables/markdown_references/jira_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -describe "Jira", :js, :quarantine do +describe "Jira", :js do let(:user) { create(:user) } let(:actual_project) { create(:project, :public, :repository) } let(:merge_request) { create(:merge_request, target_project: actual_project, source_project: actual_project) } diff --git a/spec/frontend/notes/stores/utils_spec.js b/spec/frontend/notes/stores/utils_spec.js new file mode 100644 index 00000000000..b31b7491334 --- /dev/null +++ b/spec/frontend/notes/stores/utils_spec.js @@ -0,0 +1,17 @@ +import { hasQuickActions } from '~/notes/stores/utils'; + +describe('hasQuickActions', () => { + it.each` + input | expected + ${'some comment'} | ${false} + ${'/quickaction'} | ${true} + ${'some comment with\n/quickaction'} | ${true} + `('returns $expected for $input', ({ input, expected }) => { + expect(hasQuickActions(input)).toBe(expected); + }); + + it('is stateless', () => { + expect(hasQuickActions('some comment')).toBe(hasQuickActions('some comment')); + expect(hasQuickActions('/quickaction')).toBe(hasQuickActions('/quickaction')); + }); +}); diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index 05f10fb40f0..c138c87c4ac 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe GitlabSchema do + let(:user) { build :user } + it 'uses batch loading' do expect(field_instrumenters).to include(BatchLoader::GraphQL) end @@ -33,43 +35,75 @@ describe GitlabSchema do expect(connection).to eq(Gitlab::Graphql::Connections::KeysetConnection) end - context 'for different types of users' do - it 'returns DEFAULT_MAX_COMPLEXITY for no context' do - expect(GraphQL::Schema) - .to receive(:execute) - .with('query', hash_including(max_complexity: GitlabSchema::DEFAULT_MAX_COMPLEXITY)) + describe '.execute' do + context 'for different types of users' do + context 'when no context' do + it 'returns DEFAULT_MAX_COMPLEXITY' do + expect(GraphQL::Schema) + .to receive(:execute) + .with('query', hash_including(max_complexity: GitlabSchema::DEFAULT_MAX_COMPLEXITY)) - described_class.execute('query') - end + described_class.execute('query') + end + end - it 'returns DEFAULT_MAX_COMPLEXITY for no user' do - expect(GraphQL::Schema) - .to receive(:execute) - .with('query', hash_including(max_complexity: GitlabSchema::DEFAULT_MAX_COMPLEXITY)) + context 'when no user' do + it 'returns DEFAULT_MAX_COMPLEXITY' do + expect(GraphQL::Schema) + .to receive(:execute) + .with('query', hash_including(max_complexity: GitlabSchema::DEFAULT_MAX_COMPLEXITY)) - described_class.execute('query', context: {}) - end + described_class.execute('query', context: {}) + end - it 'returns AUTHENTICATED_COMPLEXITY for a logged in user' do - user = build :user + it 'returns ANONYMOUS_MAX_DEPTH' do + expect(GraphQL::Schema) + .to receive(:execute) + .with('query', hash_including(max_depth: GitlabSchema::ANONYMOUS_MAX_DEPTH)) - expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::AUTHENTICATED_COMPLEXITY)) + described_class.execute('query', context: {}) + end + end - described_class.execute('query', context: { current_user: user }) - end + context 'when a logged in user' do + it 'returns AUTHENTICATED_COMPLEXITY' do + expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::AUTHENTICATED_COMPLEXITY)) - it 'returns ADMIN_COMPLEXITY for an admin user' do - user = build :user, :admin + described_class.execute('query', context: { current_user: user }) + end - expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::ADMIN_COMPLEXITY)) + it 'returns AUTHENTICATED_MAX_DEPTH' do + expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_depth: GitlabSchema::AUTHENTICATED_MAX_DEPTH)) - described_class.execute('query', context: { current_user: user }) - end + described_class.execute('query', context: { current_user: user }) + end + end + + context 'when an admin user' do + it 'returns ADMIN_COMPLEXITY' do + user = build :user, :admin + + expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::ADMIN_COMPLEXITY)) + + described_class.execute('query', context: { current_user: user }) + end + end + + context 'when max_complexity passed on the query' do + it 'returns what was passed on the query' do + expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: 1234)) + + described_class.execute('query', max_complexity: 1234) + end + end - it 'returns what was passed on the query' do - expect(GraphQL::Schema).to receive(:execute).with('query', { max_complexity: 1234 }) + context 'when max_depth passed on the query' do + it 'returns what was passed on the query' do + expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_depth: 1234)) - described_class.execute('query', max_complexity: 1234) + described_class.execute('query', max_depth: 1234) + end + end end end diff --git a/spec/javascripts/ci_variable_list/ajax_variable_list_spec.js b/spec/javascripts/ci_variable_list/ajax_variable_list_spec.js index 481b1a4d4b0..2839922fbd3 100644 --- a/spec/javascripts/ci_variable_list/ajax_variable_list_spec.js +++ b/spec/javascripts/ci_variable_list/ajax_variable_list_spec.js @@ -113,7 +113,7 @@ describe('AjaxFormVariableList', () => { it('hides secret values', done => { mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200, {}); - const row = container.querySelector('.js-row:first-child'); + const row = container.querySelector('.js-row'); const valueInput = row.querySelector('.js-ci-variable-input-value'); const valuePlaceholder = row.querySelector('.js-secret-value-placeholder'); diff --git a/spec/javascripts/notes/stores/actions_spec.js b/spec/javascripts/notes/stores/actions_spec.js index 39901276b8c..7a9f32ddcff 100644 --- a/spec/javascripts/notes/stores/actions_spec.js +++ b/spec/javascripts/notes/stores/actions_spec.js @@ -794,6 +794,51 @@ describe('Actions Notes Store', () => { }); }); + describe('saveNote', () => { + const payload = { endpoint: TEST_HOST, data: { 'note[note]': 'some text' } }; + + describe('if response contains errors', () => { + const res = { errors: { something: ['went wrong'] } }; + + it('throws an error', done => { + actions + .saveNote( + { + commit() {}, + dispatch: () => Promise.resolve(res), + }, + payload, + ) + .then(() => done.fail('Expected error to be thrown!')) + .catch(error => { + expect(error.message).toBe('Failed to save comment!'); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('if response contains no errors', () => { + const res = { valid: true }; + + it('returns the response', done => { + actions + .saveNote( + { + commit() {}, + dispatch: () => Promise.resolve(res), + }, + payload, + ) + .then(data => { + expect(data).toBe(res); + }) + .then(done) + .catch(done.fail); + }); + }); + }); + describe('submitSuggestion', () => { const discussionId = 'discussion-id'; const noteId = 'note-id'; diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index e1a2bae5fe8..a02c00e3340 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -222,6 +222,46 @@ describe Gitlab::BitbucketImport::Importer do body: {}.to_json) end + context 'creating labels on project' do + before do + allow(importer).to receive(:import_wiki) + end + + it 'creates labels as expected' do + expect { importer.execute }.to change { Label.count }.from(0).to(Gitlab::BitbucketImport::Importer::LABELS.size) + end + + it 'does not fail if label is already existing' do + label = Gitlab::BitbucketImport::Importer::LABELS.first + ::Labels::CreateService.new(label).execute(project: project) + + expect { importer.execute }.not_to raise_error + end + + it 'does not create new labels' do + Gitlab::BitbucketImport::Importer::LABELS.each do |label| + create(:label, project: project, title: label[:title]) + end + + expect { importer.execute }.not_to change { Label.count } + end + + it 'does not update existing ones' do + label_title = Gitlab::BitbucketImport::Importer::LABELS.first[:title] + existing_label = create(:label, project: project, title: label_title) + # Reload label from database so we avoid timestamp comparison issues related to time precision when comparing + # attributes later. + existing_label.reload + + Timecop.freeze(Time.now + 1.minute) do + importer.execute + + label_after_import = project.labels.find(existing_label.id) + expect(label_after_import.attributes).to eq(existing_label.attributes) + end + end + end + it 'maps statuses to open or closed' do allow(importer).to receive(:import_wiki) diff --git a/spec/requests/api/graphql/gitlab_schema_spec.rb b/spec/requests/api/graphql/gitlab_schema_spec.rb index b63b4fb34df..dd518274f82 100644 --- a/spec/requests/api/graphql/gitlab_schema_spec.rb +++ b/spec/requests/api/graphql/gitlab_schema_spec.rb @@ -3,15 +3,43 @@ require 'spec_helper' describe 'GitlabSchema configurations' do include GraphqlHelpers - it 'shows an error if complexity is too high' do - project = create(:project, :repository) - query = graphql_query_for('project', { 'fullPath' => project.full_path }, %w(id name description)) + let(:project) { create(:project, :repository) } + let(:query) { graphql_query_for('project', { 'fullPath' => project.full_path }, %w(id name description)) } + let(:current_user) { create(:user) } - allow(GitlabSchema).to receive(:max_query_complexity).and_return 1 + describe '#max_complexity' do + context 'when complexity is too high' do + it 'shows an error' do + allow(GitlabSchema).to receive(:max_query_complexity).and_return 1 - post_graphql(query, current_user: nil) + post_graphql(query, current_user: nil) - expect(graphql_errors.first['message']).to include('which exceeds max complexity of 1') + expect(graphql_errors.first['message']).to include('which exceeds max complexity of 1') + end + end + end + + describe '#max_depth' do + context 'when query depth is too high' do + it 'shows error' do + errors = [{ "message" => "Query has depth of 2, which exceeds max depth of 1" }] + allow(GitlabSchema).to receive(:max_query_depth).and_return 1 + + post_graphql(query) + + expect(graphql_errors).to eq(errors) + end + end + + context 'when query depth is within range' do + it 'has no error' do + allow(GitlabSchema).to receive(:max_query_depth).and_return 5 + + post_graphql(query) + + expect(graphql_errors).to be_nil + end + end end context 'when IntrospectionQuery' do diff --git a/spec/support/features/variable_list_shared_examples.rb b/spec/support/features/variable_list_shared_examples.rb index 693b796fbdc..92a19dd22a2 100644 --- a/spec/support/features/variable_list_shared_examples.rb +++ b/spec/support/features/variable_list_shared_examples.rb @@ -17,7 +17,7 @@ shared_examples 'variable list' do visit page_path # We check the first row because it re-sorts to alphabetical order on refresh - page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do + page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do expect(find('.js-ci-variable-input-key').value).to eq('key') expect(find('.js-ci-variable-input-value', visible: false).value).to eq('key_value') end @@ -38,7 +38,7 @@ shared_examples 'variable list' do visit page_path # We check the first row because it re-sorts to alphabetical order on refresh - page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do + page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do expect(find('.js-ci-variable-input-key').value).to eq('key') expect(find('.js-ci-variable-input-value', visible: false).value).to eq('key_value') expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('true') @@ -59,7 +59,7 @@ shared_examples 'variable list' do visit page_path # We check the first row because it re-sorts to alphabetical order on refresh - page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do + page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do expect(find('.js-ci-variable-input-key').value).to eq('key') expect(find('.js-ci-variable-input-value', visible: false).value).to eq('key_value') expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('true') @@ -116,19 +116,19 @@ shared_examples 'variable list' do page.within('.js-ci-variable-list-section') do expect(first('.js-ci-variable-input-key').value).to eq(variable.key) expect(first('.js-ci-variable-input-value', visible: false).value).to eq(variable.value) - expect(page).to have_content('*' * 20) + expect(page).to have_content('*' * 17) click_button('Reveal value') expect(first('.js-ci-variable-input-key').value).to eq(variable.key) expect(first('.js-ci-variable-input-value').value).to eq(variable.value) - expect(page).not_to have_content('*' * 20) + expect(page).not_to have_content('*' * 17) click_button('Hide value') expect(first('.js-ci-variable-input-key').value).to eq(variable.key) expect(first('.js-ci-variable-input-value', visible: false).value).to eq(variable.value) - expect(page).to have_content('*' * 20) + expect(page).to have_content('*' * 17) end end @@ -149,7 +149,7 @@ shared_examples 'variable list' do page.within('.js-ci-variable-list-section') do click_button('Reveal value') - page.within('.js-row:nth-child(1)') do + page.within('.js-row:nth-child(2)') do find('.js-ci-variable-input-key').set('new_key') find('.js-ci-variable-input-value').set('new_value') end @@ -159,7 +159,7 @@ shared_examples 'variable list' do visit page_path - page.within('.js-row:nth-child(1)') do + page.within('.js-row:nth-child(2)') do expect(find('.js-ci-variable-input-key').value).to eq('new_key') expect(find('.js-ci-variable-input-value', visible: false).value).to eq('new_value') end @@ -181,7 +181,7 @@ shared_examples 'variable list' do visit page_path # We check the first row because it re-sorts to alphabetical order on refresh - page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do + page.within('.js-ci-variable-list-section .js-row:nth-child(3)') do find('.ci-variable-protected-item .js-project-feature-toggle').click expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('true') @@ -193,7 +193,7 @@ shared_examples 'variable list' do visit page_path # We check the first row because it re-sorts to alphabetical order on refresh - page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do + page.within('.js-ci-variable-list-section .js-row:nth-child(3)') do expect(find('.js-ci-variable-input-key').value).to eq('unprotected_key') expect(find('.js-ci-variable-input-value', visible: false).value).to eq('unprotected_value') expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('true') @@ -215,7 +215,7 @@ shared_examples 'variable list' do visit page_path - page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do + page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do find('.ci-variable-protected-item .js-project-feature-toggle').click expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('false') @@ -226,7 +226,7 @@ shared_examples 'variable list' do visit page_path - page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do + page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do expect(find('.js-ci-variable-input-key').value).to eq('protected_key') expect(find('.js-ci-variable-input-value', visible: false).value).to eq('protected_value') expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('false') @@ -234,7 +234,7 @@ shared_examples 'variable list' do end it 'edits variable to be unmasked' do - page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do + page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('true') find('.ci-variable-masked-item .js-project-feature-toggle').click @@ -247,13 +247,13 @@ shared_examples 'variable list' do visit page_path - page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do + page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('false') end end it 'edits variable to be masked' do - page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do + page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('true') find('.ci-variable-masked-item .js-project-feature-toggle').click @@ -266,7 +266,7 @@ shared_examples 'variable list' do visit page_path - page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do + page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('false') find('.ci-variable-masked-item .js-project-feature-toggle').click @@ -279,7 +279,7 @@ shared_examples 'variable list' do visit page_path - page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do + page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('true') end end @@ -302,7 +302,7 @@ shared_examples 'variable list' do expect(page).to have_selector('.js-row', count: 4) # Remove the `akey` variable - page.within('.js-row:nth-child(2)') do + page.within('.js-row:nth-child(3)') do first('.js-row-remove-button').click end diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index b49d743fb9a..f15944652fd 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -102,6 +102,7 @@ module GraphqlHelpers def all_graphql_fields_for(class_name, parent_types = Set.new) allow_unlimited_graphql_complexity + allow_unlimited_graphql_depth type = GitlabSchema.types[class_name.to_s] return "" unless type @@ -190,4 +191,9 @@ module GraphqlHelpers allow_any_instance_of(GitlabSchema).to receive(:max_complexity).and_return nil allow(GitlabSchema).to receive(:max_query_complexity).with(any_args).and_return nil end + + def allow_unlimited_graphql_depth + allow_any_instance_of(GitlabSchema).to receive(:max_depth).and_return nil + allow(GitlabSchema).to receive(:max_query_depth).with(any_args).and_return nil + end end |