Merge branch '29483-no-feedback-when-checking-on-checklist-if-potential-spam-was-detected' into 'master'

Resolve "No feedback when checking on checklist if potential spam was detected"

Closes #29483

See merge request gitlab-org/gitlab-ce!15408

(cherry picked from commit d32032de08585476c3b303803a307ff94d910ebd)

efc86c26 Mock spam?
8e9a9030 Merge remote-tracking branch 'origin/master' into…
019db0fd TEMP mock askismet spam check
11e51be6 respond_to json with json version of verify view in recaptcha_check_with_fallback
18955cdc Differentiate between spam related error and other update errors
9dcf093c Start recaptcha modal and render partial instead
64b13133 Append and remove api script when reCAPTCHA is needed
c9a67fab put-over-post the recaptcha form
cb22ae66 remove comments
20517d64 remove comments
e174d02e Review changes
7a7d4a2a Add spec
8f617b45 Added changelog
db3f1a47 Review changes
4d0c78de Add to tasklist
36ea921b Remove duplicate TaskList
6bffe620 Add app_spec for spam
d95350bc Finished specs
cf951cba FOR REVIEWERS
6c50e850 FOR MERGE
25931f73 Allow script src to be mockable
c3463e00 Merge remote-tracking branch 'origin/master' into…
2c170b8d fix eslint
a318c757 Fix redirect
8a02fbd0 BE review changes
dda03f4d Fix spec
910c7205 Fix spec
fafbd7e5 remove duplicate destroy

11 files changed, 249 insertions, 63 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/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'