Merge branch 'ce-to-ee-2018-09-08' into 'master'

CE upstream - 2018-09-08 09:21 UTC

Closes gitlab-runner#2992 and #2934

See merge request gitlab-org/gitlab-ee!7291

.gitlab-ci.yml

-image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.4.4-golang-1.9-git-2.18-chrome-67.0-node-8.x-yarn-1.2-postgresql-9.6-graphicsmagick-1.3.29"
+image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.4.4-golang-1.9-git-2.18-chrome-69.0-node-8.x-yarn-1.2-postgresql-9.6-graphicsmagick-1.3.29"
 
 .dedicated-runner: &dedicated-runner
   retry: 1
@@ -888,7 +888,6 @@ gitlab:assets:compile:
     SETUP_DB: "false"
     SKIP_STORAGE_VALIDATION: "true"
     WEBPACK_REPORT: "true"
-    NO_COMPRESSION: "true"
     # we override the max_old_space_size to prevent OOM errors
     NODE_OPTIONS: --max_old_space_size=3584
   script:
@@ -902,6 +901,7 @@ gitlab:assets:compile:
     expire_in: 31d
     paths:
       - webpack-report/
+      - public/assets/
 
 karma:
   <<: *dedicated-no-docs-and-no-qa-pull-cache-job

.gitlab/CODEOWNERS

@@ -13,3 +13,5 @@ db/ @abrandl @NikolayS
 
 # Feature specific owners
 /ee/lib/gitlab/code_owners/ @reprazent
+/ee/lib/ee/gitlab/auth/ldap/ @dblessing @mkozono
+/lib/gitlab/auth/ldap/ @dblessing @mkozono

Dangerfile

@@ -4,4 +4,5 @@ danger.import_dangerfile(path: 'danger/changelog')
 danger.import_dangerfile(path: 'danger/specs')
 danger.import_dangerfile(path: 'danger/gemfile')
 danger.import_dangerfile(path: 'danger/database')
+danger.import_dangerfile(path: 'danger/documentation')
 danger.import_dangerfile(path: 'danger/frozen_string')

Gemfile.lock

@@ -232,7 +232,7 @@ GEM
     fast_blank (1.0.0)
     fast_gettext (1.6.0)
     ffaker (2.4.0)
-    ffi (1.9.18)
+    ffi (1.9.25)
     flipper (0.13.0)
     flipper-active_record (0.13.0)
       activerecord (>= 3.2, < 6)

Gemfile.rails5.lock

@@ -235,7 +235,7 @@ GEM
     fast_blank (1.0.0)
     fast_gettext (1.6.0)
     ffaker (2.4.0)
-    ffi (1.9.18)
+    ffi (1.9.25)
     flipper (0.13.0)
     flipper-active_record (0.13.0)
       activerecord (>= 3.2, < 6)

app/assets/javascripts/commons/gitlab_ui.js

 import Vue from 'vue';
-import progressBar from '@gitlab-org/gitlab-ui/dist/base/progress_bar';
+import progressBar from '@gitlab-org/gitlab-ui/dist/components/base/progress_bar';
+import modal from '@gitlab-org/gitlab-ui/dist/components/base/modal';
+
+import dModal from '@gitlab-org/gitlab-ui/dist/directives/modal';
 
 Vue.component('gl-progress-bar', progressBar);
+Vue.component('gl-ui-modal', modal);
+
+Vue.directive('gl-modal', dModal);

app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js

-/* eslint-disable object-shorthand, func-names, comma-dangle, no-else-return, quotes */
+/* eslint-disable object-shorthand, func-names, no-else-return */
 /* global CommentsStore */
 /* global ResolveService */
 
