diff options
36 files changed, 753 insertions, 179 deletions
diff --git a/app/assets/javascripts/issue.js b/app/assets/javascripts/issue.js index 7de07e9403d..91b5ef1c10a 100644 --- a/app/assets/javascripts/issue.js +++ b/app/assets/javascripts/issue.js @@ -8,18 +8,7 @@ import IssuablesHelper from './helpers/issuables_helper'; export default class Issue { constructor() { - if ($('a.btn-close').length) { - this.taskList = new TaskList({ - dataType: 'issue', - fieldName: 'description', - selector: '.detail-page-description', - onSuccess: (result) => { - document.querySelector('#task_status').innerText = result.task_status; - document.querySelector('#task_status_short').innerText = result.task_status_short; - } - }); - this.initIssueBtnEventListeners(); - } + if ($('a.btn-close').length) this.initIssueBtnEventListeners(); Issue.$btnNewBranch = $('#new-branch'); Issue.createMrDropdownWrap = document.querySelector('.create-mr-dropdown-wrap'); diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index 5bdc7c99503..c7ce16bb623 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -9,6 +9,7 @@ import descriptionComponent from './description.vue'; import editedComponent from './edited.vue'; import formComponent from './form.vue'; import '../../lib/utils/url_utility'; +import RecaptchaDialogImplementor from '../../vue_shared/mixins/recaptcha_dialog_implementor'; export default { props: { @@ -149,6 +150,11 @@ export default { editedComponent, formComponent, }, + + mixins: [ + RecaptchaDialogImplementor, + ], + methods: { openForm() { if (!this.showForm) { @@ -164,9 +170,11 @@ export default { closeForm() { this.showForm = false; }, + updateIssuable() { this.service.updateIssuable(this.store.formState) .then(res => res.json()) + .then(data => this.checkForSpam(data)) .then((data) => { if (location.pathname !== data.web_url) { gl.utils.visitUrl(data.web_url); @@ -179,11 +187,24 @@ export default { this.store.updateState(data); eventHub.$emit('close.form'); }) - .catch(() => { - eventHub.$emit('close.form'); - window.Flash(`Error updating ${this.issuableType}`); + .catch((error) => { + if (error && error.name === 'SpamError') { + this.openRecaptcha(); + } else { + eventHub.$emit('close.form'); + window.Flash(`Error updating ${this.issuableType}`); + } }); }, + + closeRecaptchaDialog() { + this.store.setFormState({ + updateLoading: false, + }); + + this.closeRecaptcha(); + }, + deleteIssuable() { this.service.deleteIssuable() .then(res => res.json()) @@ -237,9 +258,9 @@ export default { </script> <template> - <div> +<div> + <div v-if="canUpdate && showForm"> <form-component - v-if="canUpdate && showForm" :form-state="formState" :can-destroy="canDestroy" :issuable-templates="issuableTemplates" @@ -251,30 +272,37 @@ export default { :can-attach-file="canAttachFile" :enable-autocomplete="enableAutocomplete" /> - <div v-else> - <title-component - :issuable-ref="issuableRef" - :can-update="canUpdate" - :title-html="state.titleHtml" - :title-text="state.titleText" - :show-inline-edit-button="showInlineEditButton" - /> - <description-component - v-if="state.descriptionHtml" - :can-update="canUpdate" - :description-html="state.descriptionHtml" - :description-text="state.descriptionText" - :updated-at="state.updatedAt" - :task-status="state.taskStatus" - :issuable-type="issuableType" - :update-url="updateEndpoint" - /> - <edited-component - v-if="hasUpdated" - :updated-at="state.updatedAt" - :updated-by-name="state.updatedByName" - :updated-by-path="state.updatedByPath" - /> - </div> + + <recaptcha-dialog + v-show="showRecaptcha" + :html="recaptchaHTML" + @close="closeRecaptchaDialog" + /> + </div> + <div v-else> + <title-component + :issuable-ref="issuableRef" + :can-update="canUpdate" + :title-html="state.titleHtml" + :title-text="state.titleText" + :show-inline-edit-button="showInlineEditButton" + /> + <description-component + v-if="state.descriptionHtml" + :can-update="canUpdate" + :description-html="state.descriptionHtml" + :description-text="state.descriptionText" + :updated-at="state.updatedAt" + :task-status="state.taskStatus" + :issuable-type="issuableType" + :update-url="updateEndpoint" + /> + <edited-component + v-if="hasUpdated" + :updated-at="state.updatedAt" + :updated-by-name="state.updatedByName" + :updated-by-path="state.updatedByPath" + /> </div> +</div> </template> diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue index b7559ced946..feb73481422 100644 --- a/app/assets/javascripts/issue_show/components/description.vue +++ b/app/assets/javascripts/issue_show/components/description.vue @@ -1,9 +1,14 @@ <script> import animateMixin from '../mixins/animate'; import TaskList from '../../task_list'; + import RecaptchaDialogImplementor from '../../vue_shared/mixins/recaptcha_dialog_implementor'; export default { - mixins: [animateMixin], + mixins: [ + animateMixin, + RecaptchaDialogImplementor, + ], + props: { canUpdate: { type: Boolean, @@ -51,6 +56,7 @@ this.updateTaskStatusText(); }, }, + methods: { renderGFM() { $(this.$refs['gfm-content']).renderGFM(); @@ -61,9 +67,19 @@ dataType: this.issuableType, fieldName: 'description', selector: '.detail-page-description', + onSuccess: this.taskListUpdateSuccess.bind(this), }); } }, + + taskListUpdateSuccess(data) { + try { + this.checkForSpam(data); + } catch (error) { + if (error && error.name === 'SpamError') this.openRecaptcha(); + } + }, + updateTaskStatusText() { const taskRegexMatches = this.taskStatus.match(/(\d+) of ((?!0)\d+)/); const $issuableHeader = $('.issuable-meta'); @@ -109,5 +125,11 @@ :data-update-url="updateUrl" > </textarea> + + <recaptcha-dialog + v-show="showRecaptcha" + :html="recaptchaHTML" + @close="closeRecaptcha" + /> </div> </template> diff --git a/app/assets/javascripts/vue_shared/components/popup_dialog.vue b/app/assets/javascripts/vue_shared/components/popup_dialog.vue index 47efee64c6e..6d15bbd84ba 100644 --- a/app/assets/javascripts/vue_shared/components/popup_dialog.vue +++ b/app/assets/javascripts/vue_shared/components/popup_dialog.vue @@ -38,7 +38,8 @@ export default { }, primaryButtonLabel: { type: String, - required: true, + required: false, + default: '', }, submitDisabled: { type: Boolean, @@ -113,8 +114,9 @@ export default { {{ closeButtonLabel }} </button> <button + v-if="primaryButtonLabel" type="button" - class="btn pull-right" + class="btn pull-right js-primary-button" :disabled="submitDisabled" :class="btnKindClass" @click="emitSubmit(true)"> diff --git a/app/assets/javascripts/vue_shared/components/recaptcha_dialog.vue b/app/assets/javascripts/vue_shared/components/recaptcha_dialog.vue new file mode 100644 index 00000000000..3ec50f14eb4 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/recaptcha_dialog.vue @@ -0,0 +1,85 @@ +<script> +import PopupDialog from './popup_dialog.vue'; + +export default { + name: 'recaptcha-dialog', + + props: { + html: { + type: String, + required: false, + default: '', + }, + }, + + data() { + return { + script: {}, + scriptSrc: 'https://www.google.com/recaptcha/api.js', + }; + }, + + components: { + PopupDialog, + }, + + methods: { + appendRecaptchaScript() { + this.removeRecaptchaScript(); + + const script = document.createElement('script'); + script.src = this.scriptSrc; + script.classList.add('js-recaptcha-script'); + script.async = true; + script.defer = true; + + this.script = script; + + document.body.appendChild(script); + }, + + removeRecaptchaScript() { + if (this.script instanceof Element) this.script.remove(); + }, + + close() { + this.removeRecaptchaScript(); + this.$emit('close'); + }, + + submit() { + this.$el.querySelector('form').submit(); + }, + }, + + watch: { + html() { + this.appendRecaptchaScript(); + }, + }, + + mounted() { + window.recaptchaDialogCallback = this.submit.bind(this); + }, +}; +</script> + +<template> +<popup-dialog + kind="warning" + class="recaptcha-dialog js-recaptcha-dialog" + :hide-footer="true" + :title="__('Please solve the reCAPTCHA')" + @toggle="close" +> + <div slot="body"> + <p> + {{__('We want to be sure it is you, please confirm you are not a robot.')}} + </p> + <div + ref="recaptcha" + v-html="html" + ></div> + </div> +</popup-dialog> +</template> diff --git a/app/assets/javascripts/vue_shared/mixins/recaptcha_dialog_implementor.js b/app/assets/javascripts/vue_shared/mixins/recaptcha_dialog_implementor.js new file mode 100644 index 00000000000..ef70f9432e3 --- /dev/null +++ b/app/assets/javascripts/vue_shared/mixins/recaptcha_dialog_implementor.js @@ -0,0 +1,36 @@ +import RecaptchaDialog from '../components/recaptcha_dialog.vue'; + +export default { + data() { + return { + showRecaptcha: false, + recaptchaHTML: '', + }; + }, + + components: { + RecaptchaDialog, + }, + + 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/assets/stylesheets/framework/modal.scss b/app/assets/stylesheets/framework/modal.scss index 5c9838c1029..ce551e6b7ce 100644 --- a/app/assets/stylesheets/framework/modal.scss +++ b/app/assets/stylesheets/framework/modal.scss @@ -48,3 +48,10 @@ body.modal-open { display: block; } +.recaptcha-dialog .recaptcha-form { + display: inline-block; + + .recaptcha { + margin: 0; + } +} diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index 744e448e8df..ecac9be0360 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -25,7 +25,7 @@ module IssuableActions end format.json do - render_entity_json + recaptcha_check_with_fallback(false) { render_entity_json } end end diff --git a/app/controllers/concerns/spammable_actions.rb b/app/controllers/concerns/spammable_actions.rb index ada0dde87fb..03d8e188093 100644 --- a/app/controllers/concerns/spammable_actions.rb +++ b/app/controllers/concerns/spammable_actions.rb @@ -23,8 +23,8 @@ module SpammableActions @spam_config_loaded = Gitlab::Recaptcha.load_configurations! end - def recaptcha_check_with_fallback(&fallback) - if spammable.valid? + def recaptcha_check_with_fallback(should_redirect = true, &fallback) + if should_redirect && spammable.valid? redirect_to spammable_path elsif render_recaptcha? ensure_spam_config_loaded! @@ -33,7 +33,18 @@ module SpammableActions flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' end - render :verify + respond_to do |format| + format.html do + render :verify + end + + format.json do + locals = { spammable: spammable, script: false, has_submit: false } + recaptcha_html = render_to_string(partial: 'shared/recaptcha_form', formats: :html, locals: locals) + + render json: { recaptcha_html: recaptcha_html } + end + end else yield end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 1e5f2ed4dd2..85db2760e23 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -12,18 +12,19 @@ module Ci def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil, &block) @pipeline = Ci::Pipeline.new - command = OpenStruct.new(source: source, - origin_ref: params[:ref], - checkout_sha: params[:checkout_sha], - after_sha: params[:after], - before_sha: params[:before], - trigger_request: trigger_request, - schedule: schedule, - ignore_skip_ci: ignore_skip_ci, - save_incompleted: save_on_errors, - seeds_block: block, - project: project, - current_user: current_user) + command = Gitlab::Ci::Pipeline::Chain::Command.new( + source: source, + origin_ref: params[:ref], + checkout_sha: params[:checkout_sha], + after_sha: params[:after], + before_sha: params[:before], + trigger_request: trigger_request, + schedule: schedule, + ignore_skip_ci: ignore_skip_ci, + save_incompleted: save_on_errors, + seeds_block: block, + project: project, + current_user: current_user) sequence = Gitlab::Ci::Pipeline::Chain::Sequence .new(pipeline, command, SEQUENCE) diff --git a/app/views/layouts/_recaptcha_verification.html.haml b/app/views/layouts/_recaptcha_verification.html.haml index 77c77dc6754..e6f87ddd383 100644 --- a/app/views/layouts/_recaptcha_verification.html.haml +++ b/app/views/layouts/_recaptcha_verification.html.haml @@ -1,5 +1,4 @@ - humanized_resource_name = spammable.class.model_name.human.downcase -- resource_name = spammable.class.model_name.singular %h3.page-title Anti-spam verification @@ -8,16 +7,4 @@ %p #{"We detected potential spam in the #{humanized_resource_name}. Please solve the reCAPTCHA to proceed."} -= form_for form do |f| - .recaptcha - - params[resource_name].each do |field, value| - = hidden_field(resource_name, field, value: value) - = hidden_field_tag(:spam_log_id, spammable.spam_log.id) - = hidden_field_tag(:recaptcha_verification, true) - = recaptcha_tags - - -# Yields a block with given extra params. - = yield - - .row-content-block.footer-block - = f.submit "Submit #{humanized_resource_name}", class: 'btn btn-create' += render 'shared/recaptcha_form', spammable: spammable diff --git a/app/views/shared/_recaptcha_form.html.haml b/app/views/shared/_recaptcha_form.html.haml new file mode 100644 index 00000000000..0e816870f15 --- /dev/null +++ b/app/views/shared/_recaptcha_form.html.haml @@ -0,0 +1,19 @@ +- resource_name = spammable.class.model_name.singular +- humanized_resource_name = spammable.class.model_name.human.downcase +- script = local_assigns.fetch(:script, true) +- has_submit = local_assigns.fetch(:has_submit, true) + += form_for resource_name, method: :post, html: { class: 'recaptcha-form js-recaptcha-form' } do |f| + .recaptcha + - params[resource_name].each do |field, value| + = hidden_field(resource_name, field, value: value) + = hidden_field_tag(:spam_log_id, spammable.spam_log.id) + = hidden_field_tag(:recaptcha_verification, true) + = recaptcha_tags script: script, callback: 'recaptchaDialogCallback' + + -# Yields a block with given extra params. + = yield + + - if has_submit + .row-content-block.footer-block + = f.submit "Submit #{humanized_resource_name}", class: 'btn btn-create' diff --git a/changelogs/unreleased/29483-no-feedback-when-checking-on-checklist-if-potential-spam-was-detected.yml b/changelogs/unreleased/29483-no-feedback-when-checking-on-checklist-if-potential-spam-was-detected.yml new file mode 100644 index 00000000000..6bfcc5e70de --- /dev/null +++ b/changelogs/unreleased/29483-no-feedback-when-checking-on-checklist-if-potential-spam-was-detected.yml @@ -0,0 +1,5 @@ +--- +title: Add recaptcha modal to issue updates detected as spam +merge_request: 15408 +author: +type: fixed diff --git a/changelogs/unreleased/anchor-issue-references.yml b/changelogs/unreleased/anchor-issue-references.yml new file mode 100644 index 00000000000..78896427417 --- /dev/null +++ b/changelogs/unreleased/anchor-issue-references.yml @@ -0,0 +1,6 @@ +--- +title: Fix false positive issue references in merge requests caused by header anchor + links. +merge_request: +author: +type: fixed diff --git a/lib/banzai/filter/table_of_contents_filter.rb b/lib/banzai/filter/table_of_contents_filter.rb index 47151626208..97244159985 100644 --- a/lib/banzai/filter/table_of_contents_filter.rb +++ b/lib/banzai/filter/table_of_contents_filter.rb @@ -32,6 +32,7 @@ module Banzai .gsub(PUNCTUATION_REGEXP, '') # remove punctuation .tr(' ', '-') # replace spaces with dash .squeeze('-') # replace multiple dashes with one + .gsub(/\A(\d+)\z/, 'anchor-\1') # digits-only hrefs conflict with issue refs uniq = headers[id] > 0 ? "-#{headers[id]}" : '' headers[id] += 1 diff --git a/lib/gitlab/ci/pipeline/chain/base.rb b/lib/gitlab/ci/pipeline/chain/base.rb index 8d82e1b288d..efed19da21c 100644 --- a/lib/gitlab/ci/pipeline/chain/base.rb +++ b/lib/gitlab/ci/pipeline/chain/base.rb @@ -3,14 +3,13 @@ module Gitlab module Pipeline module Chain class Base - attr_reader :pipeline, :project, :current_user + attr_reader :pipeline, :command + + delegate :project, :current_user, to: :command def initialize(pipeline, command) @pipeline = pipeline @command = command - - @project = command.project - @current_user = command.current_user end def perform! diff --git a/lib/gitlab/ci/pipeline/chain/build.rb b/lib/gitlab/ci/pipeline/chain/build.rb index a126dded1ae..70732d26bbd 100644 --- a/lib/gitlab/ci/pipeline/chain/build.rb +++ b/lib/gitlab/ci/pipeline/chain/build.rb @@ -3,20 +3,18 @@ module Gitlab module Pipeline module Chain class Build < Chain::Base - include Chain::Helpers - def perform! @pipeline.assign_attributes( source: @command.source, - project: @project, - ref: ref, - sha: sha, - before_sha: before_sha, - tag: tag_exists?, + project: @command.project, + ref: @command.ref, + sha: @command.sha, + before_sha: @command.before_sha, + tag: @command.tag_exists?, trigger_requests: Array(@command.trigger_request), - user: @current_user, + user: @command.current_user, pipeline_schedule: @command.schedule, - protected: protected_ref? + protected: @command.protected_ref? ) @pipeline.set_config_source @@ -25,32 +23,6 @@ module Gitlab def break? false end - - private - - def ref - @ref ||= Gitlab::Git.ref_name(origin_ref) - end - - def sha - @project.commit(origin_sha || origin_ref).try(:id) - end - - def origin_ref - @command.origin_ref - end - - def origin_sha - @command.checkout_sha || @command.after_sha - end - - def before_sha - @command.checkout_sha || @command.before_sha || Gitlab::Git::BLANK_SHA - end - - def protected_ref? - @project.protected_for?(ref) - end end end end diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb new file mode 100644 index 00000000000..7b19b10e05b --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -0,0 +1,61 @@ +module Gitlab + module Ci + module Pipeline + module Chain + Command = Struct.new( + :source, :project, :current_user, + :origin_ref, :checkout_sha, :after_sha, :before_sha, + :trigger_request, :schedule, + :ignore_skip_ci, :save_incompleted, + :seeds_block + ) do + include Gitlab::Utils::StrongMemoize + + def initialize(**params) + params.each do |key, value| + self[key] = value + end + end + + def branch_exists? + strong_memoize(:is_branch) do + project.repository.branch_exists?(ref) + end + end + + def tag_exists? + strong_memoize(:is_tag) do + project.repository.tag_exists?(ref) + end + end + + def ref + strong_memoize(:ref) do + Gitlab::Git.ref_name(origin_ref) + end + end + + def sha + strong_memoize(:sha) do + project.commit(origin_sha || origin_ref).try(:id) + end + end + + def origin_sha + checkout_sha || after_sha + end + + def before_sha + self[:before_sha] || checkout_sha || Gitlab::Git::BLANK_SHA + end + + def protected_ref? + strong_memoize(:protected_ref) do + project.protected_for?(ref) + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/helpers.rb b/lib/gitlab/ci/pipeline/chain/helpers.rb index 02d81286f21..bf1380a1da9 100644 --- a/lib/gitlab/ci/pipeline/chain/helpers.rb +++ b/lib/gitlab/ci/pipeline/chain/helpers.rb @@ -3,18 +3,6 @@ module Gitlab module Pipeline module Chain module Helpers - def branch_exists? - return @is_branch if defined?(@is_branch) - - @is_branch = project.repository.branch_exists?(pipeline.ref) - end - - def tag_exists? - return @is_tag if defined?(@is_tag) - - @is_tag = project.repository.tag_exists?(pipeline.ref) - end - def error(message) pipeline.errors.add(:base, message) end diff --git a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb index 4913a604079..13c6fedd831 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb @@ -14,7 +14,7 @@ module Gitlab unless allowed_to_trigger_pipeline? if can?(current_user, :create_pipeline, project) - return error("Insufficient permissions for protected ref '#{pipeline.ref}'") + return error("Insufficient permissions for protected ref '#{command.ref}'") else return error('Insufficient permissions to create a new pipeline') end @@ -29,7 +29,7 @@ module Gitlab if current_user allowed_to_create? else # legacy triggers don't have a corresponding user - !project.protected_for?(@pipeline.ref) + !@command.protected_ref? end end @@ -38,10 +38,10 @@ module Gitlab access = Gitlab::UserAccess.new(current_user, project: project) - if branch_exists? - access.can_update_branch?(@pipeline.ref) - elsif tag_exists? - access.can_create_tag?(@pipeline.ref) + if @command.branch_exists? + access.can_update_branch?(@command.ref) + elsif @command.tag_exists? + access.can_create_tag?(@command.ref) else true # Allow it for now and we'll reject when we check ref existence end diff --git a/lib/gitlab/ci/pipeline/chain/validate/repository.rb b/lib/gitlab/ci/pipeline/chain/validate/repository.rb index 70a4cfdbdea..9699c24e5b6 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/repository.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/repository.rb @@ -7,14 +7,11 @@ module Gitlab include Chain::Helpers def perform! - unless branch_exists? || tag_exists? + unless @command.branch_exists? || @command.tag_exists? return error('Reference not found') end - ## TODO, we check commit in the service, that is why - # there is no repository access here. - # - unless pipeline.sha + unless @command.sha return error('Commit not found') end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 4dbbaecdd6d..c5d08cb0b9d 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -272,6 +272,20 @@ describe Projects::IssuesController do expect(response).to have_http_status(:ok) expect(issue.reload.title).to eq('New title') end + + context 'when Akismet is enabled and the issue is identified as spam' do + before do + stub_application_setting(recaptcha_enabled: true) + allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + end + + it 'renders json with recaptcha_html' do + subject + + expect(JSON.parse(response.body)).to have_key('recaptcha_html') + end + end end context 'when user does not have access to update issue' do @@ -504,17 +518,16 @@ describe Projects::IssuesController do expect(spam_logs.first.recaptcha_verified).to be_falsey end - it 'renders json errors' do + it 'renders recaptcha_html json response' do update_issue - expect(json_response) - .to eql("errors" => ["Your issue has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed."]) + expect(json_response).to have_key('recaptcha_html') end - it 'returns 422 status' do + it 'returns 200 status' do update_issue - expect(response).to have_gitlab_http_status(422) + expect(response).to have_gitlab_http_status(200) end end diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js index b47a8bf705f..53b8a368d28 100644 --- a/spec/javascripts/issue_show/components/app_spec.js +++ b/spec/javascripts/issue_show/components/app_spec.js @@ -4,6 +4,7 @@ import '~/render_gfm'; import issuableApp from '~/issue_show/components/app.vue'; import eventHub from '~/issue_show/event_hub'; import issueShowData from '../mock_data'; +import setTimeoutPromise from '../../helpers/set_timeout_promise_helper'; function formatText(text) { return text.trim().replace(/\s\s+/g, ' '); @@ -55,6 +56,8 @@ describe('Issuable output', () => { Vue.http.interceptors = _.without(Vue.http.interceptors, interceptor); vm.poll.stop(); + + vm.$destroy(); }); it('should render a title/description/edited and update title/description/edited on update', (done) => { @@ -268,6 +271,52 @@ describe('Issuable output', () => { }); }); + it('opens recaptcha dialog if update rejected as spam', (done) => { + function mockScriptSrc() { + const recaptchaChild = vm.$children + .find(child => child.$options._componentTag === 'recaptcha-dialog'); // eslint-disable-line no-underscore-dangle + + recaptchaChild.scriptSrc = '//scriptsrc'; + } + + let modal; + const promise = new Promise((resolve) => { + resolve({ + json() { + return { + recaptcha_html: '<div class="g-recaptcha">recaptcha_html</div>', + }; + }, + }); + }); + + spyOn(vm.service, 'updateIssuable').and.returnValue(promise); + + vm.canUpdate = true; + vm.showForm = true; + + vm.$nextTick() + .then(() => mockScriptSrc()) + .then(() => vm.updateIssuable()) + .then(promise) + .then(() => setTimeoutPromise()) + .then(() => { + modal = vm.$el.querySelector('.js-recaptcha-dialog'); + + expect(modal.style.display).not.toEqual('none'); + expect(modal.querySelector('.g-recaptcha').textContent).toEqual('recaptcha_html'); + expect(document.body.querySelector('.js-recaptcha-script').src).toMatch('//scriptsrc'); + }) + .then(() => modal.querySelector('.close').click()) + .then(() => vm.$nextTick()) + .then(() => { + expect(modal.style.display).toEqual('none'); + expect(document.body.querySelector('.js-recaptcha-script')).toBeNull(); + }) + .then(done) + .catch(done.fail); + }); + describe('deleteIssuable', () => { it('changes URL when deleted', (done) => { spyOn(gl.utils, 'visitUrl'); diff --git a/spec/javascripts/issue_show/components/description_spec.js b/spec/javascripts/issue_show/components/description_spec.js index 163e5cdd062..2e000a1063f 100644 --- a/spec/javascripts/issue_show/components/description_spec.js +++ b/spec/javascripts/issue_show/components/description_spec.js @@ -51,6 +51,35 @@ describe('Description component', () => { }); }); + it('opens recaptcha dialog if update rejected as spam', (done) => { + let modal; + const recaptchaChild = vm.$children + .find(child => child.$options._componentTag === 'recaptcha-dialog'); // eslint-disable-line no-underscore-dangle + + recaptchaChild.scriptSrc = '//scriptsrc'; + + vm.taskListUpdateSuccess({ + recaptcha_html: '<div class="g-recaptcha">recaptcha_html</div>', + }); + + vm.$nextTick() + .then(() => { + modal = vm.$el.querySelector('.js-recaptcha-dialog'); + + expect(modal.style.display).not.toEqual('none'); + expect(modal.querySelector('.g-recaptcha').textContent).toEqual('recaptcha_html'); + expect(document.body.querySelector('.js-recaptcha-script').src).toMatch('//scriptsrc'); + }) + .then(() => modal.querySelector('.close').click()) + .then(() => vm.$nextTick()) + .then(() => { + expect(modal.style.display).toEqual('none'); + expect(document.body.querySelector('.js-recaptcha-script')).toBeNull(); + }) + .then(done) + .catch(done.fail); + }); + describe('TaskList', () => { beforeEach(() => { vm = mountComponent(DescriptionComponent, Object.assign({}, props, { @@ -86,6 +115,7 @@ describe('Description component', () => { dataType: 'issuableType', fieldName: 'description', selector: '.detail-page-description', + onSuccess: jasmine.any(Function), }); done(); }); diff --git a/spec/javascripts/vue_shared/components/popup_dialog_spec.js b/spec/javascripts/vue_shared/components/popup_dialog_spec.js new file mode 100644 index 00000000000..5c1d2a196f4 --- /dev/null +++ b/spec/javascripts/vue_shared/components/popup_dialog_spec.js @@ -0,0 +1,12 @@ +import Vue from 'vue'; +import PopupDialog from '~/vue_shared/components/popup_dialog.vue'; +import mountComponent from '../../helpers/vue_mount_component_helper'; + +describe('PopupDialog', () => { + it('does not render a primary button if no primaryButtonLabel', () => { + const popupDialog = Vue.extend(PopupDialog); + const vm = mountComponent(popupDialog); + + expect(vm.$el.querySelector('.js-primary-button')).toBeNull(); + }); +}); diff --git a/spec/lib/banzai/filter/table_of_contents_filter_spec.rb b/spec/lib/banzai/filter/table_of_contents_filter_spec.rb index 85eddde732e..0cfef4ff5bf 100644 --- a/spec/lib/banzai/filter/table_of_contents_filter_spec.rb +++ b/spec/lib/banzai/filter/table_of_contents_filter_spec.rb @@ -65,6 +65,13 @@ describe Banzai::Filter::TableOfContentsFilter do expect(doc.css('h2 a').first.attr('href')).to eq '#one-1' end + it 'prepends a prefix to digits-only ids' do + doc = filter(header(1, "123") + header(2, "1.0")) + + expect(doc.css('h1 a').first.attr('href')).to eq '#anchor-123' + expect(doc.css('h2 a').first.attr('href')).to eq '#anchor-10' + end + it 'supports Unicode' do doc = filter(header(1, '한글')) expect(doc.css('h1 a').first.attr('id')).to eq 'user-content-한글' diff --git a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb index 0f1d72080c5..3ae7053a995 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb @@ -6,46 +6,81 @@ describe Gitlab::Ci::Pipeline::Chain::Build do let(:pipeline) { Ci::Pipeline.new } let(:command) do - double('command', source: :push, - origin_ref: 'master', - checkout_sha: project.commit.id, - after_sha: nil, - before_sha: nil, - trigger_request: nil, - schedule: nil, - project: project, - current_user: user) + Gitlab::Ci::Pipeline::Chain::Command.new( + source: :push, + origin_ref: 'master', + checkout_sha: project.commit.id, + after_sha: nil, + before_sha: nil, + trigger_request: nil, + schedule: nil, + project: project, + current_user: user) end let(:step) { described_class.new(pipeline, command) } before do stub_repository_ci_yaml_file(sha: anything) - - step.perform! end it 'never breaks the chain' do + step.perform! + expect(step.break?).to be false end it 'fills pipeline object with data' do + step.perform! + expect(pipeline.sha).not_to be_empty expect(pipeline.sha).to eq project.commit.id expect(pipeline.ref).to eq 'master' + expect(pipeline.tag).to be false expect(pipeline.user).to eq user expect(pipeline.project).to eq project end it 'sets a valid config source' do + step.perform! + expect(pipeline.repository_source?).to be true end it 'returns a valid pipeline' do + step.perform! + expect(pipeline).to be_valid end it 'does not persist a pipeline' do + step.perform! + expect(pipeline).not_to be_persisted end + + context 'when pipeline is running for a tag' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + source: :push, + origin_ref: 'mytag', + checkout_sha: project.commit.id, + after_sha: nil, + before_sha: nil, + trigger_request: nil, + schedule: nil, + project: project, + current_user: user) + end + + before do + allow_any_instance_of(Repository).to receive(:tag_exists?).with('mytag').and_return(true) + + step.perform! + end + + it 'correctly indicated that this is a tagged pipeline' do + expect(pipeline).to be_tag + end + end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb new file mode 100644 index 00000000000..75a177d2d1f --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -0,0 +1,185 @@ +require 'spec_helper' + +describe Gitlab::Ci::Pipeline::Chain::Command do + set(:project) { create(:project, :repository) } + + describe '#initialize' do + subject do + described_class.new(origin_ref: 'master') + end + + it 'properly initialises object from hash' do + expect(subject.origin_ref).to eq('master') + end + end + + context 'handling of origin_ref' do + let(:command) { described_class.new(project: project, origin_ref: origin_ref) } + + describe '#branch_exists?' do + subject { command.branch_exists? } + + context 'for existing branch' do + let(:origin_ref) { 'master' } + + it { is_expected.to eq(true) } + end + + context 'for invalid branch' do + let(:origin_ref) { 'something' } + + it { is_expected.to eq(false) } + end + end + + describe '#tag_exists?' do + subject { command.tag_exists? } + + context 'for existing ref' do + let(:origin_ref) { 'v1.0.0' } + + it { is_expected.to eq(true) } + end + + context 'for invalid ref' do + let(:origin_ref) { 'something' } + + it { is_expected.to eq(false) } + end + end + + describe '#ref' do + subject { command.ref } + + context 'for regular ref' do + let(:origin_ref) { 'master' } + + it { is_expected.to eq('master') } + end + + context 'for branch ref' do + let(:origin_ref) { 'refs/heads/master' } + + it { is_expected.to eq('master') } + end + + context 'for tag ref' do + let(:origin_ref) { 'refs/tags/1.0.0' } + + it { is_expected.to eq('1.0.0') } + end + + context 'for other refs' do + let(:origin_ref) { 'refs/merge-requests/11/head' } + + it { is_expected.to eq('refs/merge-requests/11/head') } + end + end + end + + describe '#sha' do + subject { command.sha } + + context 'when invalid checkout_sha is specified' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa') } + + it 'returns empty value' do + is_expected.to be_nil + end + end + + context 'when a valid checkout_sha is specified' do + let(:command) { described_class.new(project: project, checkout_sha: project.commit.id) } + + it 'returns checkout_sha' do + is_expected.to eq(project.commit.id) + end + end + + context 'when a valid after_sha is specified' do + let(:command) { described_class.new(project: project, after_sha: project.commit.id) } + + it 'returns after_sha' do + is_expected.to eq(project.commit.id) + end + end + + context 'when a valid origin_ref is specified' do + let(:command) { described_class.new(project: project, origin_ref: 'HEAD') } + + it 'returns SHA for given ref' do + is_expected.to eq(project.commit.id) + end + end + end + + describe '#origin_sha' do + subject { command.origin_sha } + + context 'when using checkout_sha and after_sha' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa', after_sha: 'bbb') } + + it 'uses checkout_sha' do + is_expected.to eq('aaa') + end + end + + context 'when using after_sha only' do + let(:command) { described_class.new(project: project, after_sha: 'bbb') } + + it 'uses after_sha' do + is_expected.to eq('bbb') + end + end + end + + describe '#before_sha' do + subject { command.before_sha } + + context 'when using checkout_sha and before_sha' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa', before_sha: 'bbb') } + + it 'uses before_sha' do + is_expected.to eq('bbb') + end + end + + context 'when using checkout_sha only' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa') } + + it 'uses checkout_sha' do + is_expected.to eq('aaa') + end + end + + context 'when checkout_sha and before_sha are empty' do + let(:command) { described_class.new(project: project) } + + it 'uses BLANK_SHA' do + is_expected.to eq(Gitlab::Git::BLANK_SHA) + end + end + end + + describe '#protected_ref?' do + let(:command) { described_class.new(project: project, origin_ref: 'my-branch') } + + subject { command.protected_ref? } + + context 'when a ref is protected' do + before do + expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(true) + end + + it { is_expected.to eq(true) } + end + + context 'when a ref is unprotected' do + before do + expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(false) + end + + it { is_expected.to eq(false) } + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb index f54e2326b06..1b03227d67b 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb @@ -10,9 +10,9 @@ describe Gitlab::Ci::Pipeline::Chain::Create do end let(:command) do - double('command', project: project, - current_user: user, - seeds_block: nil) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, seeds_block: nil) end let(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb index e165e0fac2a..eca23694a2b 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::Ci::Pipeline::Chain::Sequence do set(:user) { create(:user) } let(:pipeline) { build_stubbed(:ci_pipeline) } - let(:command) { double('command' ) } + let(:command) { Gitlab::Ci::Pipeline::Chain::Command.new } let(:first_step) { spy('first step') } let(:second_step) { spy('second step') } let(:sequence) { [first_step, second_step] } diff --git a/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb index 32bd5de829b..dc13cae961c 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb @@ -6,10 +6,11 @@ describe Gitlab::Ci::Pipeline::Chain::Skip do set(:pipeline) { create(:ci_pipeline, project: project) } let(:command) do - double('command', project: project, - current_user: user, - ignore_skip_ci: false, - save_incompleted: true) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, + ignore_skip_ci: false, + save_incompleted: true) end let(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb index 0bbdd23f4d6..a973ccda8de 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb @@ -5,11 +5,12 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do set(:user) { create(:user) } let(:pipeline) do - build_stubbed(:ci_pipeline, ref: ref, project: project) + build_stubbed(:ci_pipeline, project: project) end let(:command) do - double('command', project: project, current_user: user) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: ref) end let(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb index 8357af38f92..5c12c6e6392 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb @@ -5,9 +5,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Config do set(:user) { create(:user) } let(:command) do - double('command', project: project, - current_user: user, - save_incompleted: true) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, + save_incompleted: true) end let!(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb index bb356efe9ad..fb1b53fc55c 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb @@ -3,10 +3,7 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do set(:project) { create(:project, :repository) } set(:user) { create(:user) } - - let(:command) do - double('command', project: project, current_user: user) - end + let(:pipeline) { build_stubbed(:ci_pipeline) } let!(:step) { described_class.new(pipeline, command) } @@ -14,9 +11,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do step.perform! end - context 'when pipeline ref and sha exists' do - let(:pipeline) do - build_stubbed(:ci_pipeline, ref: 'master', sha: '123', project: project) + context 'when ref and sha exists' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'master', checkout_sha: project.commit.id) end it 'does not break the chain' do @@ -28,9 +26,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do end end - context 'when pipeline ref does not exist' do - let(:pipeline) do - build_stubbed(:ci_pipeline, ref: 'something', project: project) + context 'when ref does not exist' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'something') end it 'breaks the chain' do @@ -43,9 +42,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do end end - context 'when pipeline does not have SHA set' do - let(:pipeline) do - build_stubbed(:ci_pipeline, ref: 'master', sha: nil, project: project) + context 'when does not have existing SHA set' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'master', checkout_sha: 'something') end it 'breaks the chain' do diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index ef874368077..8ec3f55e6de 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -115,6 +115,15 @@ describe Gitlab::ReferenceExtractor do end end + it 'does not include anchors from table of contents in issue references' do + issue1 = create(:issue, project: project) + issue2 = create(:issue, project: project) + + subject.analyze("not real issue <h4>#{issue1.iid}</h4>, real issue #{issue2.to_reference}") + + expect(subject.issues).to match_array([issue2]) + end + it 'accesses valid issue objects' do @i0 = create(:issue, project: project) @i1 = create(:issue, project: project) diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index b0de8d447a2..41ce81e0651 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -518,5 +518,20 @@ describe Ci::CreatePipelineService do end end end + + context 'when pipeline is running for a tag' do + before do + config = YAML.dump(test: { script: 'test', only: ['branches'] }, + deploy: { script: 'deploy', only: ['tags'] }) + + stub_ci_pipeline_yaml_file(config) + end + + it 'creates a tagged pipeline' do + pipeline = execute_service(ref: 'v1.0.0') + + expect(pipeline.tag?).to be true + end + end end end |