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

CE upstream - 2018-08-07 12:24 UTC

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

app/assets/javascripts/autosave.js

@@ -53,4 +53,8 @@ export default class Autosave {
 
     return window.localStorage.removeItem(this.key);
   }
+
+  dispose() {
+    this.field.off('input');
+  }
 }

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

@@ -30,6 +30,7 @@ export default {
           :render-header="false"
           :render-diff-file="false"
           :always-expanded="true"
+          :discussions-by-diff-order="true"
         />
       </ul>
     </div>

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

@@ -189,7 +189,6 @@ export default {
       </button>
       <a
         v-if="lineNumber"
-        v-once
         :data-linenumber="lineNumber"
         :href="lineHref"
       >

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

 <script>
-import $ from 'jquery';
 import { mapState, mapGetters, mapActions } from 'vuex';
 import createFlash from '~/flash';
 import { s__ } from '~/locale';
 import noteForm from '../../notes/components/note_form.vue';
 import { getNoteFormData } from '../store/utils';
-import Autosave from '../../autosave';
-import { DIFF_NOTE_TYPE, NOTE_TYPE } from '../constants';
+import autosave from '../../notes/mixins/autosave';
+import { DIFF_NOTE_TYPE } from '../constants';
 
 export default {
   components: {
     noteForm,
   },
+  mixins: [autosave],
   props: {
     diffFileHash: {
       type: String,
@@ -41,28 +41,35 @@ export default {
   },
   mounted() {
     if (this.isLoggedIn) {
-      const noteableData = this.getNoteableData;
       const keys = [
-        NOTE_TYPE,
-        this.noteableType,
-        noteableData.id,
-        noteableData.diff_head_sha,
+        this.noteableData.diff_head_sha,
         DIFF_NOTE_TYPE,
-        noteableData.source_project_id,
+        this.noteableData.source_project_id,
         this.line.lineCode,
       ];
 
-      this.autosave = new Autosave($(this.$refs.noteForm.$refs.textarea), keys);
+      this.initAutoSave(this.noteableData, keys);
     }
   },
   methods: {
     ...mapActions('diffs', ['cancelCommentForm']),
     ...mapActions(['saveNote', 'refetchDiscussionById']),
-    handleCancelCommentForm() {
-      this.autosave.reset();
+    handleCancelCommentForm(shouldConfirm, isDirty) {
+      if (shouldConfirm && isDirty) {
+        const msg = s__('Notes|Are you sure you want to cancel creating this comment?');
+
+        // eslint-disable-next-line no-alert
+        if (!window.confirm(msg)) {
+          return;
+        }
+      }
+
       this.cancelCommentForm({
         lineCode: this.line.lineCode,
       });
+      this.$nextTick(() => {
+        this.resetAutoSave();
+      });
     },
     handleSaveNote(note) {
       const selectedDiffFile = this.getDiffFileByHash(this.diffFileHash);

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

@@ -101,7 +101,6 @@ export default {
       class="diff-line-num new_line"
     />
     <td
-      v-once
       :class="line.type"
       class="line_content"
       v-html="line.richText"

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

@@ -119,7 +119,6 @@ export default {
       class="diff-line-num old_line"
     />
     <td
-      v-once
       :id="line.left.lineCode"
       :class="parallelViewLeftLineType"
       class="line_content parallel left-side"
@@ -140,7 +139,6 @@ export default {
       class="diff-line-num new_line"
     />
     <td
-      v-once
       :id="line.right.lineCode"
       :class="line.right.type"
       class="line_content parallel right-side"

app/assets/javascripts/lazy_loader.js

 import _ from 'underscore';
 
-export const placeholderImage = 'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==';
+export const placeholderImage =
+  'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==';
 const SCROLL_THRESHOLD = 300;
 
 export default class LazyLoader {
@@ -48,7 +49,7 @@ export default class LazyLoader {
     const visHeight = scrollTop + window.innerHeight + SCROLL_THRESHOLD;
 
     // Loading Images which are in the current viewport or close to them
-    this.lazyImages = this.lazyImages.filter((selectedImage) => {
+    this.lazyImages = this.lazyImages.filter(selectedImage => {
       if (selectedImage.getAttribute('data-src')) {
         const imgBoundRect = selectedImage.getBoundingClientRect();
         const imgTop = scrollTop + imgBoundRect.top;
@@ -66,7 +67,18 @@ export default class LazyLoader {
   }
   static loadImage(img) {
     if (img.getAttribute('data-src')) {
-      img.setAttribute('src', img.getAttribute('data-src'));
+      let imgUrl = img.getAttribute('data-src');
+      // Only adding width + height for avatars for now
+      if (imgUrl.indexOf('/avatar/') > -1 && imgUrl.indexOf('?') === -1) {
+        let targetWidth = null;
+        if (img.getAttribute('width')) {
+          targetWidth = img.getAttribute('width');
+        } else {
+          targetWidth = img.width;
+        }
+        if (targetWidth) imgUrl += `?width=${targetWidth}`;
+      }
+      img.setAttribute('src', imgUrl);
       img.removeAttribute('data-src');
       img.classList.remove('lazy');
       img.classList.add('js-lazy-loaded');

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

@@ -5,19 +5,20 @@ import resolvedSvg from 'icons/_icon_status_success_solid.svg';
 import mrIssueSvg from 'icons/_icon_mr_issue.svg';
 import nextDiscussionSvg from 'icons/_next_discussion.svg';
 import { pluralize } from '../../lib/utils/text_utility';
-import { scrollToElement } from '../../lib/utils/common_utils';
+import discussionNavigation from '../mixins/discussion_navigation';
 import tooltip from '../../vue_shared/directives/tooltip';
 
 export default {
   directives: {
     tooltip,
   },
+  mixins: [discussionNavigation],
   computed: {
     ...mapGetters([
       'getUserData',
       'getNoteableData',
       'discussionCount',
-      'unresolvedDiscussions',
+      'firstUnresolvedDiscussionId',
       'resolvedDiscussionCount',
     ]),
     isLoggedIn() {
@@ -35,11 +36,6 @@ export default {
     resolveAllDiscussionsIssuePath() {
       return this.getNoteableData.create_issue_to_resolve_discussions_path;
     },
-    firstUnresolvedDiscussionId() {
-      const item = this.unresolvedDiscussions[0] || {};
-
-      return item.id;
-    },
   },
   created() {
     this.resolveSvg = resolveSvg;
@@ -50,22 +46,10 @@ export default {
   methods: {
     ...mapActions(['expandDiscussion']),
     jumpToFirstUnresolvedDiscussion() {
-      const discussionId = this.firstUnresolvedDiscussionId;
-      if (!discussionId) {
-        return;
-      }
-
-      const el = document.querySelector(`[data-discussion-id="${discussionId}"]`);
-      const activeTab = window.mrTabs.currentAction;
-
-      if (activeTab === 'commits' || activeTab === 'pipelines') {
-        window.mrTabs.activateTab('show');
-      }
+      const diffTab = window.mrTabs.currentAction === 'diffs';
+      const discussionId = this.firstUnresolvedDiscussionId(diffTab);
 
-      if (el) {
-        this.expandDiscussion({ discussionId });
-        scrollToElement(el);
-      }
+      this.jumpToDiscussion(discussionId);
     },
   },
 };

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

@@ -7,7 +7,7 @@ import issuableStateMixin from '../mixins/issuable_state';
 import resolvable from '../mixins/resolvable';
 
 export default {
-  name: 'IssueNoteForm',
+  name: 'NoteForm',
   components: {
     issueWarning,
     markdownField,

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

 <script>
-import _ from 'underscore';
 import { mapActions, mapGetters } from 'vuex';
 import resolveDiscussionsSvg from 'icons/_icon_mr_issue.svg';
 import nextDiscussionsSvg from 'icons/_next_discussion.svg';
-import { convertObjectPropsToCamelCase, scrollToElement } from '~/lib/utils/common_utils';
+import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import { truncateSha } from '~/lib/utils/text_utility';
 import systemNote from '~/vue_shared/components/notes/system_note.vue';
+import { s__ } from '~/locale';
 import Flash from '../../flash';
 import { SYSTEM_NOTE } from '../constants';
 import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
@@ -20,6 +20,7 @@ import placeholderSystemNote from '../../vue_shared/components/notes/placeholder
 import autosave from '../mixins/autosave';
 import noteable from '../mixins/noteable';
 import resolvable from '../mixins/resolvable';
+import discussionNavigation from '../mixins/discussion_navigation';
 import tooltip from '../../vue_shared/directives/tooltip';
 
 export default {
@@ -39,7 +40,7 @@ export default {
   directives: {
     tooltip,
   },
-  mixins: [autosave, noteable, resolvable],
+  mixins: [autosave, noteable, resolvable, discussionNavigation],
   props: {
     discussion: {
       type: Object,
@@ -60,6 +61,11 @@ export default {
       required: false,
       default: false,
     },
+    discussionsByDiffOrder: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   data() {
     return {
@@ -74,7 +80,12 @@ export default {
       'discussionCount',
       'resolvedDiscussionCount',
       'allDiscussions',
+      'unresolvedDiscussionsIdsByDiff',
+      'unresolvedDiscussionsIdsByDate',
       'unresolvedDiscussions',
+      'unresolvedDiscussionsIdsOrdered',
+      'nextUnresolvedDiscussionId',
+      'isLastUnresolvedDiscussion',
     ]),
     transformedDiscussion() {
       return {
@@ -125,6 +136,10 @@ export default {
     hasMultipleUnresolvedDiscussions() {
       return this.unresolvedDiscussions.length > 1;
     },
+    showJumpToNextDiscussion() {
+      return this.hasMultipleUnresolvedDiscussions &&
+        !this.isLastUnresolvedDiscussion(this.discussion.id, this.discussionsByDiffOrder);
+    },
     shouldRenderDiffs() {
       const { diffDiscussion, diffFile } = this.transformedDiscussion;
 
@@ -144,19 +159,17 @@ export default {
       return this.isDiffDiscussion ? '' : 'card discussion-wrapper';
     },
   },
-  mounted() {
-    if (this.isReplying) {
-      this.initAutoSave(this.transformedDiscussion);
-    }
-  },
-  updated() {
-    if (this.isReplying) {
-      if (!this.autosave) {
-        this.initAutoSave(this.transformedDiscussion);
+  watch: {
+    isReplying() {
+      if (this.isReplying) {
+        this.$nextTick(() => {
+          // Pass an extra key to separate reply and note edit forms
+          this.initAutoSave(this.transformedDiscussion, ['Reply']);
+        });
       } else {
-        this.setAutoSave();
+        this.disposeAutoSave();
       }
-    }
+    },
   },
   created() {
     this.resolveDiscussionsSvg = resolveDiscussionsSvg;
@@ -194,16 +207,18 @@ export default {
     showReplyForm() {
       this.isReplying = true;
     },
-    cancelReplyForm(shouldConfirm) {
-      if (shouldConfirm && this.$refs.noteForm.isDirty) {
+    cancelReplyForm(shouldConfirm, isDirty) {
+      if (shouldConfirm && isDirty) {
+        const msg = s__('Notes|Are you sure you want to cancel creating this comment?');
+
         // eslint-disable-next-line no-alert
-        if (!window.confirm('Are you sure you want to cancel creating this comment?')) {
+        if (!window.confirm(msg)) {
           return;
         }
       }
 
-      this.resetAutoSave();
       this.isReplying = false;
+      this.resetAutoSave();
     },
     saveReply(noteText, form, callback) {
       const postData = {
@@ -241,21 +256,10 @@ Please check your network connection and try again.`;
         });
     },
     jumpToNextDiscussion() {
-      const discussionIds = this.allDiscussions.map(d => d.id);
-      const unresolvedIds = this.unresolvedDiscussions.map(d => d.id);
-      const currentIndex = discussionIds.indexOf(this.discussion.id);
-      const remainingAfterCurrent = discussionIds.slice(currentIndex + 1);
-      const nextIndex = _.findIndex(remainingAfterCurrent, id => unresolvedIds.indexOf(id) > -1);
-
-      if (nextIndex > -1) {
-        const nextId = remainingAfterCurrent[nextIndex];
-        const el = document.querySelector(`[data-discussion-id="${nextId}"]`);
+      const nextId =
+        this.nextUnresolvedDiscussionId(this.discussion.id, this.discussionsByDiffOrder);
 
-        if (el) {
-          this.expandDiscussion({ discussionId: nextId });
-          scrollToElement(el);
-        }
-      }
+      this.jumpToDiscussion(nextId);
     },
   },
 };
@@ -397,7 +401,7 @@ Please check your network connection and try again.`;
                           </a>
                         </div>
                         <div
-                          v-if="hasMultipleUnresolvedDiscussions"
+                          v-if="showJumpToNextDiscussion"
                           class="btn-group"
                           role="group">
                           <button
@@ -420,7 +424,8 @@ Please check your network connection and try again.`;
                     :is-editing="false"
                     save-button-title="Comment"
                     @handleFormUpdate="saveReply"
-                    @cancelForm="cancelReplyForm" />
+                    @cancelForm="cancelReplyForm"
+                  />
                   <note-signed-out-widget v-if="!canReply" />
                 </div>
               </div>

app/assets/javascripts/notes/mixins/autosave.js

@@ -4,12 +4,18 @@ import { capitalizeFirstCharacter } from '../../lib/utils/text_utility';
 
 export default {
   methods: {
-    initAutoSave(noteable) {
-      this.autosave = new Autosave($(this.$refs.noteForm.$refs.textarea), [
+    initAutoSave(noteable, extraKeys = []) {
+      let keys = [
         'Note',
-        capitalizeFirstCharacter(noteable.noteable_type),
+        capitalizeFirstCharacter(noteable.noteable_type || noteable.noteableType),
         noteable.id,
-      ]);
+      ];
+
+      if (extraKeys) {
+        keys = keys.concat(extraKeys);
+      }
+
+      this.autosave = new Autosave($(this.$refs.noteForm.$refs.textarea), keys);
     },
     resetAutoSave() {
       this.autosave.reset();
@@ -17,5 +23,8 @@ export default {
     setAutoSave() {
       this.autosave.save();
     },
+    disposeAutoSave() {
+      this.autosave.dispose();
+    },
   },
 };

app/assets/javascripts/notes/mixins/discussion_navigation.js

+import { scrollToElement } from '~/lib/utils/common_utils';
+
+export default {
+  methods: {
+    jumpToDiscussion(id) {
+      if (id) {
+        const activeTab = window.mrTabs.currentAction;
+        const selector =
+          activeTab === 'diffs'
+            ? `ul.notes[data-discussion-id="${id}"]`
+            : `div.discussion[data-discussion-id="${id}"]`;
+        const el = document.querySelector(selector);
+
+        if (activeTab === 'commits' || activeTab === 'pipelines') {
+          window.mrTabs.activateTab('show');
+        }
+
+        if (el) {
+          this.expandDiscussion({ discussionId: id });
+
+          scrollToElement(el);
+          return true;
+        }
+      }
+
+      return false;
+    },
+  },
+};

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

@@ -82,6 +82,9 @@ export const allDiscussions = (state, getters) => {
   return Object.values(resolved).concat(unresolved);
 };
 
+export const allResolvableDiscussions = (state, getters) =>
+  getters.allDiscussions.filter(d => !d.individual_note && d.resolvable);
+
 export const resolvedDiscussionsById = state => {
   const map = {};
 
@@ -98,6 +101,51 @@ export const resolvedDiscussionsById = state => {
   return map;
 };
 
+// Gets Discussions IDs ordered by the date of their initial note
+export const unresolvedDiscussionsIdsByDate = (state, getters) =>
+  getters.allResolvableDiscussions
+    .filter(d => !d.resolved)
+    .sort((a, b) => {
+      const aDate = new Date(a.notes[0].created_at);
+      const bDate = new Date(b.notes[0].created_at);
+
+      if (aDate < bDate) {
+        return -1;
+      }
+
+      return aDate === bDate ? 0 : 1;
+    })
+    .map(d => d.id);
+
+// Gets Discussions IDs ordered by their position in the diff
+//
+// Sorts the array of resolvable yet unresolved discussions by
+// comparing file names first. If file names are the same, compares
+// line numbers.
+export const unresolvedDiscussionsIdsByDiff = (state, getters) =>
+  getters.allResolvableDiscussions
+    .filter(d => !d.resolved)
+    .sort((a, b) => {
+      if (!a.diff_file || !b.diff_file) {
+        return 0;
+      }
+
+      // Get file names comparison result
+      const filenameComparison = a.diff_file.file_path.localeCompare(b.diff_file.file_path);
+
+      // Get the line numbers, to compare within the same file
+      const aLines = [a.position.formatter.new_line, a.position.formatter.old_line];
+      const bLines = [b.position.formatter.new_line, b.position.formatter.old_line];
+
+      return filenameComparison < 0 ||
+        (filenameComparison === 0 &&
+          // .max() because one of them might be zero (if removed/added)
+          Math.max(aLines[0], aLines[1]) < Math.max(bLines[0], bLines[1]))
+        ? -1
+        : 1;
+    })
+    .map(d => d.id);
+
 export const resolvedDiscussionCount = (state, getters) => {
   const resolvedMap = getters.resolvedDiscussionsById;
 
@@ -114,5 +162,42 @@ export const discussionTabCounter = state => {
   return all.length;
 };
 
+// Returns the list of discussion IDs ordered according to given parameter
+// @param {Boolean} diffOrder - is ordered by diff?
+export const unresolvedDiscussionsIdsOrdered = (state, getters) => diffOrder => {
+  if (diffOrder) {
+    return getters.unresolvedDiscussionsIdsByDiff;
+  }
+  return getters.unresolvedDiscussionsIdsByDate;
+};
+
+// Checks if a given discussion is the last in the current order (diff or date)
+// @param {Boolean} discussionId - id of the discussion
+// @param {Boolean} diffOrder - is ordered by diff?
+export const isLastUnresolvedDiscussion = (state, getters) => (discussionId, diffOrder) => {
+  const idsOrdered = getters.unresolvedDiscussionsIdsOrdered(diffOrder);
+  const lastDiscussionId = idsOrdered[idsOrdered.length - 1];
+
+  return lastDiscussionId === discussionId;
+};
+
+// Gets the ID of the discussion following the one provided, respecting order (diff or date)
+// @param {Boolean} discussionId - id of the current discussion
+// @param {Boolean} diffOrder - is ordered by diff?
+export const nextUnresolvedDiscussionId = (state, getters) => (discussionId, diffOrder) => {
+  const idsOrdered = getters.unresolvedDiscussionsIdsOrdered(diffOrder);
+  const currentIndex = idsOrdered.indexOf(discussionId);
+
+  return idsOrdered.slice(currentIndex + 1, currentIndex + 2)[0];
+};
+
+// @param {Boolean} diffOrder - is ordered by diff?
+export const firstUnresolvedDiscussionId = (state, getters) => diffOrder => {
+  if (diffOrder) {
+    return getters.unresolvedDiscussionsIdsByDiff[0];
+  }
+  return getters.unresolvedDiscussionsIdsByDate[0];
+};
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_image.vue

 <script>
-
 /* This is a re-usable vue component for rendering a user avatar that
   does not need to link to the user's profile. The image and an optional
   tooltip can be configured by props passed to this component.
@@ -67,7 +66,9 @@ export default {
     // we provide an empty string when we use it inside user avatar link.
     // In both cases we should render the defaultAvatarUrl
     sanitizedSource() {
-      return this.imgSrc === '' || this.imgSrc === null ? defaultAvatarUrl : this.imgSrc;
+      let baseSrc = this.imgSrc === '' || this.imgSrc === null ? defaultAvatarUrl : this.imgSrc;
+      if (baseSrc.indexOf('?') === -1) baseSrc += `?width=${this.size}`;
+      return baseSrc;
     },
     resultantSrcAttribute() {
       return this.lazy ? placeholderImage : this.sanitizedSource;

app/models/concerns/avatarable.rb

@@ -19,7 +19,7 @@ module Avatarable
       # We use avatar_path instead of overriding avatar_url because of carrierwave.
       # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11001/diffs#note_28659864
 
-      avatar_path(only_path: args.fetch(:only_path, true)) || super
+      avatar_path(only_path: args.fetch(:only_path, true), size: args[:size]) || super
     end
 
     def retrieve_upload(identifier, paths)
@@ -40,12 +40,13 @@ module Avatarable
     end
   end
 
-  def avatar_path(only_path: true)
+  def avatar_path(only_path: true, size: nil)
     return unless self[:avatar].present?
 
     asset_host = ActionController::Base.asset_host
     use_asset_host = asset_host.present?
     use_authentication = respond_to?(:public?) && !public?
+    query_params = size&.nonzero? ? "?width=#{size}" : ""
 
     # Avatars for private and internal groups and projects require authentication to be viewed,
     # which means they can only be served by Rails, on the regular GitLab host.
@@ -64,7 +65,7 @@ module Avatarable
       url_base << gitlab_config.relative_url_root
     end
 
-    url_base + avatar.local_url
+    url_base + avatar.local_url + query_params
   end
 
   # Path that is persisted in the tracking Upload model. Used to fetch the

app/models/merge_request.rb

@@ -1095,23 +1095,29 @@ class MergeRequest < ActiveRecord::Base
 
   def can_be_reverted?(current_user)
     return false unless merge_commit
+    return false unless merged_at
 
-    merged_at = metrics&.merged_at
-    notes_association = notes_with_associations
+    # It is not guaranteed that Note#created_at will be strictly later than
+    # MergeRequestMetric#merged_at. Nanoseconds on MySQL may break this
+    # comparison, as will a HA environment if clocks are not *precisely*
+    # synchronized. Add a minute's leeway to compensate for both possibilities
+    cutoff = merged_at - 1.minute
 
-    if merged_at
-      # It is not guaranteed that Note#created_at will be strictly later than
-      # MergeRequestMetric#merged_at. Nanoseconds on MySQL may break this
-      # comparison, as will a HA environment if clocks are not *precisely*
-      # synchronized. Add a minute's leeway to compensate for both possibilities
-      cutoff = merged_at - 1.minute
-
-      notes_association = notes_association.where('created_at >= ?', cutoff)
-    end
+    notes_association = notes_with_associations.where('created_at >= ?', cutoff)
 
     !merge_commit.has_been_reverted?(current_user, notes_association)
   end
 
+  def merged_at
+    strong_memoize(:merged_at) do
+      next unless merged?
+
+      metrics&.merged_at ||
+        merge_event&.created_at ||
+        notes.system.reorder(nil).find_by(note: 'merged')&.created_at
+    end
+  end
+
   def can_be_cherry_picked?
     merge_commit.present?
   end

app/views/admin/projects/_projects.html.haml

@@ -20,7 +20,7 @@
             = link_to(admin_namespace_project_path(project.namespace, project)) do
               .dash-project-avatar
                 .avatar-container.s40
-                  = project_icon(project, alt: '', class: 'avatar project-avatar s40')
+                  = project_icon(project, alt: '', class: 'avatar project-avatar s40', width: 40, height: 40)
               %span.project-full-name
                 %span.namespace-name
                   - if project.namespace

app/views/layouts/nav/sidebar/_project.html.haml

@@ -4,7 +4,7 @@
     .context-header
       = link_to project_path(@project), title: @project.name do
         .avatar-container.s40.project-avatar
-          = project_icon(@project, alt: @project.name, class: 'avatar s40 avatar-tile')
+          = project_icon(@project, alt: @project.name, class: 'avatar s40 avatar-tile', width: 40, height: 40)
         .sidebar-context-title
           = @project.name
     %ul.sidebar-top-level-items

app/views/projects/_home_panel.html.haml

@@ -3,7 +3,7 @@
 .project-home-panel.text-center{ class: ("empty-project" if empty_repo) }
   .limit-container-width{ class: container_class }
     .avatar-container.s70.project-avatar
-      = project_icon(@project, alt: @project.name, class: 'avatar s70 avatar-tile')
+      = project_icon(@project, alt: @project.name, class: 'avatar s70 avatar-tile', width: 70, height: 70)
     %h1.project-title.qa-project-name
       = @project.name
       %span.visibility-icon.has-tooltip{ data: { container: 'body' }, title: visibility_icon_description(@project) }

app/views/projects/edit.html.haml

@@ -51,7 +51,7 @@
           .form-group
             - if @project.avatar?
               .avatar-container.s160.append-bottom-15
-                = project_icon(@project.full_path, alt: '', class: 'avatar project-avatar s160')
+                = project_icon(@project.full_path, alt: '', class: 'avatar project-avatar s160', width: 160, height: 160)
             - if @project.avatar_in_git
               %p.light
                 = _("Project avatar in repository: %{link}").html_safe % { link: @project.avatar_in_git }

app/views/shared/projects/_project.html.haml

@@ -19,7 +19,7 @@
           - if project.creator && use_creator_avatar
             = image_tag avatar_icon_for_user(project.creator, 40), class: "avatar s40", alt:''
           - else
-            = project_icon(project, alt: '', class: 'avatar project-avatar s40')
+            = project_icon(project, alt: '', class: 'avatar project-avatar s40', width: 40, height: 40)
     .project-details
       %h3.prepend-top-0.append-bottom-0
         = link_to project_path(project), class: 'text-plain' do

changelogs/unreleased/36409-frontend-for-clarifying-the-usefulness-of-the-search-bar.yml

+---
+title: UX improvements to top nav search bar
+merge_request: 20537
+author:
+type: changed

changelogs/unreleased/48456-fix-system-level-labels-admin-ui.yml

+---
+title: Fix the UI for listing system-level labels
+merge_request:
+author:
+type: fixed

changelogs/unreleased/49854-recover-mr-regression-fixes-safe-1.yml

+---
+title: Fix rendering of the context lines in MR diffs page.
+merge_request: 20968
+author:
+type: fixed

changelogs/unreleased/49854-recover-mr-regression-fixes-safe-2.yml

+---
+title: Fix autosave and ESC confirmation issues for MR discussions.
+merge_request: 20968
+author:
+type: fixed

changelogs/unreleased/49854-recover-mr-regression-fixes-safe-3.yml

+---
+title: Fix navigation to First and Next discussion on MR Changes tab.
+merge_request: 20968
+author:
+type: fixed

changelogs/unreleased/git-rerere-link-doc-update.yml

+---
+title: Update git rerere link in docs
+merge_request: 21060
+author: gfyoung
+type: other

changelogs/unreleased/osw-fix-n-plus-1-for-mrs-without-merge-info.yml

+---
+title: Avoid N+1 on MRs page when metrics merging date cannot be found
+merge_request: 21053
+author:
+type: performance

doc/administration/gitaly/index.md

@@ -63,7 +63,7 @@ Gitaly network traffic is unencrypted so you should use a firewall to
 restrict access to your Gitaly server.
 
 Below we describe how to configure a Gitaly server at address
-`gitaly.internal:9999` with secret token `abc123secret`. We assume
+`gitaly.internal:8075` with secret token `abc123secret`. We assume
 your GitLab installation has two repository storages, `default` and
 `storage1`.
 
@@ -108,8 +108,30 @@ Omnibus installations:
 
 ```ruby
 # /etc/gitlab/gitlab.rb
-gitaly['listen_addr'] = '0.0.0.0:9999'
+
+# Avoid running unnecessary services on the gitaly server
+postgresql['enable'] = false
+redis['enable'] = false
+nginx['enable'] = false
+prometheus['enable'] = false
+unicorn['enable'] = false
+sidekiq['enable'] = false
+gitlab_workhorse['enable'] = false
+
+# Prevent database connections during 'gitlab-ctl reconfigure'
+gitlab_rails['rake_cache_clear'] = false
+gitlab_rails['auto_migrate'] = false
+
+# Configure the gitlab-shell API callback URL. Without this, `git push` will
+# fail. This can be your 'front door' GitLab URL or an internal load
+# balancer.
+gitlab_rails['internal_api_url'] = 'https://gitlab.example.com'
+
+# Make Gitaly accept connections on all network interfaces. You must use
+# firewalls to restrict access to this address/port.
+gitaly['listen_addr'] = "0.0.0.0:8075"
 gitaly['auth_token'] = 'abc123secret'
+
 gitaly['storage'] = [
   { 'name' => 'default', 'path' => '/path/to/default/repositories' },
   { 'name' => 'storage1', 'path' => '/path/to/storage1/repositories' },
@@ -120,7 +142,7 @@ Source installations:
 
 ```toml
 # /home/git/gitaly/config.toml
-listen_addr = '0.0.0.0:9999'
+listen_addr = '0.0.0.0:8075'
 
 [auth]
 token = 'abc123secret'
@@ -146,7 +168,7 @@ server from reaching the Gitaly server then all Gitaly requests will
 fail.
 
 We assume that your Gitaly server can be reached at
-`gitaly.internal:9999` from your GitLab server, and that your GitLab
+`gitaly.internal:8075` from your GitLab server, and that your GitLab
 NFS shares are mounted at `/mnt/gitlab/default` and
 `/mnt/gitlab/storage1` respectively.
 
@@ -155,8 +177,8 @@ Omnibus installations:
 ```ruby
 # /etc/gitlab/gitlab.rb
 git_data_dirs({
-  'default' => { 'path' => '/mnt/gitlab/default', 'gitaly_address' => 'tcp://gitlab.internal:9999' },
-  'storage1' => { 'path' => '/mnt/gitlab/storage1', 'gitaly_address' => 'tcp://gitlab.internal:9999' },
+  'default' => { 'path' => '/mnt/gitlab/default', 'gitaly_address' => 'tcp://gitaly.internal:8075' },
+  'storage1' => { 'path' => '/mnt/gitlab/storage1', 'gitaly_address' => 'tcp://gitaly.internal:8075' },
 })
 
 gitlab_rails['gitaly_token'] = 'abc123secret'
@@ -171,10 +193,10 @@ gitlab:
     storages:
       default:
         path: /mnt/gitlab/default/repositories
-        gitaly_address: tcp://gitlab.internal:9999
+        gitaly_address: tcp://gitaly.internal:8075
       storage1:
         path: /mnt/gitlab/storage1/repositories
-        gitaly_address: tcp://gitlab.internal:9999
+        gitaly_address: tcp://gitaly.internal:8075
 
   gitaly:
     token: 'abc123secret'

doc/administration/operations/fast_ssh_key_lookup.md

-# Consider using SSH certificates instead of, or in addition to this
+# Fast lookup of authorized SSH keys in the database
 
-This document describes a drop-in replacement for the
+NOTE: **Note:** This document describes a drop-in replacement for the
 `authorized_keys` file for normal (non-deploy key) users. Consider
 using [ssh certificates](ssh_certificates.md), they are even faster,
-but are not is not a drop-in replacement.
-
-# Fast lookup of authorized SSH keys in the database
+but are not a drop-in replacement.
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/1631) in 
 > [GitLab Starter](https://about.gitlab.com/gitlab-ee) 9.3.

doc/development/automatic_ce_ee_merge.md

@@ -100,7 +100,7 @@ Notes:
   number of times you have to resolve conflicts.
 - Please remember to
   [always have your EE merge request merged before the CE version](#always-merge-ee-merge-requests-before-their-ce-counterparts).
-- You can use [`git rerere`](https://git-scm.com/blog/2010/03/08/rerere.html)
+- You can use [`git rerere`](https://git-scm.com/docs/git-rerere)
   to avoid resolving the same conflicts multiple times.
 
 ### Cherry-picking from CE to EE

doc/user/profile/index.md

@@ -30,6 +30,7 @@ You can edit your account settings by navigating from the up-right corner menu b
 From there, you can:
 
 - Update your personal information
+- Set a [custom status](#current-status) for your profile
 - Manage [2FA](account/two_factor_authentication.md)
 - Change your username and [delete your account](account/delete_account.md)
 - Manage applications that can
@@ -90,6 +91,27 @@ To enable private profile:
 NOTE: **Note:**
 You and GitLab admins can see your the abovementioned information on your profile even if it is private.
 
+## Current status
+
+> Introduced in GitLab 11.2.
+
+You can provide a custom status message for your user profile along with an emoji that describes it.
+This may be helpful when you are out of office or otherwise not available.
+Other users can then take your status into consideration when responding to your issues or assigning work to you.
+Please be aware that your status is publicly visible even if your [profile is private](#private-profile).
+
+To set your current status:
+
+1. Navigate to your personal [profile settings](#profile-settings).
+1. In the text field below `Your status`, enter your status message.
+1. Select an emoji from the dropdown if you like.
+1. Hit **Update profile settings**.
+
+Status messages are restricted to 100 characters of plain text.
+They may however contain emoji codes such as `I'm on vacation :palm_tree:`.
+
+You can also set your current status [using the API](../../api/users.md#user-status).
+
 ## Troubleshooting
 
 ### Why do I keep getting signed out?

ee/spec/javascripts/epics/epic_show/components/epic_header_spec.js

@@ -21,7 +21,7 @@ describe('epicHeader', () => {
   });
 
   it('should render author avatar', () => {
-    expect(vm.$el.querySelector('img').src).toEqual(author.src);
+    expect(vm.$el.querySelector('img').src).toEqual(`${author.src}?width=24`);
   });
 
   it('should render author name', () => {
@@ -29,7 +29,9 @@ describe('epicHeader', () => {
   });
 
   it('should render username tooltip', () => {
-    expect(vm.$el.querySelector('.user-avatar-link span').dataset.originalTitle).toEqual(author.username);
+    expect(vm.$el.querySelector('.user-avatar-link span').dataset.originalTitle).toEqual(
+      author.username,
+    );
   });
 
   describe('canDelete', () => {
@@ -37,7 +39,7 @@ describe('epicHeader', () => {
       expect(vm.$el.querySelector('.btn-remove')).toBeNull();
     });
 
-    it('should show loading button if canDelete', (done) => {
+    it('should show loading button if canDelete', done => {
       vm.canDelete = true;
       Vue.nextTick(() => {
         expect(vm.$el.querySelector('.btn-remove')).toBeDefined();
@@ -49,7 +51,7 @@ describe('epicHeader', () => {
   describe('delete epic', () => {
     let deleteEpic;
 
-    beforeEach((done) => {
+    beforeEach(done => {
       deleteEpic = jasmine.createSpy();
       spyOn(window, 'confirm').and.returnValue(true);
       vm.canDelete = true;

locale/gitlab.pot

@@ -4641,6 +4641,9 @@ msgstr ""
 msgid "Note: Consider asking your GitLab administrator to configure %{github_integration_link}, which will allow login via GitHub and allow importing repositories without generating a Personal Access Token."
 msgstr ""
 
+msgid "Notes|Are you sure you want to cancel creating this comment?"
+msgstr ""
+
 msgid "Notification events"
 msgstr ""
 

spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb

@@ -342,8 +342,9 @@ describe 'Merge request > User resolves diff notes and discussions', :js do
         end
       end
 
-      it 'shows jump to next discussion button' do
-        expect(page.all('.discussion-reply-holder', count: 2)).to all(have_selector('.discussion-next-btn'))
+      it 'shows jump to next discussion button, apart from the last one' do
+        expect(page).to have_selector('.discussion-reply-holder', count: 2)
+        expect(page).to have_selector('.discussion-reply-holder .discussion-next-btn', count: 1)
       end
 
       it 'displays next discussion even if hidden' do

spec/features/uploads/user_uploads_avatar_to_profile_spec.rb

@@ -15,7 +15,7 @@ describe 'User uploads avatar to profile' do
 
     visit user_path(user)
 
-    expect(page).to have_selector(%Q(img[data-src$="/uploads/-/system/user/avatar/#{user.id}/dk.png"]))
+    expect(page).to have_selector(%Q(img[data-src$="/uploads/-/system/user/avatar/#{user.id}/dk.png?width=90"]))
 
     # Cheating here to verify something that isn't user-facing, but is important
     expect(user.reload.avatar.file).to exist

spec/javascripts/autosave_spec.js

@@ -59,12 +59,10 @@ describe('Autosave', () => {
 
         Autosave.prototype.restore.call(autosave);
 
-        expect(
-          field.trigger,
-        ).toHaveBeenCalled();
+        expect(field.trigger).toHaveBeenCalled();
       });
 
-      it('triggers native event', (done) => {
+      it('triggers native event', done => {
         autosave.field.get(0).addEventListener('change', () => {
           done();
         });
@@ -81,9 +79,7 @@ describe('Autosave', () => {
       it('does not trigger event', () => {
         spyOn(field, 'trigger').and.callThrough();
 
-        expect(
-          field.trigger,
-        ).not.toHaveBeenCalled();
+        expect(field.trigger).not.toHaveBeenCalled();
       });
     });
   });

spec/javascripts/boards/issue_card_spec.js

@@ -72,109 +72,100 @@ describe('Issue card component', () => {
   });
 
   it('renders issue title', () => {
-    expect(
-      component.$el.querySelector('.board-card-title').textContent,
-    ).toContain(issue.title);
+    expect(component.$el.querySelector('.board-card-title').textContent).toContain(issue.title);
   });
 
   it('includes issue base in link', () => {
-    expect(
-      component.$el.querySelector('.board-card-title a').getAttribute('href'),
-    ).toContain('/test');
+    expect(component.$el.querySelector('.board-card-title a').getAttribute('href')).toContain(
+      '/test',
+    );
   });
 
   it('includes issue title on link', () => {
-    expect(
-      component.$el.querySelector('.board-card-title a').getAttribute('title'),
-    ).toBe(issue.title);
+    expect(component.$el.querySelector('.board-card-title a').getAttribute('title')).toBe(
+      issue.title,
+    );
   });
 
   it('does not render confidential icon', () => {
-    expect(
-      component.$el.querySelector('.fa-eye-flash'),
-    ).toBeNull();
+    expect(component.$el.querySelector('.fa-eye-flash')).toBeNull();
   });
 
-  it('renders confidential icon', (done) => {
+  it('renders confidential icon', done => {
     component.issue.confidential = true;
 
     Vue.nextTick(() => {
-      expect(
-        component.$el.querySelector('.confidential-icon'),
-      ).not.toBeNull();
+      expect(component.$el.querySelector('.confidential-icon')).not.toBeNull();
       done();
     });
   });
 
   it('renders issue ID with #', () => {
-    expect(
-      component.$el.querySelector('.board-card-number').textContent,
-    ).toContain(`#${issue.id}`);
+    expect(component.$el.querySelector('.board-card-number').textContent).toContain(`#${issue.id}`);
   });
 
   describe('assignee', () => {
     it('does not render assignee', () => {
-      expect(
-        component.$el.querySelector('.board-card-assignee .avatar'),
-      ).toBeNull();
+      expect(component.$el.querySelector('.board-card-assignee .avatar')).toBeNull();
     });
 
     describe('exists', () => {
-      beforeEach((done) => {
+      beforeEach(done => {
         component.issue.assignees = [user];
 
         Vue.nextTick(() => done());
       });
 
       it('renders assignee', () => {
-        expect(
-          component.$el.querySelector('.board-card-assignee .avatar'),
-        ).not.toBeNull();
+        expect(component.$el.querySelector('.board-card-assignee .avatar')).not.toBeNull();
       });
 
       it('sets title', () => {
         expect(
-          component.$el.querySelector('.board-card-assignee img').getAttribute('data-original-title'),
+          component.$el
+            .querySelector('.board-card-assignee img')
+            .getAttribute('data-original-title'),
         ).toContain(`Assigned to ${user.name}`);
       });
 
       it('sets users path', () => {
-        expect(
-          component.$el.querySelector('.board-card-assignee a').getAttribute('href'),
-        ).toBe('/test');
+        expect(component.$el.querySelector('.board-card-assignee a').getAttribute('href')).toBe(
+          '/test',
+        );
       });
 
       it('renders avatar', () => {
-        expect(
-          component.$el.querySelector('.board-card-assignee img'),
-        ).not.toBeNull();
+        expect(component.$el.querySelector('.board-card-assignee img')).not.toBeNull();
       });
     });
 
     describe('assignee default avatar', () => {
-      beforeEach((done) => {
-        component.issue.assignees = [new ListAssignee({
-          id: 1,
-          name: 'testing 123',
-          username: 'test',
-        }, 'default_avatar')];
+      beforeEach(done => {
+        component.issue.assignees = [
+          new ListAssignee(
+            {
+              id: 1,
+              name: 'testing 123',
+              username: 'test',
+            },
+            'default_avatar',
+          ),
+        ];
 
         Vue.nextTick(done);
       });
 
       it('displays defaults avatar if users avatar is null', () => {
-        expect(
-          component.$el.querySelector('.board-card-assignee img'),
-        ).not.toBeNull();
-        expect(
-          component.$el.querySelector('.board-card-assignee img').getAttribute('src'),
-        ).toBe('default_avatar');
+        expect(component.$el.querySelector('.board-card-assignee img')).not.toBeNull();
+        expect(component.$el.querySelector('.board-card-assignee img').getAttribute('src')).toBe(
+          'default_avatar?width=20',
+        );
       });
     });
   });
 
   describe('multiple assignees', () => {
-    beforeEach((done) => {
+    beforeEach(done => {
       component.issue.assignees = [
         user,
         new ListAssignee({
@@ -194,7 +185,8 @@ describe('Issue card component', () => {
           name: 'user4',
           username: 'user4',
           avatar: 'test_image',
-        })];
+        }),
+      ];
 
       Vue.nextTick(() => done());
     });
@@ -204,26 +196,30 @@ describe('Issue card component', () => {
     });
 
     describe('more than four assignees', () => {
-      beforeEach((done) => {
-        component.issue.assignees.push(new ListAssignee({
-          id: 5,
-          name: 'user5',
-          username: 'user5',
-          avatar: 'test_image',
-        }));
+      beforeEach(done => {
+        component.issue.assignees.push(
+          new ListAssignee({
+            id: 5,
+            name: 'user5',
+            username: 'user5',
+            avatar: 'test_image',
+          }),
+        );
 
         Vue.nextTick(() => done());
       });
 
       it('renders more avatar counter', () => {
-        expect(component.$el.querySelector('.board-card-assignee .avatar-counter').innerText).toEqual('+2');
+        expect(
+          component.$el.querySelector('.board-card-assignee .avatar-counter').innerText,
+        ).toEqual('+2');
       });
 
       it('renders three assignees', () => {
         expect(component.$el.querySelectorAll('.board-card-assignee .avatar').length).toEqual(3);
       });
 
-      it('renders 99+ avatar counter', (done) => {
+      it('renders 99+ avatar counter', done => {
         for (let i = 5; i < 104; i += 1) {
           const u = new ListAssignee({
             id: i,
@@ -235,7 +231,9 @@ describe('Issue card component', () => {
         }
 
         Vue.nextTick(() => {
-          expect(component.$el.querySelector('.board-card-assignee .avatar-counter').innerText).toEqual('99+');
+          expect(
+            component.$el.querySelector('.board-card-assignee .avatar-counter').innerText,
+          ).toEqual('99+');
           done();
         });
       });
@@ -243,59 +241,51 @@ describe('Issue card component', () => {
   });
 
   describe('labels', () => {
-    beforeEach((done) => {
+    beforeEach(done => {
       component.issue.addLabel(label1);
 
       Vue.nextTick(() => done());
     });
 
     it('renders list label', () => {
-      expect(
-        component.$el.querySelectorAll('.badge').length,
-      ).toBe(2);
+      expect(component.$el.querySelectorAll('.badge').length).toBe(2);
     });
 
     it('renders label', () => {
       const nodes = [];
-      component.$el.querySelectorAll('.badge').forEach((label) => {
+      component.$el.querySelectorAll('.badge').forEach(label => {
         nodes.push(label.getAttribute('data-original-title'));
       });
 
-      expect(
-        nodes.includes(label1.description),
-      ).toBe(true);
+      expect(nodes.includes(label1.description)).toBe(true);
     });
 
     it('sets label description as title', () => {
-      expect(
-        component.$el.querySelector('.badge').getAttribute('data-original-title'),
-      ).toContain(label1.description);
+      expect(component.$el.querySelector('.badge').getAttribute('data-original-title')).toContain(
+        label1.description,
+      );
     });
 
     it('sets background color of button', () => {
       const nodes = [];
-      component.$el.querySelectorAll('.badge').forEach((label) => {
+      component.$el.querySelectorAll('.badge').forEach(label => {
         nodes.push(label.style.backgroundColor);
       });
 
-      expect(
-        nodes.includes(label1.color),
-      ).toBe(true);
+      expect(nodes.includes(label1.color)).toBe(true);
     });
 
-    it('does not render label if label does not have an ID', (done) => {
-      component.issue.addLabel(new ListLabel({
-        title: 'closed',
-      }));
+    it('does not render label if label does not have an ID', done => {
+      component.issue.addLabel(
+        new ListLabel({
+          title: 'closed',
+        }),
+      );
 
       Vue.nextTick()
         .then(() => {
-          expect(
-            component.$el.querySelectorAll('.badge').length,
-          ).toBe(2);
-          expect(
-            component.$el.textContent,
-          ).not.toContain('closed');
+          expect(component.$el.querySelectorAll('.badge').length).toBe(2);
+          expect(component.$el.textContent).not.toContain('closed');
 
           done();
         })

spec/javascripts/diffs/components/diff_line_note_form_spec.js

@@ -3,6 +3,7 @@ import DiffLineNoteForm from '~/diffs/components/diff_line_note_form.vue';
 import store from '~/mr_notes/stores';
 import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
 import diffFileMockData from '../mock_data/diff_file';
+import { noteableDataMock } from '../../notes/mock_data';
 
 describe('DiffLineNoteForm', () => {
   let component;
@@ -21,10 +22,9 @@ describe('DiffLineNoteForm', () => {
       noteTargetLine: diffLines[0],
     });
 
-    Object.defineProperty(component, 'isLoggedIn', {
-      get() {
-        return true;
-      },
+    Object.defineProperties(component, {
+      noteableData: { value: noteableDataMock },
+      isLoggedIn: { value: true },
     });
 
     component.$mount();
@@ -32,12 +32,37 @@ describe('DiffLineNoteForm', () => {
 
   describe('methods', () => {
     describe('handleCancelCommentForm', () => {
-      it('should call cancelCommentForm with lineCode', () => {
+      it('should ask for confirmation when shouldConfirm and isDirty passed as truthy', () => {
+        spyOn(window, 'confirm').and.returnValue(false);
+
+        component.handleCancelCommentForm(true, true);
+        expect(window.confirm).toHaveBeenCalled();
+      });
+
+      it('should ask for confirmation when one of the params false', () => {
+        spyOn(window, 'confirm').and.returnValue(false);
+
+        component.handleCancelCommentForm(true, false);
+        expect(window.confirm).not.toHaveBeenCalled();
+
+        component.handleCancelCommentForm(false, true);
+        expect(window.confirm).not.toHaveBeenCalled();
+      });
+
+      it('should call cancelCommentForm with lineCode', done => {
+        spyOn(window, 'confirm');
         spyOn(component, 'cancelCommentForm');
+        spyOn(component, 'resetAutoSave');
         component.handleCancelCommentForm();
 
-        expect(component.cancelCommentForm).toHaveBeenCalledWith({
-          lineCode: diffLines[0].lineCode,
+        expect(window.confirm).not.toHaveBeenCalled();
+        component.$nextTick(() => {
+          expect(component.cancelCommentForm).toHaveBeenCalledWith({
+            lineCode: diffLines[0].lineCode,
+          });
+          expect(component.resetAutoSave).toHaveBeenCalled();
+
+          done();
         });
       });
     });
@@ -66,7 +91,7 @@ describe('DiffLineNoteForm', () => {
 
   describe('mounted', () => {
     it('should init autosave', () => {
-      const key = 'autosave/Note/issue///DiffNote//1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1';
+      const key = 'autosave/Note/Issue/98//DiffNote//1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1';
 
       expect(component.autosave).toBeDefined();
       expect(component.autosave.key).toEqual(key);

spec/javascripts/notes/components/discussion_counter_spec.js

@@ -46,7 +46,7 @@ describe('DiscussionCounter component', () => {
           discussions,
         });
         setFixtures(`
-          <div data-discussion-id="${firstDiscussionId}"></div>
+          <div class="discussion" data-discussion-id="${firstDiscussionId}"></div>
         `);
 
         vm.jumpToFirstUnresolvedDiscussion();

spec/javascripts/notes/components/noteable_discussion_spec.js

@@ -14,6 +14,7 @@ describe('noteable_discussion component', () => {
   preloadFixtures(discussionWithTwoUnresolvedNotes);
 
   beforeEach(() => {
+    window.mrTabs = {};
     store = createStore();
     store.dispatch('setNoteableData', noteableDataMock);
     store.dispatch('setNotesData', notesDataMock);
@@ -46,10 +47,15 @@ describe('noteable_discussion component', () => {
 
     it('should toggle reply form', done => {
       vm.$el.querySelector('.js-vue-discussion-reply').click();
+
       Vue.nextTick(() => {
-        expect(vm.$refs.noteForm).not.toBeNull();
         expect(vm.isReplying).toEqual(true);
-        done();
+
+        // There is a watcher for `isReplying` which will init autosave in the next tick
+        Vue.nextTick(() => {
+          expect(vm.$refs.noteForm).not.toBeNull();
+          done();
+        });
       });
     });
 
@@ -101,33 +107,29 @@ describe('noteable_discussion component', () => {
 
   describe('methods', () => {
     describe('jumpToNextDiscussion', () => {
-      it('expands next unresolved discussion', () => {
-        spyOn(vm, 'expandDiscussion').and.stub();
-        const discussions = [
-          discussionMock,
-          {
-            ...discussionMock,
-            id: discussionMock.id + 1,
-            notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: true }],
-          },
-          {
-            ...discussionMock,
-            id: discussionMock.id + 2,
-            notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: false }],
-          },
-        ];
-        const nextDiscussionId = discussionMock.id + 2;
-        store.replaceState({
-          ...store.state,
-          discussions,
-        });
-        setFixtures(`
-          <div data-discussion-id="${nextDiscussionId}"></div>
-        `);
+      it('expands next unresolved discussion', done => {
+        const discussion2 = getJSONFixture(discussionWithTwoUnresolvedNotes)[0];
+        discussion2.resolved = false;
+        discussion2.id = 'next'; // prepare this for being identified as next one (to be jumped to)
+        vm.$store.dispatch('setInitialNotes', [discussionMock, discussion2]);
+        window.mrTabs.currentAction = 'show';
+
+        Vue.nextTick()
+          .then(() => {
+            spyOn(vm, 'expandDiscussion').and.stub();
+
+            const nextDiscussionId = discussion2.id;
+
+            setFixtures(`
+              <div class="discussion" data-discussion-id="${nextDiscussionId}"></div>
+            `);
 
-        vm.jumpToNextDiscussion();
+            vm.jumpToNextDiscussion();
 
-        expect(vm.expandDiscussion).toHaveBeenCalledWith({ discussionId: nextDiscussionId });
+            expect(vm.expandDiscussion).toHaveBeenCalledWith({ discussionId: nextDiscussionId });
+          })
+          .then(done)
+          .catch(done.fail);
       });
     });
   });

spec/javascripts/notes/mock_data.js

@@ -1168,3 +1168,87 @@ export const collapsedSystemNotes = [
     diff_discussion: false,
   },
 ];
+
+export const discussion1 = {
+  id: 'abc1',
+  resolvable: true,
+  resolved: false,
+  diff_file: {
+    file_path: 'about.md',
+  },
+  position: {
+    formatter: {
+      new_line: 50,
+      old_line: null,
+    },
+  },
+  notes: [
+    {
+      created_at: '2018-07-04T16:25:41.749Z',
+    },
+  ],
+};
+
+export const resolvedDiscussion1 = {
+  id: 'abc1',
+  resolvable: true,
+  resolved: true,
+  diff_file: {
+    file_path: 'about.md',
+  },
+  position: {
+    formatter: {
+      new_line: 50,
+      old_line: null,
+    },
+  },
+  notes: [
+    {
+      created_at: '2018-07-04T16:25:41.749Z',
+    },
+  ],
+};
+
+export const discussion2 = {
+  id: 'abc2',
+  resolvable: true,
+  resolved: false,
+  diff_file: {
+    file_path: 'README.md',
+  },
+  position: {
+    formatter: {
+      new_line: null,
+      old_line: 20,
+    },
+  },
+  notes: [
+    {
+      created_at: '2018-07-04T12:05:41.749Z',
+    },
+  ],
+};
+
+export const discussion3 = {
+  id: 'abc3',
+  resolvable: true,
+  resolved: false,
+  diff_file: {
+    file_path: 'README.md',
+  },
+  position: {
+    formatter: {
+      new_line: 21,
+      old_line: null,
+    },
+  },
+  notes: [
+    {
+      created_at: '2018-07-05T17:25:41.749Z',
+    },
+  ],
+};
+
+export const unresolvableDiscussion = {
+  resolvable: false,
+};

spec/javascripts/notes/stores/getters_spec.js

@@ -5,6 +5,11 @@ import {
   noteableDataMock,
   individualNote,
   collapseNotesMock,
+  discussion1,
+  discussion2,
+  discussion3,
+  resolvedDiscussion1,
+  unresolvableDiscussion,
 } from '../mock_data';
 
 const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json';
@@ -109,4 +114,154 @@ describe('Getters Notes Store', () => {
       expect(getters.isNotesFetched(state)).toBeFalsy();
     });
   });
+
+  describe('allResolvableDiscussions', () => {
+    it('should return only resolvable discussions in same order', () => {
+      const localGetters = {
+        allDiscussions: [
+          discussion3,
+          unresolvableDiscussion,
+          discussion1,
+          unresolvableDiscussion,
+          discussion2,
+        ],
+      };
+
+      expect(getters.allResolvableDiscussions(state, localGetters)).toEqual([
+        discussion3,
+        discussion1,
+        discussion2,
+      ]);
+    });
+
+    it('should return empty array if there are no resolvable discussions', () => {
+      const localGetters = {
+        allDiscussions: [unresolvableDiscussion, unresolvableDiscussion],
+      };
+
+      expect(getters.allResolvableDiscussions(state, localGetters)).toEqual([]);
+    });
+  });
+
+  describe('unresolvedDiscussionsIdsByDiff', () => {
+    it('should return all discussions IDs in diff order', () => {
+      const localGetters = {
+        allResolvableDiscussions: [discussion3, discussion1, discussion2],
+      };
+
+      expect(getters.unresolvedDiscussionsIdsByDiff(state, localGetters)).toEqual([
+        'abc1',
+        'abc2',
+        'abc3',
+      ]);
+    });
+
+    it('should return empty array if all discussions have been resolved', () => {
+      const localGetters = {
+        allResolvableDiscussions: [resolvedDiscussion1],
+      };
+
+      expect(getters.unresolvedDiscussionsIdsByDiff(state, localGetters)).toEqual([]);
+    });
+  });
+
+  describe('unresolvedDiscussionsIdsByDate', () => {
+    it('should return all discussions in date ascending order', () => {
+      const localGetters = {
+        allResolvableDiscussions: [discussion3, discussion1, discussion2],
+      };
+
+      expect(getters.unresolvedDiscussionsIdsByDate(state, localGetters)).toEqual([
+        'abc2',
+        'abc1',
+        'abc3',
+      ]);
+    });
+
+    it('should return empty array if all discussions have been resolved', () => {
+      const localGetters = {
+        allResolvableDiscussions: [resolvedDiscussion1],
+      };
+
+      expect(getters.unresolvedDiscussionsIdsByDate(state, localGetters)).toEqual([]);
+    });
+  });
+
+  describe('unresolvedDiscussionsIdsOrdered', () => {
+    const localGetters = {
+      unresolvedDiscussionsIdsByDate: ['123', '456'],
+      unresolvedDiscussionsIdsByDiff: ['abc', 'def'],
+    };
+
+    it('should return IDs ordered by diff when diffOrder param is true', () => {
+      expect(getters.unresolvedDiscussionsIdsOrdered(state, localGetters)(true)).toEqual([
+        'abc',
+        'def',
+      ]);
+    });
+
+    it('should return IDs ordered by date when diffOrder param is not true', () => {
+      expect(getters.unresolvedDiscussionsIdsOrdered(state, localGetters)(false)).toEqual([
+        '123',
+        '456',
+      ]);
+      expect(getters.unresolvedDiscussionsIdsOrdered(state, localGetters)(undefined)).toEqual([
+        '123',
+        '456',
+      ]);
+    });
+  });
+
+  describe('isLastUnresolvedDiscussion', () => {
+    const localGetters = {
+      unresolvedDiscussionsIdsOrdered: () => ['123', '456', '789'],
+    };
+
+    it('should return true if the discussion id provided is the last', () => {
+      expect(getters.isLastUnresolvedDiscussion(state, localGetters)('789')).toBe(true);
+    });
+
+    it('should return false if the discussion id provided is not the last', () => {
+      expect(getters.isLastUnresolvedDiscussion(state, localGetters)('123')).toBe(false);
+      expect(getters.isLastUnresolvedDiscussion(state, localGetters)('456')).toBe(false);
+    });
+  });
+
+  describe('nextUnresolvedDiscussionId', () => {
+    const localGetters = {
+      unresolvedDiscussionsIdsOrdered: () => ['123', '456', '789'],
+    };
+
+    it('should return the ID of the discussion after the ID provided', () => {
+      expect(getters.nextUnresolvedDiscussionId(state, localGetters)('123')).toBe('456');
+      expect(getters.nextUnresolvedDiscussionId(state, localGetters)('456')).toBe('789');
+      expect(getters.nextUnresolvedDiscussionId(state, localGetters)('789')).toBe(undefined);
+    });
+  });
+
+  describe('firstUnresolvedDiscussionId', () => {
+    const localGetters = {
+      unresolvedDiscussionsIdsByDate: ['123', '456'],
+      unresolvedDiscussionsIdsByDiff: ['abc', 'def'],
+    };
+
+    it('should return the first discussion id by diff when diffOrder param is true', () => {
+      expect(getters.firstUnresolvedDiscussionId(state, localGetters)(true)).toBe('abc');
+    });
+
+    it('should return the first discussion id by date when diffOrder param is not true', () => {
+      expect(getters.firstUnresolvedDiscussionId(state, localGetters)(false)).toBe('123');
+      expect(getters.firstUnresolvedDiscussionId(state, localGetters)(undefined)).toBe('123');
+    });
+
+    it('should be falsy if all discussions are resolved', () => {
+      const localGettersFalsy = {
+        unresolvedDiscussionsIdsByDiff: [],
+        unresolvedDiscussionsIdsByDate: [],
+      };
+
+      expect(getters.firstUnresolvedDiscussionId(state, localGettersFalsy)(true)).toBeFalsy();
+      expect(getters.firstUnresolvedDiscussionId(state, localGettersFalsy)(false)).toBeFalsy();
+    });
+  });
 });

spec/javascripts/pipelines/pipeline_url_spec.js

@@ -35,7 +35,9 @@ describe('Pipeline Url Component', () => {
       },
     }).$mount();
 
-    expect(component.$el.querySelector('.js-pipeline-url-link').getAttribute('href')).toEqual('foo');
+    expect(component.$el.querySelector('.js-pipeline-url-link').getAttribute('href')).toEqual(
+      'foo',
+    );
     expect(component.$el.querySelector('.js-pipeline-url-link span').textContent).toEqual('#1');
   });
 
@@ -61,11 +63,11 @@ describe('Pipeline Url Component', () => {
 
     const image = component.$el.querySelector('.js-pipeline-url-user img');
 
-    expect(
-      component.$el.querySelector('.js-pipeline-url-user').getAttribute('href'),
-    ).toEqual(mockData.pipeline.user.web_url);
+    expect(component.$el.querySelector('.js-pipeline-url-user').getAttribute('href')).toEqual(
+      mockData.pipeline.user.web_url,
+    );
     expect(image.getAttribute('data-original-title')).toEqual(mockData.pipeline.user.name);
-    expect(image.getAttribute('src')).toEqual(mockData.pipeline.user.avatar_url);
+    expect(image.getAttribute('src')).toEqual(`${mockData.pipeline.user.avatar_url}?width=20`);
   });
 
   it('should render "API" when no user is provided', () => {
@@ -100,7 +102,9 @@ describe('Pipeline Url Component', () => {
     }).$mount();
 
     expect(component.$el.querySelector('.js-pipeline-url-latest').textContent).toContain('latest');
-    expect(component.$el.querySelector('.js-pipeline-url-yaml').textContent).toContain('yaml invalid');
+    expect(component.$el.querySelector('.js-pipeline-url-yaml').textContent).toContain(
+      'yaml invalid',
+    );
     expect(component.$el.querySelector('.js-pipeline-url-stuck').textContent).toContain('stuck');
   });
 
@@ -121,9 +125,9 @@ describe('Pipeline Url Component', () => {
       },
     }).$mount();
 
-    expect(
-      component.$el.querySelector('.js-pipeline-url-autodevops').textContent.trim(),
-    ).toEqual('Auto DevOps');
+    expect(component.$el.querySelector('.js-pipeline-url-autodevops').textContent.trim()).toEqual(
+      'Auto DevOps',
+    );
   });
 
   it('should render error badge when pipeline has a failure reason set', () => {
@@ -142,6 +146,8 @@ describe('Pipeline Url Component', () => {
     }).$mount();
 
     expect(component.$el.querySelector('.js-pipeline-url-failure').textContent).toContain('error');
-    expect(component.$el.querySelector('.js-pipeline-url-failure').getAttribute('data-original-title')).toContain('some reason');
+    expect(
+      component.$el.querySelector('.js-pipeline-url-failure').getAttribute('data-original-title'),
+    ).toContain('some reason');
   });
 });

spec/javascripts/vue_shared/components/notes/placeholder_note_spec.js

@@ -27,7 +27,7 @@ describe('issue placeholder system note component', () => {
         userDataMock.path,
       );
       expect(vm.$el.querySelector('.user-avatar-link img').getAttribute('src')).toEqual(
-        userDataMock.avatar_url,
+        `${userDataMock.avatar_url}?width=40`,
       );
     });
   });

spec/javascripts/vue_shared/components/user_avatar/user_avatar_image_spec.js

@@ -12,7 +12,7 @@ const DEFAULT_PROPS = {
   tooltipPlacement: 'bottom',
 };
 
-describe('User Avatar Image Component', function () {
+describe('User Avatar Image Component', function() {
   let vm;
   let UserAvatarImage;
 
@@ -20,37 +20,37 @@ describe('User Avatar Image Component', function () {
     UserAvatarImage = Vue.extend(userAvatarImage);
   });
 
-  describe('Initialization', function () {
-    beforeEach(function () {
+  describe('Initialization', function() {
+    beforeEach(function() {
       vm = mountComponent(UserAvatarImage, {
         ...DEFAULT_PROPS,
       }).$mount();
     });
 
-    it('should return a defined Vue component', function () {
+    it('should return a defined Vue component', function() {
       expect(vm).toBeDefined();
     });
 
-    it('should have <img> as a child element', function () {
+    it('should have <img> as a child element', function() {
       expect(vm.$el.tagName).toBe('IMG');
-      expect(vm.$el.getAttribute('src')).toBe(DEFAULT_PROPS.imgSrc);
-      expect(vm.$el.getAttribute('data-src')).toBe(DEFAULT_PROPS.imgSrc);
+      expect(vm.$el.getAttribute('src')).toBe(`${DEFAULT_PROPS.imgSrc}?width=99`);
+      expect(vm.$el.getAttribute('data-src')).toBe(`${DEFAULT_PROPS.imgSrc}?width=99`);
       expect(vm.$el.getAttribute('alt')).toBe(DEFAULT_PROPS.imgAlt);
     });
 
-    it('should properly compute tooltipContainer', function () {
+    it('should properly compute tooltipContainer', function() {
       expect(vm.tooltipContainer).toBe('body');
     });
 
-    it('should properly render tooltipContainer', function () {
+    it('should properly render tooltipContainer', function() {
       expect(vm.$el.getAttribute('data-container')).toBe('body');
     });
 
-    it('should properly compute avatarSizeClass', function () {
+    it('should properly compute avatarSizeClass', function() {
       expect(vm.avatarSizeClass).toBe('s99');
     });
 
-    it('should properly render img css', function () {
+    it('should properly render img css', function() {
       const { classList } = vm.$el;
       const containsAvatar = classList.contains('avatar');
       const containsSizeClass = classList.contains('s99');
@@ -64,21 +64,21 @@ describe('User Avatar Image Component', function () {
     });
   });
 
-  describe('Initialization when lazy', function () {
-    beforeEach(function () {
+  describe('Initialization when lazy', function() {
+    beforeEach(function() {
       vm = mountComponent(UserAvatarImage, {
         ...DEFAULT_PROPS,
         lazy: true,
       }).$mount();
     });
 
-    it('should add lazy attributes', function () {
+    it('should add lazy attributes', function() {
       const { classList } = vm.$el;
       const lazyClass = classList.contains('lazy');
 
       expect(lazyClass).toBe(true);
       expect(vm.$el.getAttribute('src')).toBe(placeholderImage);
-      expect(vm.$el.getAttribute('data-src')).toBe(DEFAULT_PROPS.imgSrc);
+      expect(vm.$el.getAttribute('data-src')).toBe(`${DEFAULT_PROPS.imgSrc}?width=99`);
     });
   });
 });

spec/javascripts/vue_shared/security_reports/card_security_reports_app_spec.js

@@ -95,7 +95,9 @@ describe('Card security reports app', () => {
 
       expect(userAvatarLink).not.toBeNull();
       expect(userAvatarLink.getAttribute('href')).toBe(`${TEST_HOST}/user`);
-      expect(userAvatarLink.querySelector('img').getAttribute('src')).toBe(`${TEST_HOST}/img`);
+      expect(userAvatarLink.querySelector('img').getAttribute('src')).toBe(
+        `${TEST_HOST}/img?width=24`,
+      );
       expect(userAvatarLink.textContent).toBe('TestUser');
     });
 

spec/models/concerns/avatarable_spec.rb

@@ -43,6 +43,10 @@ describe Avatarable do
         expect(project.avatar_path(only_path: only_path)).to eq(avatar_path)
       end
 
+      it 'returns the expected avatar path with width parameter' do
+        expect(project.avatar_path(only_path: only_path, size: 128)).to eq(avatar_path + "?width=128")
+      end
+
       context "when avatar is stored remotely" do
         before do
           stub_uploads_object_storage(AvatarUploader)

spec/models/merge_request_spec.rb

@@ -1570,6 +1570,16 @@ describe MergeRequest do
                                           project.default_branch == branch)
         end
 
+        context 'but merged at timestamp cannot be found' do
+          before do
+            allow(subject).to receive(:merged_at) { nil }
+          end
+
+          it 'returns false' do
+            expect(subject.can_be_reverted?(current_user)).to be_falsey
+          end
+        end
+
         context 'when the revert commit is mentioned in a note after the MR was merged' do
           it 'returns false' do
             expect(subject.can_be_reverted?(current_user)).to be_falsey
@@ -1609,6 +1619,63 @@ describe MergeRequest do
     end
   end
 
+  describe '#merged_at' do
+    context 'when MR is not merged' do
+      let(:merge_request) { create(:merge_request, :closed) }
+
+      it 'returns nil' do
+        expect(merge_request.merged_at).to be_nil
+      end
+    end
+
+    context 'when metrics has merged_at data' do
+      let(:merge_request) { create(:merge_request, :merged) }
+
+      before do
+        merge_request.metrics.update!(merged_at: 1.day.ago)
+      end
+
+      it 'returns metrics merged_at' do
+        expect(merge_request.merged_at).to eq(merge_request.metrics.merged_at)
+      end
+    end
+
+    context 'when merged event is persisted, but no metrics merged_at is persisted' do
+      let(:user) { create(:user) }
+      let(:merge_request) { create(:merge_request, :merged) }
+
+      before do
+        EventCreateService.new.merge_mr(merge_request, user)
+      end
+
+      it 'returns merged event creation date' do
+        expect(merge_request.merge_event).to be_persisted
+        expect(merge_request.merged_at).to eq(merge_request.merge_event.created_at)
+      end
+    end
+
+    context 'when merging note is persisted, but no metrics or merge event exists' do
+      let(:user) { create(:user) }
+      let(:merge_request) { create(:merge_request, :merged) }
+
+      before do
+        merge_request.metrics.destroy!
+
+        SystemNoteService.change_status(merge_request,
+                                        merge_request.target_project,
+                                        user,
+                                        merge_request.state, nil)
+      end
+
+      it 'returns merging note creation date' do
+        expect(merge_request.reload.metrics).to be_nil
+        expect(merge_request.merge_event).to be_nil
+        expect(merge_request.notes.count).to eq(1)
+        expect(merge_request.merged_at).to eq(merge_request.notes.first.created_at)
+      end
+    end
+  end
+
   describe '#participants' do
     let(:project) { create(:project, :public) }