Add latest changes from gitlab-org/gitlab@master

CHANGELOG-EE.md

@@ -480,6 +480,13 @@ Please view this file on the master branch, on stable branches it's out of date.
 - Fixes style-lint errors and warnings for EE builds.scss file.
+## 12.2.11
+
+### Fixed (1 change)
+
+- Backport the new reliable fetcher. !21198
+
+
 ## 12.2.8
 ### Fixed (1 change)
@@ -830,6 +837,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 ## 12.0.10
+- No changes.
 ### Fixed (1 change)
 - Backport the new reliable fetcher to 12.0.9. !20532

app/assets/javascripts/ide/stores/actions.js

@@ -20,7 +20,7 @@ export const discardAllChanges = ({ state, commit, dispatch }) => {
     commit(types.DISCARD_FILE_CHANGES, file.path);
     if (file.tempFile) {
-      dispatch('closeFile', file.path);
+      dispatch('closeFile', file);
     }
   });

app/assets/javascripts/notes/components/comment_form.vue

@@ -193,23 +193,10 @@ export default {
         this.stopPolling();
         this.saveNote(noteData)
-          .then(res => {
+          .then(() => {
             this.enableButton();
             this.restartPolling();
-
-            if (res.errors) {
-              if (res.errors.commands_only) {
-                this.discard();
-              } else {
-                Flash(
-                  __('Something went wrong while adding your comment. Please try again.'),
-                  'alert',
-                  this.$refs.commentForm,
-                );
-              }
-            } else {
-              this.discard();
-            }
+            this.discard();
             if (withIssueAction) {
               this.toggleIssueState();

app/assets/javascripts/notes/components/noteable_discussion.vue

@@ -198,23 +198,22 @@ export default {
         data: postData,
       };
-      this.isReplying = false;
       this.saveNote(replyData)
-        .then(() => {
-          clearDraft(this.autosaveKey);
+        .then(res => {
+          if (res.hasFlash !== true) {
+            this.isReplying = false;
+            clearDraft(this.autosaveKey);
+          }
           callback();
         })
         .catch(err => {
           this.removePlaceholderNotes();
-          this.isReplying = true;
-          this.$nextTick(() => {
-            const msg = __(
-              'Your comment could not be submitted! Please check your network connection and try again.',
-            );
-            Flash(msg, 'alert', this.$el);
-            this.$refs.noteForm.note = noteText;
-            callback(err);
-          });
+          const msg = __(
+            'Your comment could not be submitted! Please check your network connection and try again.',
+          );
+          Flash(msg, 'alert', this.$el);
+          this.$refs.noteForm.note = noteText;
+          callback(err);
         });
     },
     jumpToNextDiscussion() {

app/assets/javascripts/notes/stores/actions.js

@@ -14,7 +14,7 @@ import sidebarTimeTrackingEventHub from '../../sidebar/event_hub';
 import { isInViewport, scrollToElement, isInMRPage } from '../../lib/utils/common_utils';
 import { mergeUrlParams } from '../../lib/utils/url_utility';
 import mrWidgetEventHub from '../../vue_merge_request_widget/event_hub';
-import { __ } from '~/locale';
+import { __, sprintf } from '~/locale';
 import Api from '~/api';
 let eTagPoll;
@@ -252,29 +252,22 @@ export const saveNote = ({ commit, dispatch }, noteData) => {
     }
   }
-  const processErrors = res => {
-    const { errors } = res;
-    if (!errors || !Object.keys(errors).length) {
-      return res;
-    }
-
+  const processQuickActions = res => {
+    const { errors: { commands_only: message } = { commands_only: null } } = res;
     /*
      The following reply means that quick actions have been successfully applied:
      {"commands_changes":{},"valid":false,"errors":{"commands_only":["Commands applied"]}}
      */
-    if (hasQuickActions) {
+    if (hasQuickActions && message) {
       eTagPoll.makeRequest();
       $('.js-gfm-input').trigger('clear-commands-cache.atwho');
-      const { commands_only: message } = errors;
       Flash(message || __('Commands applied'), 'notice', noteData.flashContainer);
-
-      return res;
     }
-    throw new Error(__('Failed to save comment!'));
+    return res;
   };
   const processEmojiAward = res => {
@@ -321,11 +314,33 @@ export const saveNote = ({ commit, dispatch }, noteData) => {
     return res;
   };
+  const processErrors = error => {
+    if (error.response) {
+      const {
+        response: { data = {} },
+      } = error;
+      const { errors = {} } = data;
+      const { base = [] } = errors;
+
+      // we handle only errors.base for now
+      if (base.length > 0) {
+        const errorMsg = sprintf(__('Your comment could not be submitted because %{error}'), {
+          error: base[0].toLowerCase(),
+        });
+        Flash(errorMsg, 'alert', noteData.flashContainer);
+        return { ...data, hasFlash: true };
+      }
+    }
+
+    throw error;
+  };
+
   return dispatch(methodToDispatch, postData, { root: true })
-    .then(processErrors)
+    .then(processQuickActions)
     .then(processEmojiAward)
     .then(processTimeTracking)
-    .then(removePlaceholder);
+    .then(removePlaceholder)
+    .catch(processErrors);
 };
 const pollSuccessCallBack = (resp, commit, state, getters, dispatch) => {

app/services/notes/build_service.rb

@@ -11,7 +11,7 @@ module Notes
         unless discussion && can?(current_user, :create_note, discussion.noteable)
           note = Note.new
-          note.errors.add(:base, 'Discussion to reply to cannot be found')
+          note.errors.add(:base, _('Discussion to reply to cannot be found'))
           return note
         end

app/services/notes/create_service.rb

@@ -2,6 +2,7 @@
 module Notes
   class CreateService < ::Notes::BaseService
+    # rubocop:disable Metrics/CyclomaticComplexity
     def execute
       merge_request_diff_head_sha = params.delete(:merge_request_diff_head_sha)
@@ -9,7 +10,9 @@ module Notes
       # n+1: https://gitlab.com/gitlab-org/gitlab-foss/issues/37440
       note_valid = Gitlab::GitalyClient.allow_n_plus_1_calls do
-        note.valid?
+        # We may set errors manually in Notes::BuildService for this reason
+        # we also need to check for already existing errors.
+        note.errors.empty? && note.valid?
       end
       return note unless note_valid
@@ -67,6 +70,7 @@ module Notes
       note
     end
+    # rubocop:enable Metrics/CyclomaticComplexity
     private

changelogs/unreleased/27239-display-a-better-message-when-starting-a-discussion-on-a-deleted-co.yml

+---
+title: Display a better message when starting a discussion on a deleted comment
+merge_request: 20031
+author: Jacopo Beschi @jacopo-beschi
+type: changed

changelogs/unreleased/27244-webide-discard-all-changes.yml

+---
+title: "Web IDE: Fix the console error that happens when discarding a newly added/uploaded file."
+merge_request: 21537
+author:
+type: fixed

doc/development/new_fe_guide/tips.md

@@ -23,3 +23,7 @@ Your feature flag can now be:
 - [Deleting a feature flag](../../api/features.md#delete-a-feature)
 - [Manage feature flags](../feature_flags/process.md)
 - [Feature flags API](../../api/features.md)
+
+## Running tests locally
+
+This can be done as outlined by the [frontend testing guide](../testing_guide/frontend_testing.md#running-frontend-tests).

doc/development/testing_guide/frontend_testing.md

@@ -552,6 +552,7 @@ For running the frontend tests, you need the following commands:
 - `rake frontend:fixtures` (re-)generates [fixtures](#frontend-test-fixtures).
 - `yarn test` executes the tests.
+- `yarn jest` executes only the Jest tests.
 As long as the fixtures don't change, `yarn test` is sufficient (and saves you some time).
@@ -593,6 +594,24 @@ glob otherwise your shell may split it into multiple arguments:
 yarn karma -f 'spec/javascripts/ide/**/file_spec.js'
 ```
+It is also possible to target individual Jest / RSpec tests:
+
+```bash
+# Run specific jest file
+yarn jest ./path/to/local_spec.js
+# Run specific jest folder
+yarn jest ./path/to/folder/
+# Run all jest files which path contain term
+yarn jest term
+```
+
+```bash
+# Run specific rspec file
+rspec ./path/to/local_spec.rb
+# Run specific block within rspec file
+rspec ./path/to/local_spec.rb:15
+```
+
 ## Frontend test fixtures
 Code that is added to HAML templates (in `app/views/`) or makes Ajax requests to the backend has tests that require HTML or JSON from the backend.

lib/gitlab/kubernetes/helm/client_command.rb

@@ -5,14 +5,24 @@ module Gitlab
     module Helm
       module ClientCommand
         def init_command
-          # Here we are always upgrading to the latest version of Tiller when
-          # installing an app. We ensure the helm version stored in the
-          # database is correct by also updating this after transition to
-          # :installed,:updated in Clusters::Concerns::ApplicationStatus
-          'helm init --upgrade'
+          if local_tiller_enabled?
+            <<~HEREDOC.chomp
+            export HELM_HOST="localhost:44134"
+            tiller -listen ${HELM_HOST} -alsologtostderr &
+            helm init --client-only
+            HEREDOC
+          else
+            # Here we are always upgrading to the latest version of Tiller when
+            # installing an app. We ensure the helm version stored in the
+            # database is correct by also updating this after transition to
+            # :installed,:updated in Clusters::Concerns::ApplicationStatus
+            'helm init --upgrade'
+          end
         end
         def wait_for_tiller_command
+          return if local_tiller_enabled?
+
           helm_check = ['helm', 'version', *optional_tls_flags].shelljoin
           # This is necessary to give Tiller time to restart after upgrade.
           # Ideally we'd be able to use --wait but cannot because of
@@ -25,6 +35,14 @@ module Gitlab
           ['helm', 'repo', 'add', name, repository].shelljoin if repository
         end
+        private
+
+        def tls_flags_if_remote_tiller
+          return [] if local_tiller_enabled?
+
+          optional_tls_flags
+        end
+
         def optional_tls_flags
           return [] unless files.key?(:'ca.pem')
@@ -35,6 +53,10 @@ module Gitlab
             '--tls-key', "#{files_dir}/key.pem"
           ]
         end
+
+        def local_tiller_enabled?
+          Feature.enabled?(:managed_apps_local_tiller)
+        end
       end
     end
   end

lib/gitlab/kubernetes/helm/delete_command.rb

@@ -39,7 +39,7 @@ module Gitlab
         private
         def delete_command
-          command = ['helm', 'delete', '--purge', name] + optional_tls_flags
+          command = ['helm', 'delete', '--purge', name] + tls_flags_if_remote_tiller
           command.shelljoin
         end

lib/gitlab/kubernetes/helm/install_command.rb

@@ -49,7 +49,7 @@ module Gitlab
           command = ['helm', 'upgrade', name, chart] +
             install_flag +
             reset_values_flag +
-            optional_tls_flags +
+            tls_flags_if_remote_tiller +
             optional_version_flag +
             rbac_create_flag +
             namespace_flag +

locale/gitlab.pot

@@ -6078,6 +6078,9 @@ msgstr ""
 msgid "Discussion"
 msgstr ""
+msgid "Discussion to reply to cannot be found"
+msgstr ""
+
 msgid "Disk Usage"
 msgstr ""
@@ -7395,9 +7398,6 @@ msgstr ""
 msgid "Failed to reset key. Please try again."
 msgstr ""
-msgid "Failed to save comment!"
-msgstr ""
-
 msgid "Failed to save merge conflicts resolutions. Please try again!"
 msgstr ""
@@ -16399,9 +16399,6 @@ msgstr ""
 msgid "Something went wrong while adding your award. Please try again."
 msgstr ""
-msgid "Something went wrong while adding your comment. Please try again."
-msgstr ""
-
 msgid "Something went wrong while applying the suggestion. Please try again."
 msgstr ""
@@ -20647,6 +20644,9 @@ msgstr ""
 msgid "Your changes have been successfully committed."
 msgstr ""
+msgid "Your comment could not be submitted because %{error}"
+msgstr ""
+
 msgid "Your comment could not be submitted! Please check your network connection and try again."
 msgstr ""

qa/qa/page/merge_request/show.rb

@@ -91,16 +91,14 @@ module QA
         end
         def merge_immediately
-          wait(reload: false, max: 60) do
-            has_merge_options?
+          wait(reload: false) do
+            finished_loading?
           end
           if has_merge_options?
-            if has_no_element? :merge_immediately_option
-              retry_until do
-                click_element :merge_moment_dropdown
-                has_element? :merge_immediately_option
-              end
+            retry_until do
+              click_element :merge_moment_dropdown
+              has_element? :merge_immediately_option
             end
             click_element :merge_immediately_option
@@ -108,7 +106,7 @@ module QA
             click_element :merge_button
           end
-          wait(reload: false, max: 60) do
+          wait(reload: false) do
             merged?
           end
         end

spec/features/merge_request/user_posts_notes_spec.rb

@@ -95,6 +95,24 @@ describe 'Merge request > User posts notes', :js do
     end
   end
+  describe 'reply on a deleted conversation' do
+    before do
+      visit project_merge_request_path(project, merge_request)
+    end
+
+    it 'shows an error message' do
+      find('.js-reply-button').click
+      note.delete
+
+      page.within('.discussion-reply-holder') do
+        fill_in 'note[note]', with: 'A reply'
+        click_button 'Comment'
+        wait_for_requests
+        expect(page).to have_content('Your comment could not be submitted because discussion to reply to cannot be found')
+      end
+    end
+  end
+
   describe 'when previewing a note' do
     it 'shows the toolbar buttons when editing a note' do
       page.within('.js-main-target-form') do

spec/javascripts/diffs/store/actions_spec.js

@@ -145,8 +145,7 @@ describe('DiffsStoreActions', () => {
   });
   describe('fetchDiffFilesBatch', () => {
-    // eslint-disable-next-line jasmine/no-focused-tests
-    fit('should fetch batch diff files', done => {
+    it('should fetch batch diff files', done => {
       const endpointBatch = '/fetch/diffs_batch';
       const batch1 = `${endpointBatch}?per_page=${DIFFS_PER_PAGE}`;
       const batch2 = `${endpointBatch}?per_page=${DIFFS_PER_PAGE}&page=2`;

spec/javascripts/ide/stores/actions_spec.js

@@ -12,6 +12,7 @@ import actions, {
   renameEntry,
   getBranchData,
   createTempEntry,
+  discardAllChanges,
 } from '~/ide/stores/actions';
 import axios from '~/lib/utils/axios_utils';
 import { createStore } from '~/ide/stores';
@@ -60,8 +61,9 @@ describe('Multi-file store actions', () => {
   });
   describe('discardAllChanges', () => {
+    let f;
     beforeEach(() => {
-      const f = file('discardAll');
+      f = file('discardAll');
       f.changed = true;
       store.state.openFiles.push(f);
@@ -89,6 +91,27 @@ describe('Multi-file store actions', () => {
         .then(done)
         .catch(done.fail);
     });
+
+    it('closes the temp file if it was open', done => {
+      f.tempFile = true;
+
+      testAction(
+        discardAllChanges,
+        undefined,
+        store.state,
+        [
+          { type: types.DISCARD_FILE_CHANGES, payload: 'discardAll' },
+          { type: types.REMOVE_ALL_CHANGES_FILES },
+        ],
+        [
+          {
+            type: 'closeFile',
+            payload: jasmine.objectContaining({ path: 'discardAll' }),
+          },
+        ],
+        done,
+      );
+    });
   });
   describe('closeAllFiles', () => {

spec/javascripts/notes/stores/actions_spec.js

@@ -751,24 +751,54 @@ describe('Actions Notes Store', () => {
   });
   describe('saveNote', () => {
-    const payload = { endpoint: TEST_HOST, data: { 'note[note]': 'some text' } };
+    const flashContainer = {};
+    const payload = { endpoint: TEST_HOST, data: { 'note[note]': 'some text' }, flashContainer };
     describe('if response contains errors', () => {
       const res = { errors: { something: ['went wrong'] } };
+      const error = { message: 'Unprocessable entity', response: { data: res } };
       it('throws an error', done => {
         actions
           .saveNote(
             {
               commit() {},
-              dispatch: () => Promise.resolve(res),
+              dispatch: () => Promise.reject(error),
             },
             payload,
           )
           .then(() => done.fail('Expected error to be thrown!'))
-          .catch(error => {
-            expect(error.message).toBe('Failed to save comment!');
+          .catch(err => {
+            expect(err).toBe(error);
+            expect(flashSpy).not.toHaveBeenCalled();
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+    });
+
+    describe('if response contains errors.base', () => {
+      const res = { errors: { base: ['something went wrong'] } };
+      const error = { message: 'Unprocessable entity', response: { data: res } };
+
+      it('sets flash alert using errors.base message', done => {
+        actions
+          .saveNote(
+            {
+              commit() {},
+              dispatch: () => Promise.reject(error),
+            },
+            { ...payload, flashContainer },
+          )
+          .then(resp => {
+            expect(resp.hasFlash).toBe(true);
+            expect(flashSpy).toHaveBeenCalledWith(
+              'Your comment could not be submitted because something went wrong',
+              'alert',
+              flashContainer,
+            );
           })
+          .catch(() => done.fail('Expected success response!'))
           .then(done)
           .catch(done.fail);
       });
@@ -788,6 +818,7 @@ describe('Actions Notes Store', () => {
           )
           .then(data => {
             expect(data).toBe(res);
+            expect(flashSpy).not.toHaveBeenCalled();
           })
           .then(done)
           .catch(done.fail);