@@ -25,44 +25,44 @@ const ResolveDiscussionBtn = Vue.extend({
     };
   },
   computed: {
-    showButton: function () {
+    showButton: function() {
       if (this.discussion) {
         return this.discussion.isResolvable();
       } else {
         return false;
       }
     },
-    isDiscussionResolved: function () {
+    isDiscussionResolved: function() {
       if (this.discussion) {
         return this.discussion.isResolved();
       } else {
         return false;
       }
     },
-    buttonText: function () {
+    buttonText: function() {
       if (this.isDiscussionResolved) {
-        return "Unresolve discussion";
+        return 'Unresolve discussion';
       } else {
-        return "Resolve discussion";
+        return 'Resolve discussion';
       }
     },
-    loading: function () {
+    loading: function() {
       if (this.discussion) {
         return this.discussion.loading;
       } else {
         return false;
       }
-    }
+    },
   },
-  created: function () {
+  created: function() {
     CommentsStore.createDiscussion(this.discussionId, this.canResolve);
 
     this.discussion = CommentsStore.state[this.discussionId];
   },
   methods: {
-    resolve: function () {
+    resolve: function() {
       ResolveService.toggleResolveForDiscussion(this.mergeRequestId, this.discussionId);
-    }
+    },
   },
 });
 

app/assets/javascripts/diff_notes/services/resolve.js

@@ -8,9 +8,7 @@ window.gl = window.gl || {};
 
 class ResolveServiceClass {
   constructor(root) {
-    this.noteResource = Vue.resource(
-      `${root}/notes{/noteId}/resolve?html=true`,
-    );
+    this.noteResource = Vue.resource(`${root}/notes{/noteId}/resolve?html=true`);
     this.discussionResource = Vue.resource(
       `${root}/merge_requests{/mergeRequestId}/discussions{/discussionId}/resolve?html=true`,
     );
@@ -51,10 +49,7 @@ class ResolveServiceClass {
         discussion.updateHeadline(data);
       })
       .catch(
-        () =>
-          new Flash(
-            'An error occurred when trying to resolve a discussion. Please try again.',
-          ),
+        () => new Flash('An error occurred when trying to resolve a discussion. Please try again.'),
       );
   }
 

app/assets/javascripts/diffs/components/app.vue

@@ -59,7 +59,7 @@ export default {
       emailPatchPath: state => state.diffs.emailPatchPath,
     }),
     ...mapGetters('diffs', ['isParallelView']),
-    ...mapGetters(['isNotesFetched']),
+    ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']),
     targetBranch() {
       return {
         branchName: this.targetBranchName,
@@ -112,13 +112,26 @@ export default {
   },
   created() {
     this.adjustView();
+    eventHub.$once('fetchedNotesData', this.setDiscussions);
   },
   methods: {
-    ...mapActions('diffs', ['setBaseConfig', 'fetchDiffFiles', 'startRenderDiffsQueue']),
+    ...mapActions('diffs', [
+      'setBaseConfig',
+      'fetchDiffFiles',
+      'startRenderDiffsQueue',
+      'assignDiscussionsToDiff',
+    ]),
+
     fetchData() {
       this.fetchDiffFiles()
         .then(() => {
-          requestIdleCallback(this.startRenderDiffsQueue, { timeout: 1000 });
+          requestIdleCallback(
+            () => {
+              this.setDiscussions();
+              this.startRenderDiffsQueue();
+            },
+            { timeout: 1000 },
+          );
         })
         .catch(() => {
           createFlash(__('Something went wrong on our end. Please try again!'));
@@ -128,6 +141,16 @@ export default {
         eventHub.$emit('fetchNotesData');
       }
     },
+    setDiscussions() {
+      if (this.isNotesFetched) {
+        requestIdleCallback(
+          () => {
+            this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode);
+          },
+          { timeout: 1000 },
+        );
+      }
+    },
     adjustView() {
       if (this.shouldShow && this.isParallelView) {
         window.mrTabs.expandViewContainer();

app/assets/javascripts/diffs/components/diff_discussions.vue

 <script>
+import { mapActions } from 'vuex';
 import noteableDiscussion from '../../notes/components/noteable_discussion.vue';
 
 export default {
@@ -11,6 +12,14 @@ export default {
       required: true,
     },
   },
+  methods: {
+    ...mapActions('diffs', ['removeDiscussionsFromDiff']),
+    deleteNoteHandler(discussion) {
+      if (discussion.notes.length <= 1) {
+        this.removeDiscussionsFromDiff(discussion);
+      }
+    },
+  },
 };
 </script>
 
@@ -31,6 +40,7 @@ export default {
           :render-diff-file="false"
           :always-expanded="true"
           :discussions-by-diff-order="true"
+          @noteDeleted="deleteNoteHandler"
         />
       </ul>
     </div>

app/assets/javascripts/diffs/components/diff_file.vue

 <script>
-import { mapActions } from 'vuex';
+import { mapActions, mapGetters } from 'vuex';
 import _ from 'underscore';
 import { __, sprintf } from '~/locale';
 import createFlash from '~/flash';
@@ -30,6 +30,7 @@ export default {
     };
   },
   computed: {
+    ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']),
     isCollapsed() {
       return this.file.collapsed || false;
     },
@@ -44,23 +45,23 @@ export default {
       );
     },
     showExpandMessage() {
-      return this.isCollapsed && !this.isLoadingCollapsedDiff && !this.file.tooLarge;
+      return (
+        !this.isCollapsed &&
+        !this.file.highlightedDiffLines &&
+        !this.isLoadingCollapsedDiff &&
+        !this.file.tooLarge &&
+        this.file.text
+      );
     },
     showLoadingIcon() {
       return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed);
     },
   },
   methods: {
-    ...mapActions('diffs', ['loadCollapsedDiff']),
+    ...mapActions('diffs', ['loadCollapsedDiff', 'assignDiscussionsToDiff']),
     handleToggle() {
-      const { collapsed, highlightedDiffLines, parallelDiffLines } = this.file;
-
-      if (
-        collapsed &&
-        !highlightedDiffLines &&
-        parallelDiffLines !== undefined &&
-        !parallelDiffLines.length
-      ) {
+      const { highlightedDiffLines, parallelDiffLines } = this.file;
+      if (!highlightedDiffLines && parallelDiffLines !== undefined && !parallelDiffLines.length) {
         this.handleLoadCollapsedDiff();
       } else {
         this.file.collapsed = !this.file.collapsed;
@@ -76,6 +77,14 @@ export default {
           this.file.collapsed = false;
           this.file.renderIt = true;
         })
+        .then(() => {
+          requestIdleCallback(
+            () => {
+              this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode);
+            },
+            { timeout: 1000 },
+          );
+        })
         .catch(() => {
           this.isLoadingCollapsedDiff = false;
           createFlash(__('Something went wrong on our end. Please try again!'));
@@ -136,11 +145,11 @@ export default {
       :diff-file="file"
     />
     <loading-icon
-      v-else-if="showLoadingIcon"
+      v-if="showLoadingIcon"
       class="diff-content loading"
     />
     <div
-      v-if="showExpandMessage"
+      v-else-if="showExpandMessage"
       class="nothing-here-block diff-collapsed"
     >
       {{ __('This diff is collapsed.') }}

app/assets/javascripts/diffs/components/diff_line_gutter_content.vue

@@ -13,6 +13,10 @@ export default {
     Icon,
   },
   props: {
+    line: {
+      type: Object,
+      required: true,
+    },
     fileHash: {
       type: String,
       required: true,
@@ -21,31 +25,16 @@ export default {
       type: String,
       required: true,
     },
-    lineType: {
-      type: String,
-      required: false,
-      default: '',
-    },
     lineNumber: {
       type: Number,
       required: false,
       default: 0,
     },
-    lineCode: {
-      type: String,
-      required: false,
-      default: '',
-    },
     linePosition: {
       type: String,
       required: false,
       default: '',
     },
-    metaData: {
-      type: Object,
-      required: false,
-      default: () => ({}),
-    },
     showCommentButton: {
       type: Boolean,
       required: false,
@@ -76,11 +65,6 @@ export default {
       required: false,
       default: false,
     },
-    discussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
   },
   computed: {
     ...mapState({
@@ -89,7 +73,7 @@ export default {
     }),
     ...mapGetters(['isLoggedIn']),
     lineHref() {
-      return this.lineCode ? `#${this.lineCode}` : '#';
+      return `#${this.line.lineCode || ''}`;
     },
     shouldShowCommentButton() {
       return (
@@ -103,20 +87,19 @@ export default {
       );
     },
     hasDiscussions() {
-      return this.discussions.length > 0;
+      return this.line.discussions && this.line.discussions.length > 0;
     },
     shouldShowAvatarsOnGutter() {
-      if (!this.lineType && this.linePosition === LINE_POSITION_RIGHT) {
+      if (!this.line.type && this.linePosition === LINE_POSITION_RIGHT) {
         return false;
       }
-
       return this.showCommentButton && this.hasDiscussions;
     },
   },
   methods: {
     ...mapActions('diffs', ['loadMoreLines', 'showCommentForm']),
     handleCommentButton() {
-      this.showCommentForm({ lineCode: this.lineCode });
+      this.showCommentForm({ lineCode: this.line.lineCode });
     },
     handleLoadMoreLines() {
       if (this.isRequesting) {
@@ -125,8 +108,8 @@ export default {
 
       this.isRequesting = true;
       const endpoint = this.contextLinesPath;
-      const oldLineNumber = this.metaData.oldPos || 0;
-      const newLineNumber = this.metaData.newPos || 0;
+      const oldLineNumber = this.line.metaData.oldPos || 0;
+      const newLineNumber = this.line.metaData.newPos || 0;
       const offset = newLineNumber - oldLineNumber;
       const bottom = this.isBottom;
       const { fileHash } = this;
@@ -201,7 +184,7 @@ export default {
       </a>
       <diff-gutter-avatars
         v-if="shouldShowAvatarsOnGutter"
-        :discussions="discussions"
+        :discussions="line.discussions"
       />
     </template>
   </div>

app/assets/javascripts/diffs/components/diff_line_note_form.vue

@@ -6,6 +6,7 @@ import noteForm from '../../notes/components/note_form.vue';
 import { getNoteFormData } from '../store/utils';
 import autosave from '../../notes/mixins/autosave';
 import { DIFF_NOTE_TYPE } from '../constants';
+import { reduceDiscussionsToLineCodes } from '../../notes/stores/utils';
 
 export default {
   components: {
@@ -52,7 +53,7 @@ export default {
     }
   },
   methods: {
-    ...mapActions('diffs', ['cancelCommentForm']),
+    ...mapActions('diffs', ['cancelCommentForm', 'assignDiscussionsToDiff']),
     ...mapActions(['saveNote', 'refetchDiscussionById']),
     handleCancelCommentForm(shouldConfirm, isDirty) {
       if (shouldConfirm && isDirty) {
@@ -88,7 +89,10 @@ export default {
           const endpoint = this.getNotesDataByProp('discussionsPath');
 
           this.refetchDiscussionById({ path: endpoint, discussionId: result.discussion_id })
-            .then(() => {
+            .then(selectedDiscussion => {
+              const lineCodeDiscussions = reduceDiscussionsToLineCodes([selectedDiscussion]);
+              this.assignDiscussionsToDiff(lineCodeDiscussions);
+
               this.handleCancelCommentForm();
             })
             .catch(() => {

app/assets/javascripts/diffs/components/diff_table_cell.vue

@@ -11,8 +11,6 @@ import {
   LINE_HOVER_CLASS_NAME,
   LINE_UNFOLD_CLASS_NAME,
   INLINE_DIFF_VIEW_TYPE,
-  LINE_POSITION_LEFT,
-  LINE_POSITION_RIGHT,
 } from '../constants';
 
 export default {
@@ -67,42 +65,24 @@ export default {
       required: false,
       default: false,
     },
-    discussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
   },
   computed: {
     ...mapGetters(['isLoggedIn']),
-    normalizedLine() {
-      let normalizedLine;
-
-      if (this.diffViewType === INLINE_DIFF_VIEW_TYPE) {
-        normalizedLine = this.line;
-      } else if (this.linePosition === LINE_POSITION_LEFT) {
-        normalizedLine = this.line.left;
-      } else if (this.linePosition === LINE_POSITION_RIGHT) {
-        normalizedLine = this.line.right;
-      }
-
-      return normalizedLine;
-    },
     isMatchLine() {
-      return this.normalizedLine.type === MATCH_LINE_TYPE;
+      return this.line.type === MATCH_LINE_TYPE;
     },
     isContextLine() {
-      return this.normalizedLine.type === CONTEXT_LINE_TYPE;
+      return this.line.type === CONTEXT_LINE_TYPE;
     },
     isMetaLine() {
-      const { type } = this.normalizedLine;
+      const { type } = this.line;
 
       return (
         type === OLD_NO_NEW_LINE_TYPE || type === NEW_NO_NEW_LINE_TYPE || type === EMPTY_CELL_TYPE
       );
     },
     classNameMap() {
-      const { type } = this.normalizedLine;
+      const { type } = this.line;
 
       return {
         [type]: type,
@@ -116,9 +96,9 @@ export default {
       };
     },
     lineNumber() {
-      const { lineType, normalizedLine } = this;
+      const { lineType } = this;
 
-      return lineType === OLD_LINE_TYPE ? normalizedLine.oldLine : normalizedLine.newLine;
+      return lineType === OLD_LINE_TYPE ? this.line.oldLine : this.line.newLine;
     },
   },
 };
@@ -129,20 +109,17 @@ export default {
     :class="classNameMap"
   >
     <diff-line-gutter-content
+      :line="line"
       :file-hash="fileHash"
       :context-lines-path="contextLinesPath"
-      :line-type="normalizedLine.type"
-      :line-code="normalizedLine.lineCode"
       :line-position="linePosition"
       :line-number="lineNumber"
-      :meta-data="normalizedLine.metaData"
       :show-comment-button="showCommentButton"
       :is-hover="isHover"
       :is-bottom="isBottom"
       :is-match-line="isMatchLine"
       :is-context-line="isContentLine"
       :is-meta-line="isMetaLine"
-      :discussions="discussions"
     />
   </td>
 </template>

app/assets/javascripts/diffs/components/inline_diff_comment_row.vue

@@ -21,18 +21,13 @@ export default {
       type: Number,
       required: true,
     },
-    discussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
   },
   computed: {
     ...mapState({
       diffLineCommentForms: state => state.diffs.diffLineCommentForms,
     }),
     className() {
-      return this.discussions.length ? '' : 'js-temp-notes-holder';
+      return this.line.discussions.length ? '' : 'js-temp-notes-holder';
     },
   },
 };
@@ -49,8 +44,8 @@ export default {
     >
       <div class="content">
         <diff-discussions
-          v-if="discussions.length"
-          :discussions="discussions"
+          v-if="line.discussions.length"
+          :discussions="line.discussions"
         />
         <diff-line-note-form
           v-if="diffLineCommentForms[line.lineCode]"

app/assets/javascripts/diffs/components/inline_diff_table_row.vue

@@ -33,11 +33,6 @@ export default {
       required: false,
       default: false,
     },
-    discussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
   },
   data() {
     return {
@@ -94,7 +89,6 @@ export default {
       :is-bottom="isBottom"
       :is-hover="isHover"
       :show-comment-button="true"
-      :discussions="discussions"
       class="diff-line-num old_line"
     />
     <diff-table-cell
@@ -104,7 +98,6 @@ export default {
       :line-type="newLineType"
       :is-bottom="isBottom"
       :is-hover="isHover"
-      :discussions="discussions"
       class="diff-line-num new_line"
     />
     <td

app/assets/javascripts/diffs/components/inline_diff_view.vue

@@ -2,7 +2,6 @@
 import { mapGetters, mapState } from 'vuex';
 import inlineDiffTableRow from './inline_diff_table_row.vue';
 import inlineDiffCommentRow from './inline_diff_comment_row.vue';
-import { trimFirstCharOfLineContent } from '../store/utils';
 
 export default {
   components: {
@@ -20,29 +19,17 @@ export default {
     },
   },
   computed: {
-    ...mapGetters('diffs', [
-      'commitId',
-      'shouldRenderInlineCommentRow',
-      'singleDiscussionByLineCode',
-    ]),
+    ...mapGetters('diffs', ['commitId', 'shouldRenderInlineCommentRow']),
     ...mapState({
       diffLineCommentForms: state => state.diffs.diffLineCommentForms,
     }),
-    normalizedDiffLines() {
-      return this.diffLines.map(line => (line.richText ? trimFirstCharOfLineContent(line) : line));
-    },
     diffLinesLength() {
-      return this.normalizedDiffLines.length;
+      return this.diffLines.length;
     },
     userColorScheme() {
       return window.gon.user_color_scheme;
     },
   },
-  methods: {
-    discussionsList(line) {
-      return line.lineCode !== undefined ? this.singleDiscussionByLineCode(line.lineCode) : [];
-    },
-  },
 };
 </script>
 
@@ -53,7 +40,7 @@ export default {
     class="code diff-wrap-lines js-syntax-highlight text-file js-diff-inline-view">
     <tbody>
       <template
-        v-for="(line, index) in normalizedDiffLines"
+        v-for="(line, index) in diffLines"
       >
         <inline-diff-table-row
           :file-hash="diffFile.fileHash"
@@ -61,7 +48,6 @@ export default {
           :line="line"
           :is-bottom="index + 1 === diffLinesLength"
           :key="line.lineCode"
-          :discussions="discussionsList(line)"
         />
         <inline-diff-comment-row
           v-if="shouldRenderInlineCommentRow(line)"
@@ -69,7 +55,6 @@ export default {
           :line="line"
           :line-index="index"
           :key="index"
-          :discussions="discussionsList(line)"
         />
       </template>
     </tbody>

app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue

@@ -21,51 +21,49 @@ export default {
       type: Number,
       required: true,
     },
-    leftDiscussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
-    rightDiscussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
   },
   computed: {
     ...mapState({
       diffLineCommentForms: state => state.diffs.diffLineCommentForms,
     }),
     leftLineCode() {
-      return this.line.left.lineCode;
+      return this.line.left && this.line.left.lineCode;
     },
     rightLineCode() {
-      return this.line.right.lineCode;
+      return this.line.right && this.line.right.lineCode;
     },
     hasExpandedDiscussionOnLeft() {
-      const discussions = this.leftDiscussions;
-
-      return discussions ? discussions.every(discussion => discussion.expanded) : false;
+      return this.line.left && this.line.left.discussions
+        ? this.line.left.discussions.every(discussion => discussion.expanded)
+        : false;
     },
     hasExpandedDiscussionOnRight() {
-      const discussions = this.rightDiscussions;
-
-      return discussions ? discussions.every(discussion => discussion.expanded) : false;
+      return this.line.right && this.line.right.discussions
+        ? this.line.right.discussions.every(discussion => discussion.expanded)
+        : false;
     },
     hasAnyExpandedDiscussion() {
       return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight;
     },
     shouldRenderDiscussionsOnLeft() {
-      return this.leftDiscussions && this.hasExpandedDiscussionOnLeft;
+      return this.line.left && this.line.left.discussions && this.hasExpandedDiscussionOnLeft;
     },
     shouldRenderDiscussionsOnRight() {
-      return this.rightDiscussions && this.hasExpandedDiscussionOnRight && this.line.right.type;
+      return (
+        this.line.right &&
+        this.line.right.discussions &&
+        this.hasExpandedDiscussionOnRight &&
+        this.line.right.type
+      );
     },
     showRightSideCommentForm() {
-      return this.line.right.type && this.diffLineCommentForms[this.rightLineCode];
+      return (
+        this.line.right && this.line.right.type && this.diffLineCommentForms[this.rightLineCode]
+      );
     },
     className() {
-      return this.leftDiscussions.length > 0 || this.rightDiscussions.length > 0
+      return (this.left && this.line.left.discussions.length > 0) ||
+        (this.right && this.line.right.discussions.length > 0)
         ? ''
         : 'js-temp-notes-holder';
     },
@@ -85,8 +83,8 @@ export default {
         class="content"
       >
         <diff-discussions
-          v-if="leftDiscussions.length"
-          :discussions="leftDiscussions"
+          v-if="line.left.discussions.length"
+          :discussions="line.left.discussions"
         />
       </div>
       <diff-line-note-form
@@ -104,8 +102,8 @@ export default {
         class="content"
       >
         <diff-discussions
-          v-if="rightDiscussions.length"
-          :discussions="rightDiscussions"
+          v-if="line.right.discussions.length"
+          :discussions="line.right.discussions"
         />
       </div>
       <diff-line-note-form

app/assets/javascripts/diffs/components/parallel_diff_table_row.vue

 <script>
 import $ from 'jquery';
-import { mapGetters } from 'vuex';
 import DiffTableCell from './diff_table_cell.vue';
 import {
   NEW_LINE_TYPE,
@@ -10,8 +9,7 @@ import {
   OLD_NO_NEW_LINE_TYPE,
   PARALLEL_DIFF_VIEW_TYPE,
   NEW_NO_NEW_LINE_TYPE,
-  LINE_POSITION_LEFT,
-  LINE_POSITION_RIGHT,
+  EMPTY_CELL_TYPE,
 } from '../constants';
 
 export default {
@@ -36,16 +34,6 @@ export default {
       required: false,
       default: false,
     },
-    leftDiscussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
-    rightDiscussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
   },
   data() {
     return {
@@ -54,29 +42,26 @@ export default {
     };
   },
   computed: {
-    ...mapGetters('diffs', ['isParallelView']),
     isContextLine() {
-      return this.line.left.type === CONTEXT_LINE_TYPE;
+      return this.line.left && this.line.left.type === CONTEXT_LINE_TYPE;
     },
     classNameMap() {
       return {
         [CONTEXT_LINE_CLASS_NAME]: this.isContextLine,
-        [PARALLEL_DIFF_VIEW_TYPE]: this.isParallelView,
+        [PARALLEL_DIFF_VIEW_TYPE]: true,
       };
     },
     parallelViewLeftLineType() {
-      if (this.line.right.type === NEW_NO_NEW_LINE_TYPE) {
+      if (this.line.right && this.line.right.type === NEW_NO_NEW_LINE_TYPE) {
         return OLD_NO_NEW_LINE_TYPE;
       }
 
-      return this.line.left.type;
+      return this.line.left ? this.line.left.type : EMPTY_CELL_TYPE;
     },
   },
   created() {
     this.newLineType = NEW_LINE_TYPE;
     this.oldLineType = OLD_LINE_TYPE;
-    this.linePositionLeft = LINE_POSITION_LEFT;
-    this.linePositionRight = LINE_POSITION_RIGHT;
     this.parallelDiffViewType = PARALLEL_DIFF_VIEW_TYPE;
   },
   methods: {
@@ -116,47 +101,57 @@ export default {
     @mouseover="handleMouseMove"
     @mouseout="handleMouseMove"
   >
-    <diff-table-cell
-      :file-hash="fileHash"
-      :context-lines-path="contextLinesPath"
-      :line="line"
-      :line-type="oldLineType"
-      :line-position="linePositionLeft"
-      :is-bottom="isBottom"
-      :is-hover="isLeftHover"
-      :show-comment-button="true"
-      :diff-view-type="parallelDiffViewType"
-      :discussions="leftDiscussions"
-      class="diff-line-num old_line"
-    />
-    <td
-      :id="line.left.lineCode"
-      :class="parallelViewLeftLineType"
-      class="line_content parallel left-side"
-      @mousedown.native="handleParallelLineMouseDown"
-      v-html="line.left.richText"
-    >
-    </td>
-    <diff-table-cell
-      :file-hash="fileHash"
-      :context-lines-path="contextLinesPath"
-      :line="line"
-      :line-type="newLineType"
-      :line-position="linePositionRight"
-      :is-bottom="isBottom"
-      :is-hover="isRightHover"
-      :show-comment-button="true"
-      :diff-view-type="parallelDiffViewType"
-      :discussions="rightDiscussions"
-      class="diff-line-num new_line"
-    />
-    <td
-      :id="line.right.lineCode"
-      :class="line.right.type"
-      class="line_content parallel right-side"
-      @mousedown.native="handleParallelLineMouseDown"
-      v-html="line.right.richText"
-    >
-    </td>
+    <template v-if="line.left">
+      <diff-table-cell
+        :file-hash="fileHash"
+        :context-lines-path="contextLinesPath"
+        :line="line.left"
+        :line-type="oldLineType"
+        :is-bottom="isBottom"
+        :is-hover="isLeftHover"
+        :show-comment-button="true"
+        :diff-view-type="parallelDiffViewType"
+        line-position="left"
+        class="diff-line-num old_line"
+      />
+      <td
+        :id="line.left.lineCode"
+        :class="parallelViewLeftLineType"
+        class="line_content parallel left-side"
+        @mousedown.native="handleParallelLineMouseDown"
+        v-html="line.left.richText"
+      >
+      </td>
+    </template>
+    <template v-else>
+      <td class="diff-line-num old_line empty-cell"></td>
+      <td class="line_content parallel left-side empty-cell"></td>
+    </template>
+    <template v-if="line.right">
+      <diff-table-cell
+        :file-hash="fileHash"
+        :context-lines-path="contextLinesPath"
+        :line="line.right"
+        :line-type="newLineType"
+        :is-bottom="isBottom"
+        :is-hover="isRightHover"
+        :show-comment-button="true"
+        :diff-view-type="parallelDiffViewType"
+        line-position="right"
+        class="diff-line-num new_line"
+      />
+      <td
+        :id="line.right.lineCode"
+        :class="line.right.type"
+        class="line_content parallel right-side"
+        @mousedown.native="handleParallelLineMouseDown"
+        v-html="line.right.richText"
+      >
+      </td>
+    </template>
+    <template v-else>
+      <td class="diff-line-num old_line empty-cell"></td>
+      <td class="line_content parallel right-side empty-cell"></td>
+    </template>
   </tr>
 </template>

app/assets/javascripts/diffs/components/parallel_diff_view.vue

@@ -2,8 +2,6 @@
 import { mapState, mapGetters } from 'vuex';
 import parallelDiffTableRow from './parallel_diff_table_row.vue';
 import parallelDiffCommentRow from './parallel_diff_comment_row.vue';
-import { EMPTY_CELL_TYPE } from '../constants';
-import { trimFirstCharOfLineContent } from '../store/utils';
 
 export default {
   components: {
@@ -21,46 +19,17 @@ export default {
     },
   },
   computed: {
-    ...mapGetters('diffs', [
-      'commitId',
-      'singleDiscussionByLineCode',
-      'shouldRenderParallelCommentRow',
-    ]),
+    ...mapGetters('diffs', ['commitId', 'shouldRenderParallelCommentRow']),
     ...mapState({
       diffLineCommentForms: state => state.diffs.diffLineCommentForms,
     }),
-    parallelDiffLines() {
-      return this.diffLines.map(line => {
-        const parallelLine = Object.assign({}, line);
-
-        if (line.left) {
-          parallelLine.left = trimFirstCharOfLineContent(line.left);
-        } else {
-          parallelLine.left = { type: EMPTY_CELL_TYPE };
-        }
-
-        if (line.right) {
-          parallelLine.right = trimFirstCharOfLineContent(line.right);
-        } else {
-          parallelLine.right = { type: EMPTY_CELL_TYPE };
-        }
-
-        return parallelLine;
-      });
-    },
     diffLinesLength() {
-      return this.parallelDiffLines.length;
+      return this.diffLines.length;
     },
     userColorScheme() {
       return window.gon.user_color_scheme;
     },
   },
-  methods: {
-    discussionsByLine(line, leftOrRight) {
-      return line[leftOrRight] && line[leftOrRight].lineCode !== undefined ?
-            this.singleDiscussionByLineCode(line[leftOrRight].lineCode) : [];
-    },
-  },
 };
 </script>
 
@@ -73,7 +42,7 @@ export default {
     <table>
       <tbody>
         <template
-          v-for="(line, index) in parallelDiffLines"
+          v-for="(line, index) in diffLines"
         >
           <parallel-diff-table-row
             :file-hash="diffFile.fileHash"
@@ -81,8 +50,6 @@ export default {
             :line="line"
             :is-bottom="index + 1 === diffLinesLength"
             :key="index"
-            :left-discussions="discussionsByLine(line, 'left')"
-            :right-discussions="discussionsByLine(line, 'right')"
           />
           <parallel-diff-comment-row
             v-if="shouldRenderParallelCommentRow(line)"
@@ -90,8 +57,6 @@ export default {
             :line="line"
             :diff-file-hash="diffFile.fileHash"
             :line-index="index"
-            :left-discussions="discussionsByLine(line, 'left')"
-            :right-discussions="discussionsByLine(line, 'right')"
           />
         </template>
       </tbody>

app/assets/javascripts/diffs/store/actions.js

@@ -3,6 +3,7 @@ import axios from '~/lib/utils/axios_utils';
 import Cookies from 'js-cookie';
 import { handleLocationHash, historyPushState } from '~/lib/utils/common_utils';
 import { mergeUrlParams } from '~/lib/utils/url_utility';
+import { getDiffPositionByLineCode } from './utils';
 import * as types from './mutation_types';
 import {
   PARALLEL_DIFF_VIEW_TYPE,
@@ -29,25 +30,53 @@ export const fetchDiffFiles = ({ state, commit }) => {
     .then(handleLocationHash);
 };
 
-export const startRenderDiffsQueue = ({ state, commit }) => {
-  const checkItem = () => {
-    const nextFile = state.diffFiles.find(
-      file => !file.renderIt && (!file.collapsed || !file.text),
-    );
-    if (nextFile) {
-      requestAnimationFrame(() => {
-        commit(types.RENDER_FILE, nextFile);
+// This is adding line discussions to the actual lines in the diff tree
+// once for parallel and once for inline mode
+export const assignDiscussionsToDiff = ({ state, commit }, allLineDiscussions) => {
+  const diffPositionByLineCode = getDiffPositionByLineCode(state.diffFiles);
+
+  Object.values(allLineDiscussions).forEach(discussions => {
+    if (discussions.length > 0) {
+      const { fileHash } = discussions[0];
+      commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, {
+        fileHash,
+        discussions,
+        diffPositionByLineCode,
       });
-      requestIdleCallback(
-        () => {
-          checkItem();
-        },
-        { timeout: 1000 },
-      );
     }
-  };
+  });
+};
+
+export const removeDiscussionsFromDiff = ({ commit }, removeDiscussion) => {
+  const { fileHash, line_code } = removeDiscussion;
+  commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash, lineCode: line_code });
+};
+
+export const startRenderDiffsQueue = ({ state, commit }) => {
+  const checkItem = () =>
+    new Promise(resolve => {
+      const nextFile = state.diffFiles.find(
+        file => !file.renderIt && (!file.collapsed || !file.text),
+      );
+
+      if (nextFile) {
+        requestAnimationFrame(() => {
+          commit(types.RENDER_FILE, nextFile);
+        });
+        requestIdleCallback(
+          () => {
+            checkItem()
+              .then(resolve)
+              .catch(() => {});
+          },
+          { timeout: 1000 },
+        );
+      } else {
+        resolve();
+      }
+    });
 
-  checkItem();
+  return checkItem();
 };
 
 export const setInlineDiffViewType = ({ commit }) => {

app/assets/javascripts/diffs/store/getters.js

@@ -17,7 +17,10 @@ export const commitId = state => (state.commit && state.commit.id ? state.commit
 export const diffHasAllExpandedDiscussions = (state, getters) => diff => {
   const discussions = getters.getDiffFileDiscussions(diff);
 
-  return (discussions.length && discussions.every(discussion => discussion.expanded)) || false;
+  return (
+    (discussions && discussions.length && discussions.every(discussion => discussion.expanded)) ||
+    false
+  );
 };
 
 /**
@@ -28,7 +31,10 @@ export const diffHasAllExpandedDiscussions = (state, getters) => diff => {
 export const diffHasAllCollpasedDiscussions = (state, getters) => diff => {
   const discussions = getters.getDiffFileDiscussions(diff);
 
-  return (discussions.length && discussions.every(discussion => !discussion.expanded)) || false;
+  return (
+    (discussions && discussions.length && discussions.every(discussion => !discussion.expanded)) ||
+    false
+  );
 };
 
 /**
@@ -40,7 +46,9 @@ export const diffHasExpandedDiscussions = (state, getters) => diff => {
   const discussions = getters.getDiffFileDiscussions(diff);
 
   return (
-    (discussions.length && discussions.find(discussion => discussion.expanded) !== undefined) ||
+    (discussions &&
+      discussions.length &&
+      discussions.find(discussion => discussion.expanded) !== undefined) ||
     false
   );
 };
@@ -64,45 +72,38 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) =
       discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash),
   ) || [];
 
-export const singleDiscussionByLineCode = (state, getters, rootState, rootGetters) => lineCode => {
-  if (!lineCode || lineCode === undefined) return [];
-  const discussions = rootGetters.discussionsByLineCode;
-  return discussions[lineCode] || [];
-};
-
-export const shouldRenderParallelCommentRow = (state, getters) => line => {
-  const leftLineCode = line.left.lineCode;
-  const rightLineCode = line.right.lineCode;
-  const leftDiscussions = getters.singleDiscussionByLineCode(leftLineCode);
-  const rightDiscussions = getters.singleDiscussionByLineCode(rightLineCode);
-  const hasDiscussion = leftDiscussions.length || rightDiscussions.length;
+export const shouldRenderParallelCommentRow = state => line => {
+  const hasDiscussion =
+    (line.left && line.left.discussions && line.left.discussions.length) ||
+    (line.right && line.right.discussions && line.right.discussions.length);
 
-  const hasExpandedDiscussionOnLeft = leftDiscussions.length
-    ? leftDiscussions.every(discussion => discussion.expanded)
-    : false;
-  const hasExpandedDiscussionOnRight = rightDiscussions.length
-    ? rightDiscussions.every(discussion => discussion.expanded)
-    : false;
+  const hasExpandedDiscussionOnLeft =
+    line.left && line.left.discussions && line.left.discussions.length
+      ? line.left.discussions.every(discussion => discussion.expanded)
+      : false;
+  const hasExpandedDiscussionOnRight =
+    line.right && line.right.discussions && line.right.discussions.length
+      ? line.right.discussions.every(discussion => discussion.expanded)
+      : false;
 
   if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) {
     return true;
   }
 
-  const hasCommentFormOnLeft = state.diffLineCommentForms[leftLineCode];
-  const hasCommentFormOnRight = state.diffLineCommentForms[rightLineCode];
+  const hasCommentFormOnLeft = line.left && state.diffLineCommentForms[line.left.lineCode];
+  const hasCommentFormOnRight = line.right && state.diffLineCommentForms[line.right.lineCode];
 
   return hasCommentFormOnLeft || hasCommentFormOnRight;
 };
 
-export const shouldRenderInlineCommentRow = (state, getters) => line => {
+export const shouldRenderInlineCommentRow = state => line => {
   if (state.diffLineCommentForms[line.lineCode]) return true;
 
-  const lineDiscussions = getters.singleDiscussionByLineCode(line.lineCode);
-  if (lineDiscussions.length === 0) {
+  if (!line.discussions || line.discussions.length === 0) {
     return false;
   }
 
-  return lineDiscussions.every(discussion => discussion.expanded);
+  return line.discussions.every(discussion => discussion.expanded);
 };
 
 // prevent babel-plugin-rewire from generating an invalid default during karma∂ tests

app/assets/javascripts/diffs/store/mutation_types.js

@@ -9,3 +9,5 @@ export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES';
 export const ADD_COLLAPSED_DIFFS = 'ADD_COLLAPSED_DIFFS';
 export const EXPAND_ALL_FILES = 'EXPAND_ALL_FILES';
 export const RENDER_FILE = 'RENDER_FILE';
+export const SET_LINE_DISCUSSIONS_FOR_FILE = 'SET_LINE_DISCUSSIONS_FOR_FILE';
+export const REMOVE_LINE_DISCUSSIONS_FOR_FILE = 'REMOVE_LINE_DISCUSSIONS_FOR_FILE';

app/assets/javascripts/diffs/store/mutations.js

 import Vue from 'vue';
-import _ from 'underscore';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
-import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } from './utils';
-import { LINES_TO_BE_RENDERED_DIRECTLY, MAX_LINES_TO_BE_RENDERED } from '../constants';
+import {
+  findDiffFile,
+  addLineReferences,
+  removeMatchLine,
+  addContextLines,
+  prepareDiffData,
+  isDiscussionApplicableToLine,
+} from './utils';
 import * as types from './mutation_types';
 
 export default {
@@ -17,38 +22,7 @@ export default {
 
   [types.SET_DIFF_DATA](state, data) {
     const diffData = convertObjectPropsToCamelCase(data, { deep: true });
-    let showingLines = 0;
-    const filesLength = diffData.diffFiles.length;
-    let i;
-    for (i = 0; i < filesLength; i += 1) {
-      const file = diffData.diffFiles[i];
-      if (file.parallelDiffLines) {
-        const linesLength = file.parallelDiffLines.length;
-        let u = 0;
-        for (u = 0; u < linesLength; u += 1) {
-          const line = file.parallelDiffLines[u];
-          if (line.left) delete line.left.text;
-          if (line.right) delete line.right.text;
-        }
-      }
-
-      if (file.highlightedDiffLines) {
-        const linesLength = file.highlightedDiffLines.length;
-        let u;
-        for (u = 0; u < linesLength; u += 1) {
-          const line = file.highlightedDiffLines[u];
-          delete line.text;
-        }
-      }
-
-      if (file.highlightedDiffLines) {
-        showingLines += file.parallelDiffLines.length;
-      }
-      Object.assign(file, {
-        renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY,
-        collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED,
-      });
-    }
+    prepareDiffData(diffData);
 
     Object.assign(state, {
       ...diffData,
@@ -98,12 +72,10 @@ export default {
 
   [types.ADD_COLLAPSED_DIFFS](state, { file, data }) {
     const normalizedData = convertObjectPropsToCamelCase(data, { deep: true });
+    prepareDiffData(normalizedData);
     const [newFileData] = normalizedData.diffFiles.filter(f => f.fileHash === file.fileHash);
-
-    if (newFileData) {
-      const index = _.findIndex(state.diffFiles, f => f.fileHash === file.fileHash);
-      state.diffFiles.splice(index, 1, newFileData);
-    }
+    const selectedFile = state.diffFiles.find(f => f.fileHash === file.fileHash);
+    Object.assign(selectedFile, { ...newFileData });
   },
 
   [types.EXPAND_ALL_FILES](state) {
@@ -112,4 +84,81 @@ export default {
       collapsed: false,
     }));
   },
+
+  [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions, diffPositionByLineCode }) {
+    const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash);
+    const firstDiscussion = discussions[0];
+    const isDiffDiscussion = firstDiscussion.diff_discussion;
+    const hasLineCode = firstDiscussion.line_code;
+    const isResolvable = firstDiscussion.resolvable;
+    const diffPosition = diffPositionByLineCode[firstDiscussion.line_code];
+
+    if (
+      selectedFile &&
+      isDiffDiscussion &&
+      hasLineCode &&
+      isResolvable &&
+      diffPosition &&
+      isDiscussionApplicableToLine(firstDiscussion, diffPosition)
+    ) {
+      const targetLine = selectedFile.parallelDiffLines.find(
+        line =>
+          (line.left && line.left.lineCode === firstDiscussion.line_code) ||
+          (line.right && line.right.lineCode === firstDiscussion.line_code),
+      );
+      if (targetLine) {
+        if (targetLine.left && targetLine.left.lineCode === firstDiscussion.line_code) {
+          Object.assign(targetLine.left, {
+            discussions,
+          });
+        } else {
+          Object.assign(targetLine.right, {
+            discussions,
+          });
+        }
+      }
+
+      if (selectedFile.highlightedDiffLines) {
+        const targetInlineLine = selectedFile.highlightedDiffLines.find(
+          line => line.lineCode === firstDiscussion.line_code,
+        );
+
+        if (targetInlineLine) {
+          Object.assign(targetInlineLine, {
+            discussions,
+          });
+        }
+      }
+    }
+  },
+
+  [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode }) {
+    const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash);
+    if (selectedFile) {
+      const targetLine = selectedFile.parallelDiffLines.find(
+        line =>
+          (line.left && line.left.lineCode === lineCode) ||
+          (line.right && line.right.lineCode === lineCode),
+      );
+      if (targetLine) {
+        const side = targetLine.left && targetLine.left.lineCode === lineCode ? 'left' : 'right';
+
+        Object.assign(targetLine[side], {
+          discussions: [],
+        });
+      }
+
+      if (selectedFile.highlightedDiffLines) {
+        const targetInlineLine = selectedFile.highlightedDiffLines.find(
+          line => line.lineCode === lineCode,
+        );
+
+        if (targetInlineLine) {
+          Object.assign(targetInlineLine, {
+            discussions: [],
+          });
+        }
+      }
+    }
+  },
 };

app/assets/javascripts/diffs/store/utils.js

@@ -8,6 +8,8 @@ import {
   NEW_LINE_TYPE,
   OLD_LINE_TYPE,
   MATCH_LINE_TYPE,
+  LINES_TO_BE_RENDERED_DIRECTLY,
+  MAX_LINES_TO_BE_RENDERED,
 } from '../constants';
 
 export function findDiffFile(files, hash) {
@@ -161,6 +163,11 @@ export function addContextLines(options) {
  * @returns {Object}
  */
 export function trimFirstCharOfLineContent(line = {}) {
+  // eslint-disable-next-line no-param-reassign
+  delete line.text;
+  // eslint-disable-next-line no-param-reassign
+  line.discussions = [];
+
   const parsedLine = Object.assign({}, line);
 
   if (line.richText) {
@@ -174,7 +181,43 @@ export function trimFirstCharOfLineContent(line = {}) {
   return parsedLine;
 }
 
-export function getDiffRefsByLineCode(diffFiles) {
+// This prepares and optimizes the incoming diff data from the server
+// by setting up incremental rendering and removing unneeded data
+export function prepareDiffData(diffData) {
+  const filesLength = diffData.diffFiles.length;
+  let showingLines = 0;
+  for (let i = 0; i < filesLength; i += 1) {
+    const file = diffData.diffFiles[i];
+
+    if (file.parallelDiffLines) {
+      const linesLength = file.parallelDiffLines.length;
+      for (let u = 0; u < linesLength; u += 1) {
+        const line = file.parallelDiffLines[u];
+        if (line.left) {
+          line.left = trimFirstCharOfLineContent(line.left);
+        }
+        if (line.right) {
+          line.right = trimFirstCharOfLineContent(line.right);
+        }
+      }
+    }
+
+    if (file.highlightedDiffLines) {
+      const linesLength = file.highlightedDiffLines.length;
+      for (let u = 0; u < linesLength; u += 1) {
+        trimFirstCharOfLineContent(file.highlightedDiffLines[u]);
+      }
+      showingLines += file.parallelDiffLines.length;
+    }
+
+    Object.assign(file, {
+      renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY,
+      collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED,
+    });
+  }
+}
+
+export function getDiffPositionByLineCode(diffFiles) {
   return diffFiles.reduce((acc, diffFile) => {
     const { baseSha, headSha, startSha } = diffFile.diffRefs;
     const { newPath, oldPath } = diffFile;
@@ -194,3 +237,12 @@ export function getDiffRefsByLineCode(diffFiles) {
     return acc;
   }, {});
 }
+
+// This method will check whether the discussion is still applicable
+// to the diff line in question regarding different versions of the MR
+export function isDiscussionApplicableToLine(discussion, diffPosition) {
+  const originalRefs = convertObjectPropsToCamelCase(discussion.original_position.formatter);
+  const refs = convertObjectPropsToCamelCase(discussion.position.formatter);
+
+  return _.isEqual(refs, diffPosition) || _.isEqual(originalRefs, diffPosition);
+}

app/assets/javascripts/groups/components/group_item.vue

@@ -100,7 +100,7 @@ export default {
       </div>
       <div
         :class="{ 'content-loading': group.isChildrenLoading }"
-        class="avatar-container s24 d-none d-sm-block"
+        class="avatar-container s24 d-none d-sm-flex"
       >
         <a
           :href="group.relativePath"

app/assets/javascripts/notes.js

@@ -154,7 +154,11 @@ export default class Notes {
     this.$wrapperEl.on('click', '.system-note-commit-list-toggler', this.toggleCommitList);
 
     this.$wrapperEl.on('click', '.js-toggle-lazy-diff', this.loadLazyDiff);
-    this.$wrapperEl.on('click', '.js-toggle-lazy-diff-retry-button', this.onClickRetryLazyLoad.bind(this));
+    this.$wrapperEl.on(
+      'click',
+      '.js-toggle-lazy-diff-retry-button',
+      this.onClickRetryLazyLoad.bind(this),
+    );
 
     // fetch notes when tab becomes visible
     this.$wrapperEl.on('visibilitychange', this.visibilityChange);
@@ -252,9 +256,7 @@ export default class Notes {
         discussionNoteForm = $textarea.closest('.js-discussion-note-form');
         if (discussionNoteForm.length) {
           if ($textarea.val() !== '') {
-            if (
-              !window.confirm('Are you sure you want to cancel creating this comment?')
-            ) {
+            if (!window.confirm('Are you sure you want to cancel creating this comment?')) {
               return;
             }
           }
@@ -266,9 +268,7 @@ export default class Notes {
           originalText = $textarea.closest('form').data('originalNote');
           newText = $textarea.val();
           if (originalText !== newText) {
-            if (
-              !window.confirm('Are you sure you want to cancel editing this comment?')
-            ) {
+            if (!window.confirm('Are you sure you want to cancel editing this comment?')) {
               return;
             }
           }
@@ -1316,8 +1316,7 @@ export default class Notes {
 
     $retryButton.prop('disabled', true);
 
-    return this.loadLazyDiff(e)
-    .then(() => {
+    return this.loadLazyDiff(e).then(() => {
       $retryButton.prop('disabled', false);
     });
   }
@@ -1343,18 +1342,18 @@ export default class Notes {
      */
     if (url) {
       return axios
-      .get(url)
-      .then(({ data }) => {
-        // Reset state in case last request returned error
-        $successContainer.removeClass('hidden');
-        $errorContainer.addClass('hidden');
-
-        Notes.renderDiffContent($container, data);
-      })
-      .catch(() => {
-        $successContainer.addClass('hidden');
-        $errorContainer.removeClass('hidden');
-      });
+        .get(url)
+        .then(({ data }) => {
+          // Reset state in case last request returned error
+          $successContainer.removeClass('hidden');
+          $errorContainer.addClass('hidden');
+
+          Notes.renderDiffContent($container, data);
+        })
+        .catch(() => {
+          $successContainer.addClass('hidden');
+          $errorContainer.removeClass('hidden');
+        });
     }
     return Promise.resolve();
   }
@@ -1545,12 +1544,8 @@ export default class Notes {
                <div class="note-header">
                   <div class="note-header-info">
                      <a href="/${_.escape(currentUsername)}">
-                       <span class="d-none d-sm-inline-block">${_.escape(
-                         currentUsername,
-                       )}</span>
-                       <span class="note-headline-light">${_.escape(
-                         currentUsername,
-                       )}</span>
+                       <span class="d-none d-sm-inline-block">${_.escape(currentUsername)}</span>
+                       <span class="note-headline-light">${_.escape(currentUsername)}</span>
                      </a>
                   </div>
                </div>
@@ -1565,9 +1560,7 @@ export default class Notes {
     );
 
     $tempNote.find('.d-none.d-sm-inline-block').text(_.escape(currentUserFullname));
-    $tempNote
-      .find('.note-headline-light')
-      .text(`@${_.escape(currentUsername)}`);
+    $tempNote.find('.note-headline-light').text(`@${_.escape(currentUsername)}`);
 
     return $tempNote;
   }

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

@@ -137,8 +137,10 @@ export default {
       return this.unresolvedDiscussions.length > 1;
     },
     showJumpToNextDiscussion() {
-      return this.hasMultipleUnresolvedDiscussions &&
-        !this.isLastUnresolvedDiscussion(this.discussion.id, this.discussionsByDiffOrder);
+      return (
+        this.hasMultipleUnresolvedDiscussions &&
+        !this.isLastUnresolvedDiscussion(this.discussion.id, this.discussionsByDiffOrder)
+      );
     },
     shouldRenderDiffs() {
       const { diffDiscussion, diffFile } = this.transformedDiscussion;
@@ -256,11 +258,16 @@ Please check your network connection and try again.`;
         });
     },
     jumpToNextDiscussion() {
-      const nextId =
-        this.nextUnresolvedDiscussionId(this.discussion.id, this.discussionsByDiffOrder);
+      const nextId = this.nextUnresolvedDiscussionId(
+        this.discussion.id,
+        this.discussionsByDiffOrder,
+      );
 
       this.jumpToDiscussion(nextId);
     },
+    deleteNoteHandler(note) {
+      this.$emit('noteDeleted', this.discussion, note);
+    },
   },
 };
 </script>
@@ -270,6 +277,7 @@ Please check your network connection and try again.`;
     <div class="timeline-entry-inner">
       <div class="timeline-icon">
         <user-avatar-link
+          v-if="author"
           :link-href="author.path"
           :img-src="author.avatar_url"
           :img-alt="author.name"
@@ -344,6 +352,7 @@ Please check your network connection and try again.`;
                     :is="componentName(note)"
                     :note="componentData(note)"
                     :key="note.id"
+                    @handleDeleteNote="deleteNoteHandler"
                   />
                 </ul>
                 <div

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

@@ -86,6 +86,7 @@ export default {
       // eslint-disable-next-line no-alert
       if (window.confirm('Are you sure you want to delete this comment?')) {
         this.isDeleting = true;
+        this.$emit('handleDeleteNote', this.note);
 
         this.deleteNote(this.note)
           .then(() => {

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

@@ -138,6 +138,7 @@ export default {
         .then(() => {
           this.isLoading = false;
           this.setNotesFetchedState(true);
+          eventHub.$emit('fetchedNotesData');
         })
         .then(() => this.$nextTick())
         .then(() => this.checkLocationHash())

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

@@ -43,14 +43,23 @@ export const fetchDiscussions = ({ commit }, path) =>
       commit(types.SET_INITIAL_DISCUSSIONS, discussions);
     });
 
-export const refetchDiscussionById = ({ commit }, { path, discussionId }) =>
-  service
-    .fetchDiscussions(path)
-    .then(res => res.json())
-    .then(discussions => {
-      const selectedDiscussion = discussions.find(discussion => discussion.id === discussionId);
-      if (selectedDiscussion) commit(types.UPDATE_DISCUSSION, selectedDiscussion);
-    });
+export const refetchDiscussionById = ({ commit, state }, { path, discussionId }) =>
+  new Promise(resolve => {
+    service
+      .fetchDiscussions(path)
+      .then(res => res.json())
+      .then(discussions => {
+        const selectedDiscussion = discussions.find(discussion => discussion.id === discussionId);
+        if (selectedDiscussion) {
+          commit(types.UPDATE_DISCUSSION, selectedDiscussion);
+          // We need to refetch as it is now the transformed one in state
+          const discussion = utils.findNoteObjectById(state.discussions, discussionId);
+
+          resolve(discussion);
+        }
+      })
+      .catch(() => {});
+  });
 
 export const deleteNote = ({ commit }, note) =>
   service.deleteNote(note.path).then(() => {
@@ -152,26 +161,28 @@ export const saveNote = ({ commit, dispatch }, noteData) => {
   const replyId = noteData.data.in_reply_to_discussion_id;
   const methodToDispatch = replyId ? 'replyToDiscussion' : 'createNewNote';
 
-  commit(types.REMOVE_PLACEHOLDER_NOTES); // remove previous placeholders
   $('.notes-form .flash-container').hide(); // hide previous flash notification
+  commit(types.REMOVE_PLACEHOLDER_NOTES); // remove previous placeholders
 
-  if (hasQuickActions) {
-    placeholderText = utils.stripQuickActions(placeholderText);
-  }
+  if (replyId) {
+    if (hasQuickActions) {
+      placeholderText = utils.stripQuickActions(placeholderText);
+    }
 
-  if (placeholderText.length) {
-    commit(types.SHOW_PLACEHOLDER_NOTE, {
-      noteBody: placeholderText,
-      replyId,
-    });
-  }
+    if (placeholderText.length) {
+      commit(types.SHOW_PLACEHOLDER_NOTE, {
+        noteBody: placeholderText,
+        replyId,
+      });
+    }
 
-  if (hasQuickActions) {
-    commit(types.SHOW_PLACEHOLDER_NOTE, {
-      isSystemNote: true,
-      noteBody: utils.getQuickActionText(note),
-      replyId,
-    });
+    if (hasQuickActions) {
+      commit(types.SHOW_PLACEHOLDER_NOTE, {
+        isSystemNote: true,
+        noteBody: utils.getQuickActionText(note),
+        replyId,
+      });
+    }
   }
 
   return dispatch(methodToDispatch, noteData).then(res => {
@@ -211,7 +222,9 @@ export const saveNote = ({ commit, dispatch }, noteData) => {
     if (errors && errors.commands_only) {
       Flash(errors.commands_only, 'notice', noteData.flashContainer);
     }
-    commit(types.REMOVE_PLACEHOLDER_NOTES);
+    if (replyId) {
+      commit(types.REMOVE_PLACEHOLDER_NOTES);
+    }
 
     return res;
   });

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

 import _ from 'underscore';
 import * as constants from '../constants';
+import { reduceDiscussionsToLineCodes } from './utils';
 import { collapseSystemNotes } from './collapse_utils';
 
 export const discussions = state => collapseSystemNotes(state.discussions);
@@ -28,17 +29,8 @@ export const notesById = state =>
     return acc;
   }, {});
 
-export const discussionsByLineCode = state =>
-  state.discussions.reduce((acc, note) => {
-    if (note.diff_discussion && note.line_code && note.resolvable) {
-      // For context about line notes: there might be multiple notes with the same line code
-      const items = acc[note.line_code] || [];
-      items.push(note);
-
-      Object.assign(acc, { [note.line_code]: items });
-    }
-    return acc;
-  }, {});
+export const discussionsStructuredByLineCode = state =>
+  reduceDiscussionsToLineCodes(state.discussions);
 
 export const noteableType = state => {
   const { ISSUE_NOTEABLE_TYPE, MERGE_REQUEST_NOTEABLE_TYPE, EPIC_NOTEABLE_TYPE } = constants;

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

@@ -54,13 +54,12 @@ export default {
 
   [types.EXPAND_DISCUSSION](state, { discussionId }) {
     const discussion = utils.findNoteObjectById(state.discussions, discussionId);
-
-    discussion.expanded = true;
+    Object.assign(discussion, { expanded: true });
   },
 
   [types.COLLAPSE_DISCUSSION](state, { discussionId }) {
     const discussion = utils.findNoteObjectById(state.discussions, discussionId);
-    discussion.expanded = false;
+    Object.assign(discussion, { expanded: false });
   },
 
   [types.REMOVE_PLACEHOLDER_NOTES](state) {
@@ -95,10 +94,15 @@ export default {
   [types.SET_USER_DATA](state, data) {
     Object.assign(state, { userData: data });
   },
+
   [types.SET_INITIAL_DISCUSSIONS](state, discussionsData) {
     const discussions = [];
 
     discussionsData.forEach(discussion => {
+      if (discussion.diff_file) {
+        Object.assign(discussion, { fileHash: discussion.diff_file.file_hash });
+      }
+
       // To support legacy notes, should be very rare case.
       if (discussion.individual_note && discussion.notes.length > 1) {
         discussion.notes.forEach(n => {
@@ -168,8 +172,7 @@ export default {
 
   [types.TOGGLE_DISCUSSION](state, { discussionId }) {
     const discussion = utils.findNoteObjectById(state.discussions, discussionId);
-
-    discussion.expanded = !discussion.expanded;
+    Object.assign(discussion, { expanded: !discussion.expanded });
   },
 
   [types.UPDATE_NOTE](state, note) {
@@ -185,16 +188,12 @@ export default {
 
   [types.UPDATE_DISCUSSION](state, noteData) {
     const note = noteData;
-    let index = 0;
-
-    state.discussions.forEach((n, i) => {
-      if (n.id === note.id) {
-        index = i;
-      }
-    });
-
+    const selectedDiscussion = state.discussions.find(disc => disc.id === note.id);
     note.expanded = true; // override expand flag to prevent collapse
-    state.discussions.splice(index, 1, note);
+    if (note.diff_file) {
+      Object.assign(note, { fileHash: note.diff_file.file_hash });
+    }
+    Object.assign(selectedDiscussion, { ...note });
   },
 
   [types.CLOSE_ISSUE](state) {

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

@@ -2,13 +2,11 @@ import AjaxCache from '~/lib/utils/ajax_cache';
 
 const REGEX_QUICK_ACTIONS = /^\/\w+.*$/gm;
 
-export const findNoteObjectById = (notes, id) =>
-  notes.filter(n => n.id === id)[0];
+export const findNoteObjectById = (notes, id) => notes.filter(n => n.id === id)[0];
 
 export const getQuickActionText = note => {
   let text = 'Applying command';
-  const quickActions =
-    AjaxCache.get(gl.GfmAutoComplete.dataSources.commands) || [];
+  const quickActions = AjaxCache.get(gl.GfmAutoComplete.dataSources.commands) || [];
 
   const executedCommands = quickActions.filter(command => {
     const commandRegex = new RegExp(`/${command.name}`);
@@ -27,7 +25,18 @@ export const getQuickActionText = note => {
   return text;
 };
 
+export const reduceDiscussionsToLineCodes = selectedDiscussions =>
+  selectedDiscussions.reduce((acc, note) => {
+    if (note.diff_discussion && note.line_code && note.resolvable) {
+      // For context about line notes: there might be multiple notes with the same line code
+      const items = acc[note.line_code] || [];
+      items.push(note);
+
+      Object.assign(acc, { [note.line_code]: items });
+    }
+    return acc;
+  }, {});
+
 export const hasQuickActions = note => REGEX_QUICK_ACTIONS.test(note);
 
-export const stripQuickActions = note =>
-  note.replace(REGEX_QUICK_ACTIONS, '').trim();
+export const stripQuickActions = note => note.replace(REGEX_QUICK_ACTIONS, '').trim();

app/assets/javascripts/pages/projects/wikis/components/delete_wiki_modal.vue

@@ -25,6 +25,9 @@ export default {
     },
   },
   computed: {
+    modalId() {
+      return 'delete-wiki-modal';
+    },
     message() {
       return s__('WikiPageConfirmDelete|Are you sure you want to delete this page?');
     },
@@ -47,31 +50,41 @@ export default {
 </script>
 
 <template>
-  <gl-modal
-    id="delete-wiki-modal"
-    :header-title-text="title"
-    :footer-primary-button-text="s__('WikiPageConfirmDelete|Delete page')"
-    footer-primary-button-variant="danger"
-    @submit="onSubmit"
-  >
-    {{ message }}
-    <form
-      ref="form"
-      :action="deleteWikiUrl"
-      method="post"
-      class="js-requires-input"
+  <div class="d-inline-block">
+    <button
+      v-gl-modal="modalId"
+      type="button"
+      class="btn btn-danger"
+    >
+      {{ __('Delete') }}
+    </button>
+    <gl-ui-modal
+      :title="title"
+      :ok-title="s__('WikiPageConfirmDelete|Delete page')"
+      :modal-id="modalId"
+      title-tag="h4"
+      ok-variant="danger"
+      @ok="onSubmit"
     >
-      <input
-        ref="method"
-        type="hidden"
-        name="_method"
-        value="delete"
-      />
-      <input
-        :value="csrfToken"
-        type="hidden"
-        name="authenticity_token"
-      />
-    </form>
-  </gl-modal>
+      {{ message }}
+      <form
+        ref="form"
+        :action="deleteWikiUrl"
+        method="post"
+        class="js-requires-input"
+      >
+        <input
+          ref="method"
+          type="hidden"
+          name="_method"
+          value="delete"
+        />
+        <input
+          :value="csrfToken"
+          type="hidden"
+          name="authenticity_token"
+        />
+      </form>
+    </gl-ui-modal>
+  </div>
 </template>

app/assets/javascripts/pages/projects/wikis/index.js

@@ -14,15 +14,15 @@ document.addEventListener('DOMContentLoaded', () => {
   new ZenMode(); // eslint-disable-line no-new
   new GLForm($('.wiki-form')); // eslint-disable-line no-new
 
-  const deleteWikiButton = document.getElementById('delete-wiki-button');
+  const deleteWikiModalWrapperEl = document.getElementById('delete-wiki-modal-wrapper');
 
-  if (deleteWikiButton) {
+  if (deleteWikiModalWrapperEl) {
     Vue.use(Translate);
 
-    const { deleteWikiUrl, pageTitle } = deleteWikiButton.dataset;
-    const deleteWikiModalEl = document.getElementById('delete-wiki-modal');
-    const deleteModal = new Vue({ // eslint-disable-line
-      el: deleteWikiModalEl,
+    const { deleteWikiUrl, pageTitle } = deleteWikiModalWrapperEl.dataset;
+
+    new Vue({ // eslint-disable-line no-new
+      el: deleteWikiModalWrapperEl,
       data: {
         deleteWikiUrl: '',
       },

app/assets/javascripts/registry/components/collapsible_container.vue

@@ -6,6 +6,7 @@
   import tooltip from '../../vue_shared/directives/tooltip';
   import tableRegistry from './table_registry.vue';
   import { errorMessages, errorMessagesTypes } from '../constants';
+  import { __ } from '../../locale';
 
   export default {
     name: 'CollapsibeContainerRegisty',
@@ -46,7 +47,10 @@
 
       handleDeleteRepository() {
         this.deleteRepo(this.repo)
-          .then(() => this.fetchRepos())
+          .then(() => {
+            Flash(__('This container registry has been scheduled for deletion.'), 'notice');
+            this.fetchRepos();
+          })
           .catch(() => this.showError(errorMessagesTypes.DELETE_REPO));
       },
 

app/assets/stylesheets/framework/avatar.scss

@@ -108,6 +108,7 @@
 
   a {
     width: 100%;
+    height: 100%;
     display: flex;
   }
 

app/controllers/groups/labels_controller.rb

@@ -2,7 +2,6 @@ class Groups::LabelsController < Groups::ApplicationController
   include ToggleSubscriptionAction
 
   before_action :label, only: [:edit, :update, :destroy]
-  before_action :available_labels, only: [:index]
   before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update, :destroy]
   before_action :save_previous_label_path, only: [:edit]
 
@@ -11,10 +10,12 @@ class Groups::LabelsController < Groups::ApplicationController
   def index
     respond_to do |format|
       format.html do
-        @labels = @available_labels.page(params[:page])
+        @labels = @group.labels
+          .optionally_search(params[:search])
+          .page(params[:page])
       end
       format.json do
-        render json: LabelSerializer.new.represent_appearance(@available_labels)
+        render json: LabelSerializer.new.represent_appearance(available_labels)
       end
     end
   end

app/controllers/projects/registry/repositories_controller.rb

@@ -18,14 +18,10 @@ module Projects
       end
 
       def destroy
-        if image.destroy
-          respond_to do |format|
-            format.json { head :no_content }
-          end
-        else
-          respond_to do |format|
-            format.json { head :bad_request }
-          end
+        DeleteContainerRepositoryWorker.perform_async(current_user.id, image.id)
+
+        respond_to do |format|
+          format.json { head :no_content }
         end
       end
 
@@ -41,10 +37,10 @@ module Projects
       # Needed to maintain a backwards compatibility.
       #
       def ensure_root_container_repository!
-        ContainerRegistry::Path.new(@project.full_path).tap do |path|
+        ::ContainerRegistry::Path.new(@project.full_path).tap do |path|
           break if path.has_repository?
 
-          ContainerRepository.build_from_path(path).tap do |repository|
+          ::ContainerRepository.build_from_path(path).tap do |repository|
             repository.save! if repository.has_tags?
           end
         end

app/models/label.rb

@@ -5,6 +5,7 @@ class Label < ActiveRecord::Base
   include Referable
   include Subscribable
   include Gitlab::SQL::Pattern
+  include OptionallySearch
 
   # Represents a "No Label" state used for filtering Issues and Merge
   # Requests that have no label assigned.

app/serializers/discussion_entity.rb

@@ -6,6 +6,7 @@ class DiscussionEntity < Grape::Entity
 
   expose :id, :reply_id
   expose :position, if: -> (d, _) { d.diff_discussion? && !d.legacy_diff_discussion? }
+  expose :original_position, if: -> (d, _) { d.diff_discussion? && !d.legacy_diff_discussion? }
   expose :line_code, if: -> (d, _) { d.diff_discussion? }
   expose :expanded?, as: :expanded
   expose :active?, as: :active, if: -> (d, _) { d.diff_discussion? }

app/services/projects/container_repository/destroy_service.rb

+# frozen_string_literal: true
+
+module Projects
+  module ContainerRepository
+    class DestroyService < BaseService
+      def execute(container_repository)
+        return false unless can?(current_user, :update_container_image, project)
+
+        container_repository.destroy
+      end
+    end
+  end
+end

app/services/projects/destroy_service.rb

@@ -160,7 +160,7 @@ module Projects
     def remove_legacy_registry_tags
       return true unless Gitlab.config.registry.enabled
 
-      ContainerRepository.build_root_repository(project).tap do |repository|
+      ::ContainerRepository.build_root_repository(project).tap do |repository|
         break repository.has_tags? ? repository.delete_tags! : true
       end
     end

app/views/projects/wikis/edit.html.haml

@@ -28,16 +28,8 @@
       = link_to project_wiki_history_path(@project, @page), class: "btn" do
         = s_("Wiki|Page history")
       - if can?(current_user, :admin_wiki, @project)
-        %button.btn.btn-danger{ data: { toggle: 'modal',
-          target: '#delete-wiki-modal',
-          delete_wiki_url: project_wiki_path(@project, @page),
-          page_title: @page.title.capitalize },
-          id: 'delete-wiki-button',
-          type: 'button' }
-          = _('Delete')
+        #delete-wiki-modal-wrapper{ data: { delete_wiki_url: project_wiki_path(@project, @page), page_title: @page.title.capitalize } }
 
 = render 'form', uploads_path: wiki_attachment_upload_url
 
 = render 'sidebar'
-
-#delete-wiki-modal.modal.fade

app/workers/all_queues.yml

@@ -90,6 +90,7 @@
 - authorized_projects
 - background_migration
 - create_gpg_signature
+- delete_container_repository
 - delete_merged_branches
 - delete_user
 - email_receiver

app/workers/delete_container_repository_worker.rb

+# frozen_string_literal: true
+
+class DeleteContainerRepositoryWorker
+  include ApplicationWorker
+  include ExclusiveLeaseGuard
+
+  LEASE_TIMEOUT = 1.hour
+
+  attr_reader :container_repository
+
+  def perform(current_user_id, container_repository_id)
+    current_user = User.find_by(id: current_user_id)
+    @container_repository = ContainerRepository.find_by(id: container_repository_id)
+    project = container_repository&.project
+
+    return unless current_user && container_repository && project
+
+    # If a user accidentally attempts to delete the same container registry in quick succession,
+    # this can lead to orphaned tags.
+    try_obtain_lease do
+      Projects::ContainerRepository::DestroyService.new(project, current_user).execute(container_repository)
+    end
+  end
+
+  # For ExclusiveLeaseGuard concern
+  def lease_key
+    @lease_key ||= "container_repository:delete:#{container_repository.id}"
+  end
+
+  # For ExclusiveLeaseGuard concern
+  def lease_timeout
+    LEASE_TIMEOUT
+  end
+end

changelogs/unreleased/48167-fix-outdated-discussions-new-datastructure.yml

+---
+title: Fix outdated discussions being shown on Merge Request Changes tab
+merge_request: 21543
+author:
+type: fixed

changelogs/unreleased/51180-update-ffi-to-1-9-25.yml

+---
+title: Update ffi to 1.9.25
+merge_request: 21561
+author: Takuya Noguchi
+type: other

changelogs/unreleased/sh-delete-container-registry-async.yml

+---
+title: Delete a container registry asynchronously
+merge_request: 21553
+author:
+type: fixed

config/sidekiq_queues.yml

@@ -46,6 +46,7 @@
   - [project_service, 1]
   - [delete_user, 1]
   - [todos_destroyer, 1]
+  - [delete_container_repository, 1]
   - [delete_merged_branches, 1]
   - [authorized_projects, 1]
   - [expire_build_instance_artifacts, 1]

danger/documentation/Dangerfile

+# frozen_string_literal: true
+
+# All the files/directories that should be reviewed by the Docs team.
+DOCS_FILES = [
+  'doc/'
+].freeze
+
+def docs_paths_requiring_review(files)
+  files.select do |file|
+    DOCS_FILES.any? { |pattern| file.start_with?(pattern) }
+  end
+end
+
+all_files = git.added_files + git.modified_files
+
+docs_paths_to_review = docs_paths_requiring_review(all_files)
+
+unless docs_paths_to_review.empty?
+  message 'This merge request adds or changes files that require a ' \
+    'review from the docs team.'
+
+  markdown(<<~MARKDOWN)
+## Docs Review
+
+The following files require a review from the Documentation team:
+
+* #{docs_paths_to_review.map { |path| "`#{path}`" }.join("\n* ")}
+
+To make sure these changes are reviewed, mention `@gl-docsteam` in a separate
+comment, and explain what needs to be reviewed by the team. Please don't mention
+the team until your changes are ready for review.
+  MARKDOWN
+
+  unless gitlab.mr_labels.include?('Documentation')
+    warn 'This merge request is missing the ~Documentation label.'
+  end
+end

doc/ci/yaml/README.md

@@ -1638,7 +1638,9 @@ There are three possible values: `none`, `normal`, and `recursive`:
     ```
 
 - `recursive` means that all submodules (including submodules of submodules)
-  will be included. It is equivalent to:
+  will be included. This feature needs Git v1.8.1 and later. When using a
+  GitLab Runner with an executor not based on Docker, make sure the Git version
+  meets that requirement. It is equivalent to:
 
     ```
     git submodule sync --recursive

doc/development/documentation/index.md

@@ -257,6 +257,15 @@ choices:
 If your branch name matches any of the above, it will run only the docs
 tests. If it doesn't, the whole test suite will run (including docs).
 
+## Danger bot
+
+GitLab uses [danger bot](https://github.com/danger/danger) for some elements in
+code review. For docs changes in merge requests, the following actions are taken:
+
+1. Whenever a change under `/doc` is made, the bot leaves a comment for the
+   author to mention `@gl-docsteam`, so that the docs can be properly
+   reviewed.
+
 ## Merge requests for GitLab documentation
 
 Before getting started, make sure you read the introductory section

doc/development/documentation/styleguide.md

@@ -13,10 +13,10 @@ Check the GitLab handbook for the [writing styles guidelines](https://about.gitl
 ## Files
 
 - [Directory structure](index.md#location-and-naming-documents): place the docs
-in the correct location
-- [Documentation files](index.md#documentation-files): name the files accordingly
+in the correct location.
+- [Documentation files](index.md#documentation-files): name the files accordingly.
 - [Markdown](../../user/markdown.md): use the GitLab Flavored Markdown in the
-documentation
+documentation.
 
 NOTE: **Note:**
 **Do not** use capital letters, spaces, or special chars in file names,
@@ -30,17 +30,17 @@ a test that will fail if it spots a new `README.md` file.
 
 - Split up long lines (wrap text), this makes it much easier to review and edit. Only
   double line breaks are shown as a full line break in [GitLab markdown][gfm].
-  80-100 characters is a good line length
+  80-100 characters is a good line length.
 - Make sure that the documentation is added in the correct
   [directory](index.md#documentation-directory-structure) and that
-  there's a link to it somewhere useful
-- Do not duplicate information
-- Be brief and clear
-- Unless there's a logical reason not to, add documents in alphabetical order
-- Write in US English
-- Use [single spaces][] instead of double spaces
+  there's a link to it somewhere useful.
+- Do not duplicate information.
+- Be brief and clear.
+- Unless there's a logical reason not to, add documents in alphabetical order.
+- Write in US English.
+- Use [single spaces][] instead of double spaces.
 - Jump a line between different markups (e.g., after every paragraph, header, list, etc)
-- Capitalize "G" and "L" in GitLab
+- Capitalize "G" and "L" in GitLab.
 - Use sentence case for titles, headings, labels, menu items, and buttons.
 - Use title case when referring to [features](https://about.gitlab.com/features/) or
 [products](https://about.gitlab.com/pricing/) (e.g., GitLab Runner, Geo,
@@ -50,10 +50,9 @@ some features are also objects (e.g. "Merge Requests" and "merge requests").
 
 ## Formatting
 
-- Use double asterisks (`**`) to mark a word or text in bold (`**bold**`)
-- Use undescore (`_`) for text in italics (`_italic_`)
-- Jump a line between different markups, for example:
-
+- Use double asterisks (`**`) to mark a word or text in bold (`**bold**`).
+- Use undescore (`_`) for text in italics (`_italic_`).
+- Put an empty line between different markups. For example:
     ```md
     ## Header
 
@@ -69,9 +68,16 @@ For punctuation rules, please refer to the [GitLab UX guide](https://design.gitl
 
 ### Ordered and unordered lists
 
-- Use dashes (`-`) for unordered lists instead of asterisks (`*`)
-- Use the number one (`1`) for ordered lists
-- For punctuation in bullet lists, please refer to the [GitLab UX guide](https://design.gitlab.com/content/punctuation/)
+- Use dashes (`-`) for unordered lists instead of asterisks (`*`).
+- Use the number one (`1`) for ordered lists.
+- Separate list items from explanatory text with a colon (`:`). For example:
+    ```md
+    The list is as follows:
+
+    - First item: This explains the first item.
+    - Second item: This explains the second item.
+    ```
+- For further guidance on punctuation in bullet lists, please refer to the [GitLab UX guide](https://design.gitlab.com/content/punctuation/).
 
 ## Headings
 
@@ -82,7 +88,7 @@ For punctuation rules, please refer to the [GitLab UX guide](https://design.gitl
 - Avoid putting numbers in headings. Numbers shift, hence documentation anchor
   links shift too, which eventually leads to dead links. If you think it is
   compelling to add numbers in headings, make sure to at least discuss it with
-  someone in the Merge Request
+  someone in the Merge Request.
 - [Avoid using symbols and special chars](https://gitlab.com/gitlab-com/gitlab-docs/issues/84)
   in headers. Whenever possible, they should be plain and short text.
 - Avoid adding things that show ephemeral statuses. For example, if a feature is
@@ -92,8 +98,8 @@ For punctuation rules, please refer to the [GitLab UX guide](https://design.gitl
   of the following GitLab members for a review: `@axil` or `@marcia`.
   This is to ensure that no document with wrong heading is going
   live without an audit, thus preventing dead links and redirection issues when
-  corrected
-- Leave exactly one new line after a heading
+  corrected.
+- Leave exactly one new line after a heading.
 
 ## Links
 
@@ -120,11 +126,11 @@ For punctuation rules, please refer to the [GitLab UX guide](https://design.gitl
 
 To indicate the steps of navigation through the UI:
 
-- Use the exact word as shown in the UI, including any capital letters as-is
+- Use the exact word as shown in the UI, including any capital letters as-is.
 - Use bold text for navigation items and the char `>` as separator
-(e.g., `Navigate to your project's **Settings > CI/CD**` )
+(e.g., `Navigate to your project's **Settings > CI/CD**` ).
 - If there are any expandable menus, make sure to mention that the user
-needs to expand the tab to find the settings you're referring to
+needs to expand the tab to find the settings you're referring to.
 
 ## Images
 
@@ -149,12 +155,12 @@ Inside the document:
   `![Proper description what the image is about](img/document_image_title.png)`
 - Always use a proper description for what the image is about. That way, when a
   browser fails to show the image, this text will be used as an alternative
-  description
+  description.
 - If there are consecutive images with little text between them, always add
   three dashes (`---`) between the image and the text to create a horizontal
-  line for better clarity
+  line for better clarity.
 - If a heading is placed right after an image, always add three dashes (`---`)
-  between the image and the heading
+  between the image and the heading.
 
 ## Alert boxes
 
@@ -262,18 +268,18 @@ below.
 When a feature is available in EE-only tiers, add the corresponding tier according to the
 feature availability:
 
-- For GitLab Starter and GitLab.com Bronze: `**[STARTER]**`
-- For GitLab Premium and GitLab.com Silver: `**[PREMIUM]**`
-- For GitLab Ultimate and GitLab.com Gold: `**[ULTIMATE]**`
-- For GitLab Core and GitLab.com Free: `**[CORE]**`
+- For GitLab Starter and GitLab.com Bronze: `**[STARTER]**`.
+- For GitLab Premium and GitLab.com Silver: `**[PREMIUM]**`.
+- For GitLab Ultimate and GitLab.com Gold: `**[ULTIMATE]**`.
+- For GitLab Core and GitLab.com Free: `**[CORE]**`.
 
 To exclude GitLab.com tiers (when the feature is not available in GitLab.com), add the
 keyword "only":
 
-- For GitLab Starter: `**[STARTER ONLY]**`
-- For GitLab Premium: `**[PREMIUM ONLY]**`
-- For GitLab Ultimate: `**[ULTIMATE ONLY]**`
-- For GitLab Core: `**[CORE ONLY]**`
+- For GitLab Starter: `**[STARTER ONLY]**`.
+- For GitLab Premium: `**[PREMIUM ONLY]**`.
+- For GitLab Ultimate: `**[ULTIMATE ONLY]**`.
+- For GitLab Core: `**[CORE ONLY]**`.
 
 The tier should be ideally added to headers, so that the full badge will be displayed.
 But it can be also mentioned from paragraphs, list items, and table cells. For these cases,
@@ -328,8 +334,8 @@ prefer to document it in the CE docs to avoid duplication.
 
 Configuration settings include:
 
-- settings that touch configuration files in `config/`
-- NGINX settings and settings in `lib/support/` in general
+- Settings that touch configuration files in `config/`.
+- NGINX settings and settings in `lib/support/` in general.
 
 When there is a list of steps to perform, usually that entails editing the
 configuration file and reconfiguring/restarting GitLab. In such case, follow

doc/topics/autodevops/index.md

@@ -238,6 +238,12 @@ There is also a feature flag to enable Auto DevOps to a percentage of projects
 which can be enabled from the console with
 `Feature.get(:force_autodevops_on_by_default).enable_percentage_of_actors(10)`.
 
+NOTE: **Enabled by default:**
+Starting with GitLab 11.3, the Auto DevOps pipeline will be enabled by default for all
+projects. If it's not explicitly enabled for the project, Auto DevOps will be automatically
+disabled on the first pipeline failure. Your project will continue to use an alternative
+[CI/CD configuration file](../../ci/yaml/README.md) if one is found.
+
 ### Deployment strategy
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/38542) in GitLab 11.0.

ee/spec/controllers/admin/application_settings_controller_spec.rb

@@ -119,5 +119,33 @@ describe Admin::ApplicationSettingsController do
       expect(response).to redirect_to(admin_application_settings_path)
       expect(ApplicationSetting.current.default_project_creation).to eq(::EE::Gitlab::Access::MAINTAINER_PROJECT_ACCESS)
     end
+
+    it 'updates repository_size_limit' do
+      put :update, application_setting: { repository_size_limit: '100' }
+
+      expect(response).to redirect_to(admin_application_settings_path)
+      expect(response).to set_flash[:notice].to('Application settings saved successfully')
+    end
+
+    it 'does not accept negative repository_size_limit' do
+      put :update, application_setting: { repository_size_limit: '-100' }
+
+      expect(response).to render_template(:show)
+      expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
+    end
+
+    it 'does not accept invalid repository_size_limit' do
+      put :update, application_setting: { repository_size_limit: 'one thousand' }
+
+      expect(response).to render_template(:show)
+      expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
+    end
+
+    it 'does not accept empty repository_size_limit' do
+      put :update, application_setting: { repository_size_limit: '' }
+
+      expect(response).to render_template(:show)
+      expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
+    end
   end
 end

lib/banzai/filter/spaced_link_filter.rb

@@ -10,11 +10,12 @@ module Banzai
     #
     # CommonMark does not allow spaces in the url portion of a link/url.
     # For example, `[example](page slug)` is not valid.
-    # Neither is `![example](test image.jpg)`. However,
+    # Neither is `![example](test image.jpg)`. However, particularly
     # in our wikis, we support (via RedCarpet) this type of link, allowing
     # wiki pages to be easily linked by their title.  This filter adds that functionality.
-    # The intent is for this to only be used in Wikis - in general, we want
-    # to adhere to CommonMark's spec.
+    #
+    # This is a small extension to the CommonMark spec.  If they start allowing
+    # spaces in urls, we could then remove this filter.
     #
     class SpacedLinkFilter < HTML::Pipeline::Filter
       include ActionView::Helpers::TagHelper

lib/banzai/pipeline/gfm_pipeline.rb

@@ -18,6 +18,7 @@ module Banzai
           Filter::MathFilter,
           Filter::ColorFilter,
           Filter::MermaidFilter,
+          Filter::SpacedLinkFilter,
           Filter::VideoLinkFilter,
           Filter::ImageLazyLoadFilter,
           Filter::ImageLinkFilter,

lib/banzai/pipeline/wiki_pipeline.rb

@@ -5,7 +5,6 @@ module Banzai
         @filters ||= begin
           super.insert_after(Filter::TableOfContentsFilter, Filter::GollumTagsFilter)
                .insert_before(Filter::TaskListFilter, Filter::WikiLinkFilter)
-               .insert_before(Filter::VideoLinkFilter, Filter::SpacedLinkFilter)
         end
       end
     end

locale/gitlab.pot

@@ -7443,6 +7443,9 @@ msgstr ""
 msgid "This branch has changed since you started editing. Would you like to create a new branch?"
 msgstr ""
 
+msgid "This container registry has been scheduled for deletion."
+msgstr ""
+
 msgid "This date is after the planned finish date, so this epic won't appear in the roadmap."
 msgstr ""
 

package.json

@@ -19,7 +19,7 @@
   },
   "dependencies": {
     "@gitlab-org/gitlab-svgs": "^1.29.0",
-    "@gitlab-org/gitlab-ui": "1.0.5",
+    "@gitlab-org/gitlab-ui": "^1.1.0",
     "autosize": "^4.0.0",
     "axios": "^0.17.1",
     "babel-core": "^6.26.3",

qa/Gemfile.lock

@@ -32,7 +32,7 @@ GEM
     diff-lcs (1.3)
     domain_name (0.5.20170404)
       unf (>= 0.0.5, < 1.0.0)
-    ffi (1.9.18)
+    ffi (1.9.25)
     http-cookie (1.0.3)
       domain_name (~> 0.5)
     i18n (0.9.1)

spec/controllers/admin/application_settings_controller_spec.rb

@@ -79,68 +79,6 @@ describe Admin::ApplicationSettingsController do
       expect(ApplicationSetting.current.restricted_visibility_levels).to be_empty
     end
 
-    it 'updates repository_size_limit' do
-      put :update, application_setting: { repository_size_limit: '100' }
-
-      expect(response).to redirect_to(admin_application_settings_path)
-      expect(response).to set_flash[:notice].to('Application settings saved successfully')
-    end
-
-    it 'does not accept negative repository_size_limit' do
-      put :update, application_setting: { repository_size_limit: '-100' }
-
-      expect(response).to render_template(:show)
-      expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
-    end
-
-    it 'does not accept invalid repository_size_limit' do
-      put :update, application_setting: { repository_size_limit: 'one thousand' }
-
-      expect(response).to render_template(:show)
-      expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
-    end
-
-    it 'does not accept empty repository_size_limit' do
-      put :update, application_setting: { repository_size_limit: '' }
-
-      expect(response).to render_template(:show)
-      expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
-    end
-  end
-
-  describe 'GET #usage_data with no access' do
-    before do
-      sign_in(user)
-    end
-
-    it 'returns 404' do
-      get :usage_data, format: :html
-
-      expect(response.status).to eq(404)
-    end
-  end
-
-  describe 'GET #usage_data' do
-    before do
-      sign_in(admin)
-    end
-
-    it 'returns HTML data' do
-      get :usage_data, format: :html
-
-      expect(response.body).to start_with('<span')
-      expect(response.status).to eq(200)
-    end
-
-    it 'returns JSON data' do
-      get :usage_data, format: :json
-
-      body = JSON.parse(response.body)
-      expect(body["version"]).to eq(Gitlab::VERSION)
-      expect(body).to include('counts')
-      expect(response.status).to eq(200)
-    end
-
     it 'updates the receive_max_input_size setting' do
       put :update, application_setting: { receive_max_input_size: "1024" }
 

spec/controllers/projects/registry/repositories_controller_spec.rb

@@ -86,9 +86,10 @@ describe Projects::Registry::RepositoriesController do
           stub_container_registry_tags(repository: :any, tags: [])
         end
 
-        it 'deletes a repository' do
-          expect { delete_repository(repository) }.to change { ContainerRepository.all.count }.by(-1)
+        it 'schedules a job to delete a repository' do
+          expect(DeleteContainerRepositoryWorker).to receive(:perform_async).with(user.id, repository.id)
 
+          delete_repository(repository)
           expect(response).to have_gitlab_http_status(:no_content)
         end
       end

spec/features/merge_request/user_posts_diff_notes_spec.rb

@@ -186,11 +186,8 @@ describe 'Merge request > User posts diff notes', :js do
 
     describe 'posting a note' do
       it 'adds as discussion' do
-        expect(page).to have_css('.js-temp-notes-holder', count: 2)
-
         should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'), asset_form_reset: false)
         expect(page).to have_css('.notes_holder .note.note-discussion', count: 1)
-        expect(page).to have_css('.js-temp-notes-holder', count: 1)
         expect(page).to have_button('Reply...')
       end
     end
@@ -267,7 +264,7 @@ describe 'Merge request > User posts diff notes', :js do
   def assert_comment_persistence(line_holder, asset_form_reset:)
     notes_holder_saved = line_holder.find(:xpath, notes_holder_input_xpath)
 
-    expect(notes_holder_saved[:class]).not_to include(notes_holder_input_class)
+    expect(notes_holder_saved[:class]).not_to include('note-edit-form')
     expect(notes_holder_saved).to have_content test_note_comment
 
     assert_form_is_reset if asset_form_reset
@@ -281,6 +278,6 @@ describe 'Merge request > User posts diff notes', :js do
   end
 
   def assert_form_is_reset
-    expect(page).to have_no_css('.js-temp-notes-holder')
+    expect(page).to have_no_css('.note-edit-form')
   end
 end

spec/features/projects/wiki/user_deletes_wiki_page_spec.rb

@@ -13,7 +13,7 @@ describe 'User deletes wiki page', :js do
   it 'deletes a page' do
     click_on('Edit')
     click_on('Delete')
-    find('.js-modal-primary-action').click
+    find('.modal-footer .btn-danger').click
 
     expect(page).to have_content('Page was successfully deleted')
   end

spec/helpers/issuables_helper_spec.rb

@@ -185,7 +185,7 @@ describe IssuablesHelper do
         issuableRef: "##{issue.iid}",
         markdownPreviewPath: "/#{@project.full_path}/preview_markdown",
         markdownDocsPath: '/help/user/markdown',
-        markdownVersion: 11,
+        markdownVersion: CacheMarkdownField::CACHE_COMMONMARK_VERSION,
         issuableTemplates: [],
         projectPath: @project.path,
         projectNamespace: @project.namespace.path,

spec/javascripts/diffs/components/diff_file_spec.js

@@ -51,7 +51,9 @@ describe('DiffFile', () => {
       });
 
       it('should have collapsed text and link', done => {
-        vm.file.collapsed = true;
+        vm.file.renderIt = true;
+        vm.file.collapsed = false;
+        vm.file.highlightedDiffLines = null;
 
         vm.$nextTick(() => {
           expect(vm.$el.innerText).toContain('This diff is collapsed');

spec/javascripts/diffs/components/diff_line_gutter_content_spec.js

@@ -6,61 +6,61 @@ import discussionsMockData from '../mock_data/diff_discussions';
 import diffFileMockData from '../mock_data/diff_file';
 
 describe('DiffLineGutterContent', () => {
-  const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)];
   const getDiffFileMock = () => Object.assign({}, diffFileMockData);
   const createComponent = (options = {}) => {
     const cmp = Vue.extend(DiffLineGutterContent);
     const props = Object.assign({}, options);
+    props.line = {
+      lineCode: 'LC_42',
+      type: 'new',
+      oldLine: null,
+      newLine: 1,
+      discussions: [],
+      text: '+<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
+      richText: '+<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
+      metaData: null,
+    };
     props.fileHash = getDiffFileMock().fileHash;
     props.contextLinesPath = '/context/lines/path';
 
     return createComponentWithStore(cmp, store, props).$mount();
   };
-  const setDiscussions = component => {
-    component.$store.dispatch('setInitialNotes', getDiscussionsMockData());
-  };
-
-  const resetDiscussions = component => {
-    component.$store.dispatch('setInitialNotes', []);
-  };
 
   describe('computed', () => {
     describe('lineHref', () => {
       it('should prepend # to lineCode', () => {
         const lineCode = 'LC_42';
-        const component = createComponent({ lineCode });
+        const component = createComponent();
         expect(component.lineHref).toEqual(`#${lineCode}`);
       });
 
       it('should return # if there is no lineCode', () => {
-        const component = createComponent({ lineCode: null });
+        const component = createComponent();
+        component.line.lineCode = '';
         expect(component.lineHref).toEqual('#');
       });
     });
 
     describe('discussions, hasDiscussions, shouldShowAvatarsOnGutter', () => {
       it('should return empty array when there is no discussion', () => {
-        const component = createComponent({ lineCode: 'LC_42' });
-        expect(component.discussions).toEqual([]);
+        const component = createComponent();
         expect(component.hasDiscussions).toEqual(false);
         expect(component.shouldShowAvatarsOnGutter).toEqual(false);
       });
 
       it('should return discussions for the given lineCode', () => {
-        const { lineCode } = getDiffFileMock().highlightedDiffLines[1];
-        const component = createComponent({
-          lineCode,
+        const cmp = Vue.extend(DiffLineGutterContent);
+        const props = {
+          line: getDiffFileMock().highlightedDiffLines[1],
+          fileHash: getDiffFileMock().fileHash,
           showCommentButton: true,
-          discussions: getDiscussionsMockData(),
-        });
+          contextLinesPath: '/context/lines/path',
+        };
+        props.line.discussions = [Object.assign({}, discussionsMockData)];
+        const component = createComponentWithStore(cmp, store, props).$mount();
 
-        setDiscussions(component);
-
-        expect(component.discussions).toEqual(getDiscussionsMockData());
         expect(component.hasDiscussions).toEqual(true);
         expect(component.shouldShowAvatarsOnGutter).toEqual(true);
-
-        resetDiscussions(component);
       });
     });
   });
@@ -104,9 +104,7 @@ describe('DiffLineGutterContent', () => {
         lineCode: getDiffFileMock().highlightedDiffLines[1].lineCode,
       });
 
-      setDiscussions(component);
       expect(component.$el.querySelector('.diff-comment-avatar-holders')).toBeDefined();
-      resetDiscussions(component);
     });
   });
 });

spec/javascripts/diffs/components/parallel_diff_view_spec.js

@@ -18,11 +18,11 @@ describe('ParallelDiffView', () => {
     }).$mount();
   });
 
-  describe('computed', () => {
-    describe('parallelDiffLines', () => {
+  describe('assigned', () => {
+    describe('diffLines', () => {
       it('should normalize lines for empty cells', () => {
-        expect(component.parallelDiffLines[0].left.type).toEqual(constants.EMPTY_CELL_TYPE);
-        expect(component.parallelDiffLines[1].left.type).toEqual(constants.EMPTY_CELL_TYPE);
+        expect(component.diffLines[0].left.type).toEqual(constants.EMPTY_CELL_TYPE);
+        expect(component.diffLines[1].left.type).toEqual(constants.EMPTY_CELL_TYPE);
       });
     });
   });

spec/javascripts/diffs/mock_data/diff_file.js

@@ -49,6 +49,7 @@ export default {
       type: 'new',
       oldLine: null,
       newLine: 1,
+      discussions: [],
       text: '+<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
       richText: '+<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
       metaData: null,
@@ -58,6 +59,7 @@ export default {
       type: 'new',
       oldLine: null,
       newLine: 2,
+      discussions: [],
       text: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
       richText: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
       metaData: null,
@@ -67,6 +69,7 @@ export default {
       type: null,
       oldLine: 1,
       newLine: 3,
+      discussions: [],
       text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
       richText: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
       metaData: null,
@@ -76,6 +79,7 @@ export default {
       type: null,
       oldLine: 2,
       newLine: 4,
+      discussions: [],
       text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
       richText: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
       metaData: null,
@@ -85,6 +89,7 @@ export default {
       type: null,
       oldLine: 3,
       newLine: 5,
+      discussions: [],
       text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
       richText: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
       metaData: null,
@@ -94,6 +99,7 @@ export default {
       type: 'match',
       oldLine: null,
       newLine: null,
+      discussions: [],
       text: '',
       richText: '',
       metaData: {
@@ -112,6 +118,7 @@ export default {
         type: 'new',
         oldLine: null,
         newLine: 1,
+        discussions: [],
         text: '+<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
         richText: '<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
         metaData: null,
@@ -126,6 +133,7 @@ export default {
         type: 'new',
         oldLine: null,
         newLine: 2,
+        discussions: [],
         text: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
         richText: '<span id="LC2" class="line" lang="plaintext"></span>\n',
         metaData: null,
@@ -137,6 +145,7 @@ export default {
         type: null,
         oldLine: 1,
         newLine: 3,
+        discussions: [],
         text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
         richText: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
         metaData: null,
@@ -146,6 +155,7 @@ export default {
         type: null,
         oldLine: 1,
         newLine: 3,
+        discussions: [],
         text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
         richText: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
         metaData: null,
@@ -157,6 +167,7 @@ export default {
         type: null,
         oldLine: 2,
         newLine: 4,
+        discussions: [],
         text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
         richText: '<span id="LC4" class="line" lang="plaintext"></span>\n',
         metaData: null,
@@ -166,6 +177,7 @@ export default {
         type: null,
         oldLine: 2,
         newLine: 4,
+        discussions: [],
         text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
         richText: '<span id="LC4" class="line" lang="plaintext"></span>\n',
         metaData: null,
@@ -177,6 +189,7 @@ export default {
         type: null,
         oldLine: 3,
         newLine: 5,
+        discussions: [],
         text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
         richText: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
         metaData: null,
@@ -186,6 +199,7 @@ export default {
         type: null,
         oldLine: 3,
         newLine: 5,
+        discussions: [],
         text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
         richText: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
         metaData: null,
@@ -197,6 +211,7 @@ export default {
         type: 'match',
         oldLine: null,
         newLine: null,
+        discussions: [],
         text: '',
         richText: '',
         metaData: {
@@ -209,6 +224,7 @@ export default {
         type: 'match',
         oldLine: null,
         newLine: null,
+        discussions: [],
         text: '',
         richText: '',
         metaData: {

spec/javascripts/diffs/store/actions_spec.js

@@ -7,10 +7,30 @@ import {
 } from '~/diffs/constants';
 import * as actions from '~/diffs/store/actions';
 import * as types from '~/diffs/store/mutation_types';
+import { reduceDiscussionsToLineCodes } from '~/notes/stores/utils';
 import axios from '~/lib/utils/axios_utils';
 import testAction from '../../helpers/vuex_action_helper';
 
 describe('DiffsStoreActions', () => {
+  const originalMethods = {
+    requestAnimationFrame: global.requestAnimationFrame,
+    requestIdleCallback: global.requestIdleCallback,
+  };
+
+  beforeEach(() => {
+    ['requestAnimationFrame', 'requestIdleCallback'].forEach(method => {
+      global[method] = cb => {
+        cb();
+      };
+    });
+  });
+
+  afterEach(() => {
+    ['requestAnimationFrame', 'requestIdleCallback'].forEach(method => {
+      global[method] = originalMethods[method];
+    });
+  });
+
   describe('setBaseConfig', () => {
     it('should set given endpoint and project path', done => {
       const endpoint = '/diffs/set/endpoint';
@@ -53,6 +73,198 @@ describe('DiffsStoreActions', () => {
     });
   });
 
+  describe('assignDiscussionsToDiff', () => {
+    it('should merge discussions into diffs', done => {
+      const state = {
+        diffFiles: [
+          {
+            fileHash: 'ABC',
+            parallelDiffLines: [
+              {
+                left: {
+                  lineCode: 'ABC_1_1',
+                  discussions: [],
+                },
+                right: {
+                  lineCode: 'ABC_1_1',
+                  discussions: [],
+                },
+              },
+            ],
+            highlightedDiffLines: [
+              {
+                lineCode: 'ABC_1_1',
+                discussions: [],
+                oldLine: 5,
+                newLine: null,
+              },
+            ],
+            diffRefs: {
+              baseSha: 'abc',
+              headSha: 'def',
+              startSha: 'ghi',
+            },
+            newPath: 'file1',
+            oldPath: 'file2',
+          },
+        ],
+      };
+
+      const diffPosition = {
+        baseSha: 'abc',
+        headSha: 'def',
+        startSha: 'ghi',
+        newLine: null,
+        newPath: 'file1',
+        oldLine: 5,
+        oldPath: 'file2',
+      };
+
+      const singleDiscussion = {
+        line_code: 'ABC_1_1',
+        diff_discussion: {},
+        diff_file: {
+          file_hash: 'ABC',
+        },
+        fileHash: 'ABC',
+        resolvable: true,
+        position: {
+          formatter: diffPosition,
+        },
+        original_position: {
+          formatter: diffPosition,
+        },
+      };
+
+      const discussions = reduceDiscussionsToLineCodes([singleDiscussion]);
+
+      testAction(
+        actions.assignDiscussionsToDiff,
+        discussions,
+        state,
+        [
+          {
+            type: types.SET_LINE_DISCUSSIONS_FOR_FILE,
+            payload: {
+              fileHash: 'ABC',
+              discussions: [singleDiscussion],
+              diffPositionByLineCode: {
+                ABC_1_1: {
+                  baseSha: 'abc',
+                  headSha: 'def',
+                  startSha: 'ghi',
+                  newLine: null,
+                  newPath: 'file1',
+                  oldLine: 5,
+                  oldPath: 'file2',
+                },
+              },
+            },
+          },
+        ],
+        [],
+        () => {
+          done();
+        },
+      );
+    });
+  });
+
+  describe('removeDiscussionsFromDiff', () => {
+    it('should remove discussions from diffs', done => {
+      const state = {
+        diffFiles: [
+          {
+            fileHash: 'ABC',
+            parallelDiffLines: [
+              {
+                left: {
+                  lineCode: 'ABC_1_1',
+                  discussions: [
+                    {
+                      id: 1,
+                    },
+                  ],
+                },
+                right: {
+                  lineCode: 'ABC_1_1',
+                  discussions: [],
+                },
+              },
+            ],
+            highlightedDiffLines: [
+              {
+                lineCode: 'ABC_1_1',
+                discussions: [],
+              },
+            ],
+          },
+        ],
+      };
+      const singleDiscussion = {
+        fileHash: 'ABC',
+        line_code: 'ABC_1_1',
+      };
+
+      testAction(
+        actions.removeDiscussionsFromDiff,
+        singleDiscussion,
+        state,
+        [
+          {
+            type: types.REMOVE_LINE_DISCUSSIONS_FOR_FILE,
+            payload: {
+              fileHash: 'ABC',
+              lineCode: 'ABC_1_1',
+            },
+          },
+        ],
+        [],
+        () => {
+          done();
+        },
+      );
+    });
+  });
+
+  describe('startRenderDiffsQueue', () => {
+    it('should set all files to RENDER_FILE', done => {
+      const state = {
+        diffFiles: [
+          {
+            id: 1,
+            renderIt: false,
+            collapsed: false,
+          },
+          {
+            id: 2,
+            renderIt: false,
+            collapsed: false,
+          },
+        ],
+      };
+
+      const pseudoCommit = (commitType, file) => {
+        expect(commitType).toBe(types.RENDER_FILE);
+        Object.assign(file, {
+          renderIt: true,
+        });
+      };
+
+      actions
+        .startRenderDiffsQueue({ state, commit: pseudoCommit })
+        .then(() => {
+          expect(state.diffFiles[0].renderIt).toBeTruthy();
+          expect(state.diffFiles[1].renderIt).toBeTruthy();
+
+          done();
+        })
+        .catch(() => {
+          done.fail();
+        });
+    });
+  });
+
   describe('setInlineDiffViewType', () => {
     it('should set diff view type to inline and also set the cookie properly', done => {
       testAction(
@@ -204,7 +416,11 @@ describe('DiffsStoreActions', () => {
 
       actions.toggleFileDiscussions({ getters, dispatch });
 
-      expect(dispatch).toHaveBeenCalledWith('collapseDiscussion', { discussionId: 1 }, { root: true });
+      expect(dispatch).toHaveBeenCalledWith(
+        'collapseDiscussion',
+        { discussionId: 1 },
+        { root: true },
+      );
     });
 
     it('should dispatch expandDiscussion when all discussions are collapsed', () => {
@@ -218,7 +434,11 @@ describe('DiffsStoreActions', () => {
 
       actions.toggleFileDiscussions({ getters, dispatch });
 
-      expect(dispatch).toHaveBeenCalledWith('expandDiscussion', { discussionId: 1 }, { root: true });
+      expect(dispatch).toHaveBeenCalledWith(
+        'expandDiscussion',
+        { discussionId: 1 },
+        { root: true },
+      );
     });
 
     it('should dispatch expandDiscussion when some discussions are collapsed and others are expanded for the collapsed discussion', () => {
@@ -232,7 +452,11 @@ describe('DiffsStoreActions', () => {
 
       actions.toggleFileDiscussions({ getters, dispatch });
 
-      expect(dispatch).toHaveBeenCalledWith('expandDiscussion', { discussionId: 1 }, { root: true });
+      expect(dispatch).toHaveBeenCalledWith(
+        'expandDiscussion',
+        { discussionId: 1 },
+        { root: true },
+      );
     });
   });
 });

spec/javascripts/diffs/store/getters_spec.js

@@ -184,101 +184,73 @@ describe('Diffs Module Getters', () => {
     });
   });
 
-  describe('singleDiscussionByLineCode', () => {
-    it('returns found discussion per line Code', () => {
-      const discussionsMock = {};
-      discussionsMock.ABC = discussionMock;
-
-      expect(
-        getters.singleDiscussionByLineCode(localState, {}, null, {
-          discussionsByLineCode: () => discussionsMock,
-        })('DEF'),
-      ).toEqual([]);
-    });
-
-    it('returns empty array when no discussions match', () => {
-      expect(
-        getters.singleDiscussionByLineCode(localState, {}, null, {
-          discussionsByLineCode: () => {},
-        })('DEF'),
-      ).toEqual([]);
-    });
-  });
-
   describe('shouldRenderParallelCommentRow', () => {
     let line;
 
     beforeEach(() => {
       line = {};
 
+      discussionMock.expanded = true;
+
       line.left = {
         lineCode: 'ABC',
+        discussions: [discussionMock],
       };
 
       line.right = {
         lineCode: 'DEF',
+        discussions: [discussionMock1],
       };
     });
 
     it('returns true when discussion is expanded', () => {
-      discussionMock.expanded = true;
-
-      expect(
-        getters.shouldRenderParallelCommentRow(localState, {
-          singleDiscussionByLineCode: () => [discussionMock],
-        })(line),
-      ).toEqual(true);
+      expect(getters.shouldRenderParallelCommentRow(localState)(line)).toEqual(true);
     });
 
     it('returns false when no discussion was found', () => {
+      line.left.discussions = [];
+      line.right.discussions = [];
+
       localState.diffLineCommentForms.ABC = false;
       localState.diffLineCommentForms.DEF = false;
 
-      expect(
-        getters.shouldRenderParallelCommentRow(localState, {
-          singleDiscussionByLineCode: () => [],
-        })(line),
-      ).toEqual(false);
+      expect(getters.shouldRenderParallelCommentRow(localState)(line)).toEqual(false);
     });
 
     it('returns true when discussionForm was found', () => {
       localState.diffLineCommentForms.ABC = {};
 
-      expect(
-        getters.shouldRenderParallelCommentRow(localState, {
-          singleDiscussionByLineCode: () => [discussionMock],
-        })(line),
-      ).toEqual(true);
+      expect(getters.shouldRenderParallelCommentRow(localState)(line)).toEqual(true);
     });
   });
 
   describe('shouldRenderInlineCommentRow', () => {
+    let line;
+
+    beforeEach(() => {
+      discussionMock.expanded = true;
+
+      line = {
+        lineCode: 'ABC',
+        discussions: [discussionMock],
+      };
+    });
+
     it('returns true when diffLineCommentForms has form', () => {
       localState.diffLineCommentForms.ABC = {};
 
-      expect(
-        getters.shouldRenderInlineCommentRow(localState)({
-          lineCode: 'ABC',
-        }),
-      ).toEqual(true);
+      expect(getters.shouldRenderInlineCommentRow(localState)(line)).toEqual(true);
     });
 
     it('returns false when no line discussions were found', () => {
-      expect(
-        getters.shouldRenderInlineCommentRow(localState, {
-          singleDiscussionByLineCode: () => [],
-        })('DEF'),
-      ).toEqual(false);
+      line.discussions = [];
+      expect(getters.shouldRenderInlineCommentRow(localState)(line)).toEqual(false);
     });
 
     it('returns true if all found discussions are expanded', () => {
       discussionMock.expanded = true;
 
-      expect(
-        getters.shouldRenderInlineCommentRow(localState, {
-          singleDiscussionByLineCode: () => [discussionMock],
-        })('ABC'),
-      ).toEqual(true);
+      expect(getters.shouldRenderInlineCommentRow(localState)(line)).toEqual(true);
     });
   });
 

spec/javascripts/diffs/store/mutations_spec.js

@@ -138,10 +138,9 @@ describe('DiffsStoreMutations', () => {
 
       const fileHash = 123;
       const state = { diffFiles: [{}, { fileHash, existingField: 0 }] };
-      const file = { fileHash };
       const data = { diff_files: [{ file_hash: fileHash, extra_field: 1, existingField: 1 }] };
 
-      mutations[types.ADD_COLLAPSED_DIFFS](state, { file, data });
+      mutations[types.ADD_COLLAPSED_DIFFS](state, { file: state.diffFiles[1], data });
       expect(spy).toHaveBeenCalledWith(data, { deep: true });
 
       expect(state.diffFiles[1].fileHash).toEqual(fileHash);
@@ -149,4 +148,141 @@ describe('DiffsStoreMutations', () => {
       expect(state.diffFiles[1].extraField).toEqual(1);
     });
   });
+
+  describe('SET_LINE_DISCUSSIONS_FOR_FILE', () => {
+    it('should add discussions to the given line', () => {
+      const diffPosition = {
+        baseSha: 'ed13df29948c41ba367caa757ab3ec4892509910',
+        headSha: 'b921914f9a834ac47e6fd9420f78db0f83559130',
+        newLine: null,
+        newPath: '500-lines-4.txt',
+        oldLine: 5,
+        oldPath: '500-lines-4.txt',
+        startSha: 'ed13df29948c41ba367caa757ab3ec4892509910',
+      };
+
+      const state = {
+        diffFiles: [
+          {
+            fileHash: 'ABC',
+            parallelDiffLines: [
+              {
+                left: {
+                  lineCode: 'ABC_1',
+                  discussions: [],
+                },
+                right: {
+                  lineCode: 'ABC_1',
+                  discussions: [],
+                },
+              },
+            ],
+            highlightedDiffLines: [
+              {
+                lineCode: 'ABC_1',
+                discussions: [],
+              },
+            ],
+          },
+        ],
+      };
+      const discussions = [
+        {
+          id: 1,
+          line_code: 'ABC_1',
+          diff_discussion: true,
+          resolvable: true,
+          original_position: {
+            formatter: diffPosition,
+          },
+          position: {
+            formatter: diffPosition,
+          },
+        },
+        {
+          id: 2,
+          line_code: 'ABC_1',
+          diff_discussion: true,
+          resolvable: true,
+          original_position: {
+            formatter: diffPosition,
+          },
+          position: {
+            formatter: diffPosition,
+          },
+        },
+      ];
+
+      const diffPositionByLineCode = {
+        ABC_1: diffPosition,
+      };
+
+      mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, {
+        fileHash: 'ABC',
+        discussions,
+        diffPositionByLineCode,
+      });
+
+      expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(2);
+      expect(state.diffFiles[0].parallelDiffLines[0].left.discussions[1].id).toEqual(2);
+
+      expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(2);
+      expect(state.diffFiles[0].highlightedDiffLines[0].discussions[1].id).toEqual(2);
+    });
+  });
+
+  describe('REMOVE_LINE_DISCUSSIONS', () => {
+    it('should remove the existing discussions on the given line', () => {
+      const state = {
+        diffFiles: [
+          {
+            fileHash: 'ABC',
+            parallelDiffLines: [
+              {
+                left: {
+                  lineCode: 'ABC_1',
+                  discussions: [
+                    {
+                      id: 1,
+                      line_code: 'ABC_1',
+                    },
+                    {
+                      id: 2,
+                      line_code: 'ABC_1',
+                    },
+                  ],
+                },
+                right: {
+                  lineCode: 'ABC_1',
+                  discussions: [],
+                },
+              },
+            ],
+            highlightedDiffLines: [
+              {
+                lineCode: 'ABC_1',
+                discussions: [
+                  {
+                    id: 1,
+                    line_code: 'ABC_1',
+                  },
+                  {
+                    id: 2,
+                    line_code: 'ABC_1',
+                  },
+                ],
+              },
+            ],
+          },
+        ],
+      };
+
+      mutations[types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, {
+        fileHash: 'ABC',
+        lineCode: 'ABC_1',
+      });
+      expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(0);
+      expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(0);
+    });
+  });
 });

spec/javascripts/diffs/store/utils_spec.js

@@ -179,32 +179,117 @@ describe('DiffsStoreUtils', () => {
 
   describe('trimFirstCharOfLineContent', () => {
     it('trims the line when it starts with a space', () => {
-      expect(utils.trimFirstCharOfLineContent({ richText: ' diff' })).toEqual({ richText: 'diff' });
+      expect(utils.trimFirstCharOfLineContent({ richText: ' diff' })).toEqual({
+        discussions: [],
+        richText: 'diff',
+      });
     });
 
     it('trims the line when it starts with a +', () => {
-      expect(utils.trimFirstCharOfLineContent({ richText: '+diff' })).toEqual({ richText: 'diff' });
+      expect(utils.trimFirstCharOfLineContent({ richText: '+diff' })).toEqual({
+        discussions: [],
+        richText: 'diff',
+      });
     });
 
     it('trims the line when it starts with a -', () => {
-      expect(utils.trimFirstCharOfLineContent({ richText: '-diff' })).toEqual({ richText: 'diff' });
+      expect(utils.trimFirstCharOfLineContent({ richText: '-diff' })).toEqual({
+        discussions: [],
+        richText: 'diff',
+      });
     });
 
     it('does not trims the line when it starts with a letter', () => {
-      expect(utils.trimFirstCharOfLineContent({ richText: 'diff' })).toEqual({ richText: 'diff' });
+      expect(utils.trimFirstCharOfLineContent({ richText: 'diff' })).toEqual({
+        discussions: [],
+        richText: 'diff',
+      });
     });
 
     it('does not modify the provided object', () => {
       const lineObj = {
+        discussions: [],
         richText: ' diff',
       };
 
       utils.trimFirstCharOfLineContent(lineObj);
-      expect(lineObj).toEqual({ richText: ' diff' });
+      expect(lineObj).toEqual({ discussions: [], richText: ' diff' });
     });
 
     it('handles a undefined or null parameter', () => {
-      expect(utils.trimFirstCharOfLineContent()).toEqual({});
+      expect(utils.trimFirstCharOfLineContent()).toEqual({ discussions: [] });
+    });
+  });
+
+  describe('prepareDiffData', () => {
+    it('sets the renderIt and collapsed attribute on files', () => {
+      const preparedDiff = { diffFiles: [getDiffFileMock()] };
+      utils.prepareDiffData(preparedDiff);
+
+      const firstParallelDiffLine = preparedDiff.diffFiles[0].parallelDiffLines[2];
+      expect(firstParallelDiffLine.left.discussions.length).toBe(0);
+      expect(firstParallelDiffLine.left).not.toHaveAttr('text');
+      expect(firstParallelDiffLine.right.discussions.length).toBe(0);
+      expect(firstParallelDiffLine.right).not.toHaveAttr('text');
+
+      expect(preparedDiff.diffFiles[0].highlightedDiffLines[0].discussions.length).toBe(0);
+      expect(preparedDiff.diffFiles[0].highlightedDiffLines[0]).not.toHaveAttr('text');
+
+      expect(preparedDiff.diffFiles[0].renderIt).toBeTruthy();
+      expect(preparedDiff.diffFiles[0].collapsed).toBeFalsy();
+    });
+  });
+
+  describe('isDiscussionApplicableToLine', () => {
+    const diffPosition = {
+      baseSha: 'ed13df29948c41ba367caa757ab3ec4892509910',
+      headSha: 'b921914f9a834ac47e6fd9420f78db0f83559130',
+      newLine: null,
+      newPath: '500-lines-4.txt',
+      oldLine: 5,
+      oldPath: '500-lines-4.txt',
+      startSha: 'ed13df29948c41ba367caa757ab3ec4892509910',
+    };
+
+    const wrongDiffPosition = {
+      baseSha: 'wrong',
+      headSha: 'wrong',
+      newLine: null,
+      newPath: '500-lines-4.txt',
+      oldLine: 5,
+      oldPath: '500-lines-4.txt',
+      startSha: 'wrong',
+    };
+
+    const discussions = {
+      upToDateDiscussion1: {
+        original_position: {
+          formatter: diffPosition,
+        },
+        position: {
+          formatter: wrongDiffPosition,
+        },
+      },
+      outDatedDiscussion1: {
+        original_position: {
+          formatter: wrongDiffPosition,
+        },
+        position: {
+          formatter: wrongDiffPosition,
+        },
+      },
+    };
+
+    it('returns true when the discussion is up to date', () => {
+      expect(
+        utils.isDiscussionApplicableToLine(discussions.upToDateDiscussion1, diffPosition),
+      ).toBe(true);
+    });
+
+    it('returns false when the discussion is not up to date', () => {
+      expect(
+        utils.isDiscussionApplicableToLine(discussions.outDatedDiscussion1, diffPosition),
+      ).toBe(false);
     });
   });
 });

spec/javascripts/lazy_loader_spec.js

@@ -2,10 +2,10 @@ import LazyLoader from '~/lazy_loader';
 
 let lazyLoader = null;
 
-describe('LazyLoader', function () {
+describe('LazyLoader', function() {
   preloadFixtures('issues/issue_with_comment.html.raw');
 
-  beforeEach(function () {
+  beforeEach(function() {
     loadFixtures('issues/issue_with_comment.html.raw');
     lazyLoader = new LazyLoader({
       observerNode: 'body',
@@ -13,8 +13,8 @@ describe('LazyLoader', function () {
     // Doing everything that happens normally in onload
     lazyLoader.loadCheck();
   });
-  describe('behavior', function () {
-    it('should copy value from data-src to src for img 1', function (done) {
+  describe('behavior', function() {
+    it('should copy value from data-src to src for img 1', function(done) {
       const img = document.querySelectorAll('img[data-src]')[0];
       const originalDataSrc = img.getAttribute('data-src');
       img.scrollIntoView();
@@ -26,7 +26,7 @@ describe('LazyLoader', function () {
       }, 100);
     });
 
-    it('should lazy load dynamically added data-src images', function (done) {
+    it('should lazy load dynamically added data-src images', function(done) {
       const newImg = document.createElement('img');
       const testPath = '/img/testimg.png';
       newImg.className = 'lazy';
@@ -41,7 +41,7 @@ describe('LazyLoader', function () {
       }, 100);
     });
 
-    it('should not alter normal images', function (done) {
+    it('should not alter normal images', function(done) {
       const newImg = document.createElement('img');
       const testPath = '/img/testimg.png';
       newImg.setAttribute('src', testPath);

spec/javascripts/lib/utils/mock_data.js

@@ -2,4 +2,4 @@ export const faviconDataUrl = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACA
 
 export const overlayDataUrl = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAYAAABzenr0AAAAAXNSR0IArs4c6QAAA85JREFUWAntVllIVGEUPv/9b46O41KplYN7PeRkti8TjQlhCUGh3MmeQugpIsGKAi2soIcIooiohxYKK2daqDAlIpIiWwxtQaJcaHE0d5tMrbn37z9XRqfR0TvVW56Hudf//uec72zfEWBCJjIwkYGJDPzvGSD/KgExN3Oi2Q+2DJgSDYQEMwItVGH1iZGmJw/Si1y+/PwVAMYYib22MYc/8hVQFgKDEfYoId0KYzagAQebsos/ewMZoeB9wdffcTYpQSaCTWHKoqSQaDk7zkIt0+aCUR8BelEHrf3dUNv9AcqbnsHtT5UKB/hTASh0SLYjnjb/CIDRJi0XiFAaJOpCD8zLpdb4NB66b1OfelthX815dtdRRfiti2aAXLvVLiMQ6olGyztGDkSo4JGGXk8/QFdGpYzpHG2GBQTDhtgVhPEaVbbVpvI6GJz22rv4TcAfrYI1x7Rj5MWWAppomKFVVb2302SFzUkZHAbkG+0b1+Gh77yNYjrmqnWTrLBLRxdvBWv8qlFujH/kYjJYyvLkj71t78zAUvzMAMnHhpN4zf9UREJhd8omyssxu1IgazQDwDnHUcNuH6vhPIE1fmuBzHt74Hn7W89jWGtcAjoaIDOFrdcMYJBkgOCoaRF0Lj0oglddDbCj6tRvKjphEpgjkzEQs2YAKsNxMzjn3nKurhzK+Ly7xe28ua8TwgMMcHJZnvvT0BPtEEKM4tDJ+C8GvIIk4ylINIXVZ0EUKJxYuh3mhCeokbudl6TtVc88dfBdLwbyaWB6zQCYQJpBYSrDGQxBQ/ZWRM2B+VNmQnVnHWx7elyNuL2/R336co7KyJR8CL9oLgEuFlREevWUkEl6uGwpVEG4FBm0OEf9N10NMgPlvWYAuNVwsWDKvcUNYsHUWTCZ13ysyFEXe6TO6aC8CUr9IiK+A05TQrc8yjwmxARHeeMAPlfQJw+AQRwu0YhL/GDXi9NwufG+S8dYkuYMqIb4SsWthotlNMOUCOM6r+G9cqXxPmd1dqrBav/o1zJy2l5/NUjJA/VORwYuFnOUaTQcPs9wMqwV++Xv8oADxKAcZ8nLPr8AoGW+xR6HSqYk3GodAz2QNj0V+Gr26dT9ASNH5239Pf0gktVNWZca8ZvfAFBprWS6hSu1pqt++Y0PD+WIwDAhIWQGtzvSHDbcodfFUFB9hg1Gjs5LXqIdFL+acFBl+FddqYwdxsWC3I70OvgfUaA65zhq2O2c8VxYcyIGFTVlXegYtvCXANCQZJMobjVcLMjtSK/IcEgyOOe8Ve5w7ryKDefp2P3+C/5ohv8HZmVLAAAAAElFTkSuQmCC';
 
-export const faviconWithOverlayDataUrl = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAYAAABzenr0AAAGt0lEQVRYR8WWf3DT5R3H35/vN0lDiCztSuiPAEnTFhSOUamIzFGokwMH55g0E845yjbP6+4qIoiHY6JjnHLeRI6h9jgQpQcD3ImH2u00eHBwjGthKLI1TSm26S8oKYVS0vT7/X52z7ckTUhqC/yx55/kks/zeb+ez6/nIWYm/B8XJQB4SGq6lL+CJA47vvRtvWs2D0nNl/Kf0qBZxx6u23arv0QAAIHivK8BynB4ffa7BgDQXJx/ngGnw+uThgTwP5ZnMocoJAxVxZDVZ+0L5n5WF75TkMafjLdJxpSg2E+gqW1X7zk3rbpaifhLiEBgTv4mEFbpBoTyu01DoDj/dQAv9rvjtdnp/k3Yx9rgAMV5QYCsAAwAgg6vL/1OTy/2BYrzzwLIBWACuNHhrXPG+otGoKaw0JA58kqGJtOFfgPS8yWT4sz88nzj7UIIfz+wd0mRdEZPLMnp2V/8R0+JrhLbBYFHJvwWzBUxYgqYNzhG+zfEhm24MIE5ectBtP0W+y0Or29FcoDifHFSRxwAcMrh9c0YrmisXaA4r0V0U8xvopgDDq9PpCQ+Ag0/zbEbNUNbMiG9fTwkDTsKHpJa2t1Zmiw1AqLg+tMZ+R6WVVtnZ2qP6Ib+FIjh05G3lsDrB4xjUIbRDeM+WZLJYZ4B1rKMzKPm/fdyzs9qg6WT225IMnPcuYjxbPZhn57qaA00zc4/QYT7b1b/wAZmDYSLjsN1WcmiM+6jXz7JTCs1aNPASBjrtrCGOXVBLK9ph72772bc0REZcsQlkEVoOxblhaFBH0Bxi6GBWFNC8gpV0XqYSe/hI85R9o1zxr/QaZbdbmuW9oRzljRrzBRkW9JhMaTgYugKzl35DlXNJ/Fp43FImoZnz7T0ln7bLihM0g85N627vkWPgLrbvYyCvAP1+rRIWETA5QsyQlcJYOCbMRasWpALtljwSsFyeJxFYsoNWqdN1y/ildM78Y/WGjxx8TL+ol3oluy8VupKe7cfoNLdCJkdqEUPOmBJ5ksJoae91mBps5lQ6pkIm20MPiz6A3KsmcNukDe/3Ye3zh3A77Q2XqcGjslLz88i/nB8pkpSoL8nAFSTBpUN4qSxS5KB5jOGUOniCebmzFQcevSN2xKP+Fp7ajt21f8TOxU/5i45JZFS6XwcTB9HxZgUnGTRNgk31x5jet+aGU7jWw+UweOcPeyTxxoqrGL25+UwdjehSvnmOVIqcz4C8y8GAABcQwjnYI5NheikhQWT+EZmDh2ev/l7cz4U2cGmYyg78TYqVH87Kbtd1wFY4hsVQAt14zu2RiDaTUZMf/BHWD35STx37wDv94k1dLeh7MRmvDZ1GR5Inxg17dX6MPnjZfh5X6tGSqXrV2B8ACIx98UNGOlV4CxCuA6zqIeq9FQ8c68bhx7ZiIK06CQdVF+Il3y1Hq03gnDfk4Uj8zbH2T51dCPOtlW39Q+iPTl2VSMfwKPiKw8aTuhgpl1Zdqxzj8PphRWwm21xZjv9VcgYkYb52dP132PFbSYr/la0DpNtrrg9a2oqsKfB2zlwG+4nSe1z7QDjaQBi2Eh6J4QRwimYt43LwOsuB2oX7YLVMCLqTAya3xx/EwZJxtYHy3WhyMkHExebXz3zAbbXfdo7AFBRaMAz1Ypa6XoaoPejKRGteZm6D3SlWVdOcOHo/Lfj2u9aXw+WHNmA00G/DiFEO0Jd+meyk0fIf/+vLfik6Xhj4qN0v7i5HCY1bBQPk+ij9GSzNbzYNdH03kMrscARfzvHQgiBocSFTVHVCrW+u+WrpK9iCIgS1rRK93oG/1GkRJVIup8KMNs1Sw/1rUtALD36ZzRca8XeJDmPtRc18vDn5SCJViYHENY3IZTK3JkE7RAYtpdkp3bAaJeOzN+CsSMTX+wqa7ih9sbVSLI2WV3znihAJYXZPThA7M6KQoM2MniyhUxTioxTpKLMadjx8Jqh5k3S//8d9GOh92XWmP/aXLKvfHgA0ZTklL0jj9m6UR6L5+9bjFWTPLcFIWbCY1+8pHb0drWybJ4aWLQrODyAWJndzoyylNyGg0hL+bV7Ll4rKIWB5CFBxMlLj21SL4W6QjDQjwOL9n4tNt0+AADPfo+UqgXPHJLSJrkso7F6ylLMy56OFMmYACIKblvtQext8Iqp0swyLYiI3zEAbs6Ml3cXv/p3Y+ryq5KcnSKb1Jmj75P7X0Rm/UV0tvO86r/WIhORwszvkmHEehH2WMo7ikDUQUWhoaIG+NNc96Os8eMEmklE2Qy2ANTO0OrA+CwFOFBfsq8pWZ7+B25aDBxvPp+QAAAAAElFTkSuQmCC';
+export const faviconWithOverlayDataUrl = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAYAAABzenr0AAAGtElEQVRYR8WXf3CT9R3H35/nSdIQIktrCf0RStI0FYRjVBAccxTq5MDBKUoz4ZyjbPO87q4yBsPDMdExTjlvIsdQexyI0oMBeuKhdjsNHhwcMgpjIlublLIm/UlJKZSSJs/z/e6+T5v0CQ22wB/7/pPck8/383l9fj6fEOec8H88NAjAS1LwknsFSVLU8WXd1rtm85LUeKnwGQKzjj3s33azvsEAAEIlnn8ByHL4/Pa7BgAQLCm8QOBOh88vDQkQeMxjMkcQEYKqYsyJWWPhgs/80TsFafzROJtkNIXFfYI0pfXqPeennjqlxPUNikBoTuEmEF+lCRBV3G0aQiWFrwH8d30AWJubGdiEfZzdGqDEEwbICnADQGGHry7zTr0X94IlnnMACggwAWh0+PxOvb5EBGqmTTNkj7ySxWS62C+g5Usm1Zn95YXG24UQ+r5n75Li6Ux4LBkyc7/4t5YSLSr6Lgg9UvBLcKocMEYKON/gGB3YoA/bcGFCczzLQdieLE9bHL66FakBSjzCU0cSAHDa4at7aLhG9XLBEk8zAVnxZxyIEhBy+PwFgwAafpxvNzK5NZUhrX28JA07Cl6SmtvcOUwm4ZAouHj7ad+jMrN1dqb3iG7oS4EYPh2etQS+XiesC8TQ3ZD3yZJsHuUPgbMcI+ej5v3ncv5PasNlk1p7JJnzJL+I0/O5h+u0VCdqIDi78AQRHuirft3hYJzQPvawPydVdPI+/OnTnNNKBjYVXHRa8rFFGeb4w1he0wZ7d/84IXTEhxzxUsgitB2LPFGwvgGUfLSeZUpEXqEqrIdz0nr4iHOUfeOccb/tNMtutzWHPeWcJc0aMxm5lkxYDGloj1zB+Sv/RXXTSXzaeBwSY3j+bHNv2bdtMYCbpHtRkNFd36xFQN3tXkZhvgP1fdPi5kMEXL4oIXKVAA58M8aCVQs84BYLXi5aDq+zGJTqYr+i4PV2vHxmJ/7WUoOn2i/jz6yhW7JjrdSV8U4fQFV+I2Q4UIsedMCSSlcsgp72WtnSajOhzDsBNtsYfFD8e+Rbs4fdIG98uw9vnj+AX7FWvk4NHZOXXphF/INx2SpJIU2L8L4GDAoMwlP9kWSg6awcKVs83tyUnY5Dj75+W8bjutae3o5d9X/HTiWAuUtOS6RUOR8Hp48TxjgU/AMSeKJ1Ej/tMWXG1sxwGt98sBxe5+xhe64XVLiK2Z9XwNgdRLXyzQsC4ENwelIHAFxDBOdh1qdCdNLCoon8RnY+HZ6/+TtzPhTZweAxlJ94C5VqoI2U3a7rACzJjQqgBd24CGscos1kxPQZ38fqSU/jhQkDvN9lrKG7FeUnNuPVKcvwYOb4hGgvi2HSx8vwRKyJkVLl+hk43gdBAcfADBD1cA4RXIdZ1EN1Zjqem+DGoUc2oigjMUlvaV8YL/1qPVpuhOG+JwdH5m1Okn3m6Eacaz3V2jeI9uTbVYY6AKOSKw8MX0MBg2lXjh3r3Hk4s7ASdrMtSWxnoBpZIzIwP3e69lxv3Gay4q/F6zDJ5kq6s6amEnsafJ0Db8P9JKkx1w5wPJuY36IToojgNMzb8rLwmsuB2kW7YDWMSCgTg+YXx9+AQZKxdUaFZiju+a2Mi8uvnH0f2/2f9g4AVE4z4LlTilrlehag9xIpEam4jO4DXfdaV97nwtH5byW137VYD5Yc2YAz4YAGIYx2RLq0z1Sex8l//fUWfBI83jh4Kd1PEuAwqVGjWEwSS+nJJmt0sWu86d0frMQCR/LbWQ8hDAxlXMgUV69Q67ubv0q5FUNAlHKmVLnXE/gfREpUiaQHqAizXbO0UN98BMTSo39Cw7UW7E2Rc728qJGHP68ASbQyNYCQTkAUzCSwQ+CwvSjnsQPGLOnI/C0YO3Lwxq5yhhtqb1KNpGqT1TXvigJU0jh33xpAf7NymoGNDJ9sJtPkYuNkqTh7KnY8vGaoeZPy93+GA1joe4kzzv/SVLqvYngA/dFgVfnlb8tjtm6Ux+I39y/Gqone24IQM+GxL15UO3q7WrhsnhJatCs8PAC9md3OrPK0goaDyEj7uXsuXi0qg4HkIUGE52XHNqmXIl0RGOiHoUV7xb+v5K14SC39At79Ximdhc8ekjImuiyjsXryUszLnY40yThIhSi4bbUHsbfBJ6ZKE5dpQdz4HQOgf2a8tLvklY+M6cuvSnJummxSZ46+X+7biMzaRnSu84IauNYsE5HCOX+HDCPWi7DrKW8/BTcVZ2UN8Me57kc5448TaCYR5XJwC0BtHMwPjs/SgAP1pfuCqSL8Pxhr/wunLWAOAAAAAElFTkSuQmCC';

spec/lib/banzai/pipeline/gfm_pipeline_spec.rb

@@ -87,4 +87,22 @@ describe Banzai::Pipeline::GfmPipeline do
       end
     end
   end
+
+  describe 'markdown link or image urls having spaces' do
+    let(:project) { create(:project, :public) }
+
+    it 'rewrites links with spaces in url' do
+      markdown = "[Link to Page](page slug)"
+      output = described_class.to_html(markdown, project: project)
+
+      expect(output).to include("href=\"page%20slug\"")
+    end
+
+    it 'rewrites images with spaces in url' do
+      markdown = "![My Image](test image.png)"
+      output = described_class.to_html(markdown, project: project)
+
+      expect(output).to include("src=\"test%20image.png\"")
+    end
+  end
 end

spec/services/projects/container_repository/destroy_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::ContainerRepository::DestroyService do
+  set(:user) { create(:user) }
+  set(:project) { create(:project, :private) }
+
+  subject { described_class.new(project, user) }
+
+  before do
+    stub_container_registry_config(enabled: true)
+  end
+
+  context 'when user does not have access to registry' do
+    let!(:repository) { create(:container_repository, :root, project: project) }
+
+    it 'does not delete a repository' do
+      expect { subject.execute(repository) }.not_to change { ContainerRepository.all.count }
+    end
+  end
+
+  context 'when user has access to registry' do
+    before do
+      project.add_developer(user)
+    end
+
+    context 'when root container repository exists' do
+      let!(:repository) { create(:container_repository, :root, project: project) }
+
+      before do
+        stub_container_registry_tags(repository: :any, tags: [])
+      end
+
+      it 'deletes the repository' do
+        expect { described_class.new(project, user).execute(repository) }.to change { ContainerRepository.all.count }.by(-1)
+      end
+    end
+  end
+end

spec/workers/delete_container_repository_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DeleteContainerRepositoryWorker do
+  let(:registry) { create(:container_repository) }
+  let(:project) { registry.project }
+  let(:user) { project.owner }
+
+  subject { described_class.new }
+
+  describe '#perform' do
+    it 'executes the destroy service' do
+      service = instance_double(Projects::ContainerRepository::DestroyService)
+      expect(service).to receive(:execute)
+      expect(Projects::ContainerRepository::DestroyService).to receive(:new).with(project, user).and_return(service)
+
+      subject.perform(user.id, registry.id)
+    end
+
+    it 'does not raise error when user could not be found' do
+      expect do
+        subject.perform(-1, registry.id)
+      end.not_to raise_error
+    end
+
+    it 'does not raise error when registry could not be found' do
+      expect do
+        subject.perform(user.id, -1)
+      end.not_to raise_error
+    end
+  end
+end

yarn.lock

@@ -82,9 +82,9 @@
   version "1.29.0"
   resolved "https://registry.yarnpkg.com/@gitlab-org/gitlab-svgs/-/gitlab-svgs-1.29.0.tgz#03b65b513f9099bbda6ecf94d673a2952f8c6c70"
 
-"@gitlab-org/gitlab-ui@1.0.5":
-  version "1.0.5"
-  resolved "https://registry.yarnpkg.com/@gitlab-org/gitlab-ui/-/gitlab-ui-1.0.5.tgz#a64b402650494115c8b494a44b72c2d6fbf33fff"
+"@gitlab-org/gitlab-ui@^1.1.0":
+  version "1.1.0"
+  resolved "https://registry.yarnpkg.com/@gitlab-org/gitlab-ui/-/gitlab-ui-1.1.0.tgz#4216b84c142e37653666da6a088384a44c9d5727"
   dependencies:
     "@gitlab-org/gitlab-svgs" "^1.23.0"
     bootstrap-vue "^2.0.0-rc.11"