Automatic merge of gitlab-org/gitlab-ce master

.gitlab/ci/review.gitlab-ci.yml

@@ -83,7 +83,6 @@ schedule:review-build-cng:
   <<: *review-schedules-only
   <<: *review-build-cng-base
 
-
 .review-deploy-base: &review-deploy-base
   <<: *review-base
   allow_failure: true
@@ -176,6 +175,7 @@ review-performance:
   <<: *review-performance-base
 
 review-stop:
+  <<: *review-base
   extends: .single-script-job-dedicated-runner
   image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-charts-build-base
   allow_failure: true

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

+<script>
+import ReplyPlaceholder from './discussion_reply_placeholder.vue';
+import ResolveDiscussionButton from './discussion_resolve_button.vue';
+import ResolveWithIssueButton from './discussion_resolve_with_issue_button.vue';
+import JumpToNextDiscussionButton from './discussion_jump_to_next_button.vue';
+
+export default {
+  name: 'DiscussionActions',
+  components: {
+    ReplyPlaceholder,
+    ResolveDiscussionButton,
+    ResolveWithIssueButton,
+    JumpToNextDiscussionButton,
+  },
+  props: {
+    discussion: {
+      type: Object,
+      required: true,
+    },
+    isResolving: {
+      type: Boolean,
+      required: true,
+    },
+    resolveButtonTitle: {
+      type: String,
+      required: true,
+    },
+    resolveWithIssuePath: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    shouldShowJumpToNextDiscussion: {
+      type: Boolean,
+      required: true,
+    },
+  },
+};
+</script>
+
+<template>
+  <div class="discussion-with-resolve-btn">
+    <reply-placeholder class="qa-discussion-reply" @onClick="$emit('showReplyForm')" />
+    <resolve-discussion-button
+      v-if="discussion.resolvable"
+      :is-resolving="isResolving"
+      :button-title="resolveButtonTitle"
+      @onClick="$emit('resolve')"
+    />
+    <div v-if="discussion.resolvable" class="btn-group discussion-actions ml-sm-2" role="group">
+      <resolve-with-issue-button v-if="resolveWithIssuePath" :url="resolveWithIssuePath" />
+      <jump-to-next-discussion-button
+        v-if="shouldShowJumpToNextDiscussion"
+        @onClick="$emit('jumpToNextDiscussion')"
+      />
+    </div>
+  </div>
+</template>

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

@@ -14,7 +14,6 @@ import { SYSTEM_NOTE } from '../constants';
 import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
 import noteableNote from './noteable_note.vue';
 import noteHeader from './note_header.vue';
-import resolveDiscussionButton from './discussion_resolve_button.vue';
 import toggleRepliesWidget from './toggle_replies_widget.vue';
 import noteSignedOutWidget from './note_signed_out_widget.vue';
 import noteEditedText from './note_edited_text.vue';
@@ -25,10 +24,8 @@ import placeholderSystemNote from '../../vue_shared/components/notes/placeholder
 import noteable from '../mixins/noteable';
 import resolvable from '../mixins/resolvable';
 import discussionNavigation from '../mixins/discussion_navigation';
-import ReplyPlaceholder from './discussion_reply_placeholder.vue';
-import ResolveWithIssueButton from './discussion_resolve_with_issue_button.vue';
-import jumpToNextDiscussionButton from './discussion_jump_to_next_button.vue';
 import eventHub from '../event_hub';
+import DiscussionActions from './discussion_actions.vue';
 
 export default {
   name: 'NoteableDiscussion',
@@ -40,16 +37,13 @@ export default {
     noteSignedOutWidget,
     noteEditedText,
     noteForm,
-    resolveDiscussionButton,
-    jumpToNextDiscussionButton,
     toggleRepliesWidget,
-    ReplyPlaceholder,
     placeholderNote,
     placeholderSystemNote,
-    ResolveWithIssueButton,
     systemNote,
     DraftNote: () => import('ee_component/batch_comments/components/draft_note.vue'),
     TimelineEntryItem,
+    DiscussionActions,
   },
   directives: {
     GlTooltip: GlTooltipDirective,
@@ -470,31 +464,17 @@ Please check your network connection and try again.`;
                 :class="{ 'is-replying': isReplying }"
                 class="discussion-reply-holder"
               >
-                <template v-if="!isReplying && canReply">
-                  <div class="discussion-with-resolve-btn">
-                    <reply-placeholder class="qa-discussion-reply" @onClick="showReplyForm" />
-                    <resolve-discussion-button
-                      v-if="discussion.resolvable"
-                      :is-resolving="isResolving"
-                      :button-title="resolveButtonTitle"
-                      @onClick="resolveHandler"
-                    />
-                    <div
-                      v-if="discussion.resolvable"
-                      class="btn-group discussion-actions ml-sm-2"
-                      role="group"
-                    >
-                      <resolve-with-issue-button
-                        v-if="resolveWithIssuePath"
-                        :url="resolveWithIssuePath"
-                      />
-                      <jump-to-next-discussion-button
-                        v-if="shouldShowJumpToNextDiscussion"
-                        @onClick="jumpToNextDiscussion"
-                      />
-                    </div>
-                  </div>
-                </template>
+                <discussion-actions
+                  v-if="!isReplying && canReply"
+                  :discussion="discussion"
+                  :is-resolving="isResolving"
+                  :resolve-button-title="resolveButtonTitle"
+                  :resolve-with-issue-path="resolveWithIssuePath"
+                  :should-show-jump-to-next-discussion="shouldShowJumpToNextDiscussion"
+                  @showReplyForm="showReplyForm"
+                  @resolve="resolveHandler"
+                  @jumpToNextDiscussion="jumpToNextDiscussion"
+                />
                 <note-form
                   v-if="isReplying"
                   ref="noteForm"

app/assets/stylesheets/components/dashboard_skeleton.scss

@@ -8,10 +8,6 @@
     &-warning {
       background-color: $orange-100;
     }
-
-    &-failed {
-      background-color: $red-100;
-    }
   }
 
   &-body {
@@ -36,10 +32,6 @@
     border-radius: $gl-padding;
     height: $gl-padding-32;
 
-    &-failed {
-      background-color: $red-100;
-    }
-
     &-arrow {
       color: $gray-300;
     }
@@ -56,6 +48,13 @@
     }
   }
 
+  &-header,
+  &-footer {
+    &-failed {
+      background-color: $red-100;
+    }
+  }
+
   &-skeleton-info {
     border-radius: $gl-padding;
     height: $gl-padding;

changelogs/unreleased/58293-extract-discussion-actions.yml

+---
+title: Extract DiscussionActions component from NoteableDiscussion
+merge_request: 27227
+author:
+type: other

doc/user/discussions/index.md

@@ -418,19 +418,23 @@ Custom commit messages will be introduced by
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/53310) in GitLab 11.10.
 
-Reviewers can also suggest changes to
-multiple lines with a single suggestion within Merge Request diff discussions.
+Reviewers can also suggest changes to multiple lines with a single suggestion
+within Merge Request diff discussions by adjusting the range offsets. The
+offsets are relative to the position of the diff discussion, and specify the
+range to be replaced by the suggestion when it is applied.
 
 ![Multi-line suggestion syntax](img/multi-line-suggestion-syntax.png)
 
-In the example above, the suggestion covers three lines above and four lines below the commented diff line.
-It'd change from 3 lines _above_ to 4 lines _below_ the commented Diff line.
+In the example above, the suggestion covers three lines above and four lines
+below the commented line. When applied, it would replace from 3 lines _above_
+to 4 lines _below_ the commented line, with the suggested change.
 
 ![Multi-line suggestion preview](img/multi-line-suggestion-preview.png)
 
 NOTE: **Note:**
-Suggestions covering multiple lines are limited to 100 lines _above_ and 100 lines _below_ 
-the commented diff line, allowing up to 200 changed lines per suggestion.
+Suggestions covering multiple lines are limited to 100 lines _above_ and 100
+lines _below_ the commented diff line, allowing up to 200 changed lines per
+suggestion.
 
 ## Start a discussion by replying to a standard comment
 

doc/user/project/clusters/serverless/index.md

@@ -158,21 +158,6 @@ Follow these steps to deploy a function using the Node.js runtime to your Knativ
        description: "node.js runtime function"
        environment:
          MY_FUNCTION: echo-js
-
-     echo-rb:
-       handler: MyEcho.my_function
-       source: ./echo-rb
-       runtime: https://gitlab.com/gitlab-org/serverless/runtimes/ruby
-       description: "Ruby runtime function"
-       environment:
-         MY_FUNCTION: echo-rb
-
-     echo-docker:
-       handler: echo-docker
-       source: ./echo-docker
-       description: "Dockerfile runtime function"
-       environment:
-         MY_FUNCTION: echo-docker
    ```
 
 Explanation of the fields used above:

qa/qa/page/component/note.rb

@@ -15,7 +15,7 @@ module QA
             element :reply_comment_button
           end
 
-          base.view 'app/assets/javascripts/notes/components/noteable_discussion.vue' do
+          base.view 'app/assets/javascripts/notes/components/discussion_actions.vue' do
             element :discussion_reply
           end
 

spec/frontend/notes/components/discussion_actions_spec.js

+import createStore from '~/notes/stores';
+import { shallowMount, mount, createLocalVue } from '@vue/test-utils';
+import { discussionMock } from '../../../javascripts/notes/mock_data';
+import DiscussionActions from '~/notes/components/discussion_actions.vue';
+import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue';
+import ResolveDiscussionButton from '~/notes/components/discussion_resolve_button.vue';
+import ResolveWithIssueButton from '~/notes/components/discussion_resolve_with_issue_button.vue';
+import JumpToNextDiscussionButton from '~/notes/components/discussion_jump_to_next_button.vue';
+
+describe('DiscussionActions', () => {
+  let wrapper;
+  const createComponentFactory = (shallow = true) => props => {
+    const localVue = createLocalVue();
+    const store = createStore();
+    const mountFn = shallow ? shallowMount : mount;
+
+    wrapper = mountFn(DiscussionActions, {
+      localVue,
+      store,
+      propsData: {
+        discussion: discussionMock,
+        isResolving: false,
+        resolveButtonTitle: 'Resolve discussion',
+        resolveWithIssuePath: '/some/issue/path',
+        shouldShowJumpToNextDiscussion: true,
+        ...props,
+      },
+    });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('rendering', () => {
+    const createComponent = createComponentFactory();
+
+    it('renders reply placeholder, resolve discussion button, resolve with issue button and jump to next discussion button', () => {
+      createComponent();
+      expect(wrapper.find(ReplyPlaceholder).exists()).toBe(true);
+      expect(wrapper.find(ResolveDiscussionButton).exists()).toBe(true);
+      expect(wrapper.find(ResolveWithIssueButton).exists()).toBe(true);
+      expect(wrapper.find(JumpToNextDiscussionButton).exists()).toBe(true);
+    });
+
+    it('only renders reply placholder if disccusion is not resolvable', () => {
+      const discussion = { ...discussionMock };
+      discussion.resolvable = false;
+      createComponent({ discussion });
+
+      expect(wrapper.find(ReplyPlaceholder).exists()).toBe(true);
+      expect(wrapper.find(ResolveDiscussionButton).exists()).toBe(false);
+      expect(wrapper.find(ResolveWithIssueButton).exists()).toBe(false);
+      expect(wrapper.find(JumpToNextDiscussionButton).exists()).toBe(false);
+    });
+
+    it('does not render resolve with issue button if resolveWithIssuePath is falsy', () => {
+      createComponent({ resolveWithIssuePath: '' });
+
+      expect(wrapper.find(ResolveWithIssueButton).exists()).toBe(false);
+    });
+
+    it('does not render jump to next discussion button if shouldShowJumpToNextDiscussion is false', () => {
+      createComponent({ shouldShowJumpToNextDiscussion: false });
+
+      expect(wrapper.find(JumpToNextDiscussionButton).exists()).toBe(false);
+    });
+  });
+
+  describe('events handling', () => {
+    const createComponent = createComponentFactory(false);
+
+    beforeEach(() => {
+      createComponent();
+    });
+
+    it('emits showReplyForm event when clicking on reply placeholder', () => {
+      jest.spyOn(wrapper.vm, '$emit');
+      wrapper
+        .find(ReplyPlaceholder)
+        .find('button')
+        .trigger('click');
+      expect(wrapper.vm.$emit).toHaveBeenCalledWith('showReplyForm');
+    });
+
+    it('emits resolve event when clicking on resolve button', () => {
+      jest.spyOn(wrapper.vm, '$emit');
+      wrapper
+        .find(ResolveDiscussionButton)
+        .find('button')
+        .trigger('click');
+      expect(wrapper.vm.$emit).toHaveBeenCalledWith('resolve');
+    });
+
+    it('emits jumpToNextDiscussion event when clicking on jump to next discussion button', () => {
+      jest.spyOn(wrapper.vm, '$emit');
+      wrapper
+        .find(JumpToNextDiscussionButton)
+        .find('button')
+        .trigger('click');
+      expect(wrapper.vm.$emit).toHaveBeenCalledWith('jumpToNextDiscussion');
+    });
+  });
+});