Merge branch 'ce-to-ee-2018-11-20' into 'master'

CE upstream - 2018-11-20 18:49 UTC

Closes gitlab-ce#51635

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

app/assets/javascripts/behaviors/shortcuts/shortcuts_issuable.js

@@ -4,6 +4,7 @@ import _ from 'underscore';
 import Sidebar from '../../right_sidebar';
 import Shortcuts from './shortcuts';
 import { CopyAsGFM } from '../markdown/copy_as_gfm';
+import { getSelectedFragment } from '~/lib/utils/common_utils';
 
 export default class ShortcutsIssuable extends Shortcuts {
   constructor(isMergeRequest) {
@@ -24,17 +25,43 @@ export default class ShortcutsIssuable extends Shortcuts {
 
   static replyWithSelectedText() {
     const $replyField = $('.js-main-target-form .js-vue-comment-form');
-    const documentFragment = window.gl.utils.getSelectedFragment();
 
-    if (!$replyField.length) {
+    if (!$replyField.length || $replyField.is(':hidden') /* Other tab selected in MR */) {
       return false;
     }
 
+    const documentFragment = getSelectedFragment(document.querySelector('.issuable-details'));
+
     if (!documentFragment) {
       $replyField.focus();
       return false;
     }
 
+    // Sanity check: Make sure the selected text comes from a discussion : it can either contain a message...
+    let foundMessage = !!documentFragment.querySelector('.md, .wiki');
+
+    // ... Or come from a message
+    if (!foundMessage) {
+      if (documentFragment.originalNodes) {
+        documentFragment.originalNodes.forEach(e => {
+          let node = e;
+          do {
+            // Text nodes don't define the `matches` method
+            if (node.matches && node.matches('.md, .wiki')) {
+              foundMessage = true;
+            }
+            node = node.parentNode;
+          } while (node && !foundMessage);
+        });
+      }
+
+      // If there is no message, just select the reply field
+      if (!foundMessage) {
+        $replyField.focus();
+        return false;
+      }
+    }
+
     const el = CopyAsGFM.transformGFMSelection(documentFragment.cloneNode(true));
     const selected = CopyAsGFM.nodeToGFM(el);
 

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

@@ -56,7 +56,10 @@ export const setFileActive = ({ commit, state, getters, dispatch }, path) => {
   dispatch('scrollToTab');
 };
 
-export const getFileData = ({ state, commit, dispatch }, { path, makeFileActive = true }) => {
+export const getFileData = (
+  { state, commit, dispatch },
+  { path, makeFileActive = true, openFile = makeFileActive },
+) => {
   const file = state.entries[path];
 
   if (file.raw || (file.tempFile && !file.prevPath)) return Promise.resolve();
@@ -71,8 +74,8 @@ export const getFileData = ({ state, commit, dispatch }, { path, makeFileActive
       const normalizedHeaders = normalizeHeaders(headers);
       setPageTitle(decodeURI(normalizedHeaders['PAGE-TITLE']));
 
-      commit(types.SET_FILE_DATA, { data, file });
-      if (makeFileActive) commit(types.TOGGLE_FILE_OPEN, path);
+      if (data) commit(types.SET_FILE_DATA, { data, file });
+      if (openFile) commit(types.TOGGLE_FILE_OPEN, path);
       if (makeFileActive) dispatch('setFileActive', path);
       commit(types.TOGGLE_LOADING, { entry: file });
     })

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

@@ -161,6 +161,7 @@ export const openMergeRequest = (
             dispatch('getFileData', {
               path: change.new_path,
               makeFileActive: ind === 0,
+              openFile: true,
             });
           }
         }

app/assets/javascripts/lib/utils/common_utils.js

@@ -226,7 +226,17 @@ export const getParameterByName = (name, urlToParse) => {
   return decodeURIComponent(results[2].replace(/\+/g, ' '));
 };
 
-const handleSelectedRange = range => {
+const handleSelectedRange = (range, restrictToNode) => {
+  // Make sure this range is within the restricting container
+  if (restrictToNode && !range.intersectsNode(restrictToNode)) return null;
+
+  // If only a part of the range is within the wanted container, we need to restrict the range to it
+  if (restrictToNode && !restrictToNode.contains(range.commonAncestorContainer)) {
+    if (!restrictToNode.contains(range.startContainer)) range.setStart(restrictToNode, 0);
+    if (!restrictToNode.contains(range.endContainer))
+      range.setEnd(restrictToNode, restrictToNode.childNodes.length);
+  }
+
   const container = range.commonAncestorContainer;
   // add context to fragment if needed
   if (container.tagName === 'OL') {
@@ -237,14 +247,22 @@ const handleSelectedRange = range => {
   return range.cloneContents();
 };
 
-export const getSelectedFragment = () => {
+export const getSelectedFragment = restrictToNode => {
   const selection = window.getSelection();
   if (selection.rangeCount === 0) return null;
+  // Most usages of the selection only want text from a part of the page (e.g. discussion)
+  if (restrictToNode && !selection.containsNode(restrictToNode, true)) return null;
+
   const documentFragment = document.createDocumentFragment();
+  documentFragment.originalNodes = [];
 
   for (let i = 0; i < selection.rangeCount; i += 1) {
     const range = selection.getRangeAt(i);
-    documentFragment.appendChild(handleSelectedRange(range));
+    const handledRange = handleSelectedRange(range, restrictToNode);
+    if (handledRange) {
+      documentFragment.appendChild(handledRange);
+      documentFragment.originalNodes.push(range.commonAncestorContainer);
+    }
   }
   if (documentFragment.textContent.length === 0) return null;
 

app/assets/stylesheets/framework/typography.scss

@@ -360,10 +360,6 @@ code {
   font-size: 95%;
 }
 
-.git-revision-dropdown-toggle {
-  @extend .monospace;
-}
-
 .git-revision-dropdown .dropdown-content ul li a {
   @extend .ref-name;
 }

app/assets/stylesheets/pages/editor.scss

@@ -32,7 +32,6 @@
 
   .file-title {
     @extend .monospace;
-
     line-height: 35px;
     padding-top: 7px;
     padding-bottom: 7px;
@@ -48,13 +47,6 @@
     margin-right: 10px;
   }
 
-  .editor-file-name {
-    @extend .monospace;
-
-    float: left;
-    margin-right: 10px;
-  }
-
   .new-file-name {
     display: inline-block;
     max-width: 420px;

app/assets/stylesheets/pages/pipelines.scss

@@ -141,10 +141,6 @@
     float: none;
   }
 
-  .api {
-    @extend .monospace;
-  }
-
   .branch-commit {
     .ref-name {
       font-weight: $gl-font-weight-bold;

app/assets/stylesheets/pages/projects.scss

@@ -352,10 +352,6 @@
 
   .mobile-git-clone {
     margin-top: $gl-padding-8;
-
-    .dropdown-menu-inner-content {
-      @extend .monospace;
-    }
   }
 }
 

app/views/peek/views/_gc.html.haml

 - local_assigns.fetch(:view)
 
 %span.bold
-  %span{ title: 'Invoke Time', data: { defer_to: "#{view.defer_key}-gc_time" } }...
+  %span{ title: _('Invoke Time'), data: { defer_to: "#{view.defer_key}-gc_time" } }...
   \/
-  %span{ title: 'Invoke Count', data: { defer_to: "#{view.defer_key}-invokes" } }...
+  %span{ title: _('Invoke Count'), data: { defer_to: "#{view.defer_key}-invokes" } }...
 gc

app/views/projects/blob/_editor.html.haml

@@ -6,12 +6,12 @@
       = sprite_icon('fork', size: 12)
       = ref
     - if current_action?(:edit) || current_action?(:update)
-      %span.editor-file-name
+      %span.pull-left.append-right-10
         = text_field_tag 'file_path', (params[:file_path] || @path),
                                       class: 'form-control new-file-path js-file-path-name-input'
 
     - if current_action?(:new) || current_action?(:create)
-      %span.editor-file-name
+      %span.pull-left.append-right-10
         \/
       = text_field_tag 'file_name', params[:file_name], placeholder: "File name",
         required: true, class: 'form-control new-file-name js-file-path-name-input'

app/views/projects/branches/new.html.haml

@@ -20,7 +20,7 @@
     .col-sm-10.create-from
       .dropdown
         = hidden_field_tag :ref, default_ref
-        = button_tag type: 'button', title: default_ref, class: 'dropdown-menu-toggle wide js-branch-select git-revision-dropdown-toggle', required: true, data: { toggle: 'dropdown', selected: default_ref, field_name: 'ref' } do
+        = button_tag type: 'button', title: default_ref, class: 'dropdown-menu-toggle wide js-branch-select monospace', required: true, data: { toggle: 'dropdown', selected: default_ref, field_name: 'ref' } do
           .text-left.dropdown-toggle-text= default_ref
           = icon('chevron-down')
         = render 'shared/ref_dropdown', dropdown_class: 'wide'

app/views/projects/ci/builds/_build.html.haml

@@ -60,7 +60,7 @@
       - if pipeline.user
         = user_avatar(user: pipeline.user, size: 20)
       - else
-        %span.api API
+        %span.monospace API
 
   - if admin
     %td

app/views/projects/compare/_form.html.haml

@@ -8,7 +8,7 @@
         .input-group-text
           = s_("CompareBranches|Source")
       = hidden_field_tag :to, params[:to]
-      = button_tag type: 'button', title: params[:to], class: "form-control compare-dropdown-toggle js-compare-dropdown has-tooltip git-revision-dropdown-toggle", required: true, data: { refs_url: refs_project_path(@project), toggle: "dropdown", target: ".js-compare-to-dropdown", selected: params[:to], field_name: :to } do
+      = button_tag type: 'button', title: params[:to], class: "form-control compare-dropdown-toggle js-compare-dropdown has-tooltip monospace", required: true, data: { refs_url: refs_project_path(@project), toggle: "dropdown", target: ".js-compare-to-dropdown", selected: params[:to], field_name: :to } do
         .dropdown-toggle-text.str-truncated= params[:to] || _("Select branch/tag")
     = render 'shared/ref_dropdown'
   .compare-ellipsis.inline ...
@@ -18,7 +18,7 @@
         .input-group-text
           = s_("CompareBranches|Target")
       = hidden_field_tag :from, params[:from]
-      = button_tag type: 'button', title: params[:from], class: "form-control compare-dropdown-toggle js-compare-dropdown has-tooltip git-revision-dropdown-toggle", required: true, data: { refs_url: refs_project_path(@project), toggle: "dropdown", target: ".js-compare-from-dropdown", selected: params[:from], field_name: :from } do
+      = button_tag type: 'button', title: params[:from], class: "form-control compare-dropdown-toggle js-compare-dropdown has-tooltip monospace", required: true, data: { refs_url: refs_project_path(@project), toggle: "dropdown", target: ".js-compare-from-dropdown", selected: params[:from], field_name: :from } do
         .dropdown-toggle-text.str-truncated= params[:from] || _("Select branch/tag")
     = render 'shared/ref_dropdown'
    

app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml

@@ -48,7 +48,7 @@
       - if generic_commit_status.pipeline.user
         = user_avatar(user: generic_commit_status.pipeline.user, size: 20)
       - else
-        %span.api API
+        %span.monospace API
 
   - if admin
     %td

app/views/projects/merge_requests/creations/_new_compare.html.haml

@@ -21,7 +21,7 @@
                   selected: f.object.source_project_id
           .merge-request-select.dropdown
             = f.hidden_field :source_branch
-            = dropdown_toggle f.object.source_branch || _("Select source branch"), { toggle: "dropdown", 'field-name': "#{f.object_name}[source_branch]", 'refs-url': refs_project_path(@source_project), selected: f.object.source_branch }, { toggle_class: "js-compare-dropdown js-source-branch git-revision-dropdown-toggle" }
+            = dropdown_toggle f.object.source_branch || _("Select source branch"), { toggle: "dropdown", 'field-name': "#{f.object_name}[source_branch]", 'refs-url': refs_project_path(@source_project), selected: f.object.source_branch }, { toggle_class: "js-compare-dropdown js-source-branch monospace" }
             .dropdown-menu.dropdown-menu-selectable.js-source-branch-dropdown.git-revision-dropdown
               = dropdown_title(_("Select source branch"))
               = dropdown_filter(_("Search branches"))
@@ -49,7 +49,7 @@
                   selected: f.object.target_project_id
           .merge-request-select.dropdown
             = f.hidden_field :target_branch
-            = dropdown_toggle f.object.target_branch, { toggle: "dropdown", 'field-name': "#{f.object_name}[target_branch]", 'refs-url': refs_project_path(f.object.target_project), selected: f.object.target_branch }, { toggle_class: "js-compare-dropdown js-target-branch git-revision-dropdown-toggle" }
+            = dropdown_toggle f.object.target_branch, { toggle: "dropdown", 'field-name': "#{f.object_name}[target_branch]", 'refs-url': refs_project_path(f.object.target_project), selected: f.object.target_branch }, { toggle_class: "js-compare-dropdown js-target-branch monospace" }
             .dropdown-menu.dropdown-menu-selectable.js-target-branch-dropdown.git-revision-dropdown
               = dropdown_title(_("Select target branch"))
               = dropdown_filter(_("Search branches"))

app/views/projects/pipelines/new.html.haml

@@ -13,7 +13,7 @@
       = f.label :ref, s_('Pipeline|Create for'), class: 'col-form-label'
       = hidden_field_tag 'pipeline[ref]', params[:ref] || @project.default_branch
       = dropdown_tag(params[:ref] || @project.default_branch,
-                     options: { toggle_class: 'js-branch-select wide git-revision-dropdown-toggle',
+                     options: { toggle_class: 'js-branch-select wide monospace',
                                 filter: true, dropdown_class: "dropdown-menu-selectable git-revision-dropdown", placeholder: s_("Pipeline|Search branches"),
                                 data: { selected: params[:ref] || @project.default_branch, field_name: 'pipeline[ref]' } })
       .form-text.text-muted

app/views/projects/protected_branches/shared/_dropdown.html.haml

 = f.hidden_field(:name)
 
 = dropdown_tag('Select branch or create wildcard',
-               options: { toggle_class: 'js-protected-branch-select js-filter-submit wide git-revision-dropdown-toggle qa-protected-branch-select',
+               options: { toggle_class: 'js-protected-branch-select js-filter-submit wide monospace qa-protected-branch-select',
                           filter: true, dropdown_class: "dropdown-menu-selectable git-revision-dropdown qa-protected-branch-dropdown", placeholder: "Search protected branches",
                           footer_content: true,
                           data: { show_no: true, show_any: true, show_upcoming: true,

app/views/projects/protected_tags/shared/_dropdown.html.haml

 = f.hidden_field(:name)
 
 = dropdown_tag('Select tag or create wildcard',
-               options: { toggle_class: 'js-protected-tag-select js-filter-submit wide git-revision-dropdown-toggle',
+               options: { toggle_class: 'js-protected-tag-select js-filter-submit wide monospace',
                           filter: true, dropdown_class: "dropdown-menu-selectable capitalize-header git-revision-dropdown", placeholder: "Search protected tags",
                           footer_content: true,
                           data: { show_no: true, show_any: true, show_upcoming: true,

changelogs/unreleased/54032-reply-shortcut-only-discussion-text.yml

+---
+title: Make reply shortcut only quote selected discussion text
+merge_request: 23096
+author: Thomas Pathier
+type: fix

changelogs/unreleased/gt-remove-instances-of-extend-monospace.yml

+---
+title: Remove monospace extend
+merge_request: 23089
+author: George Tsiolis
+type: performance

changelogs/unreleased/ide-open-all-mr-files.yml

+---
+title: Open first 10 merge request files in IDE
+merge_request:
+author:
+type: fixed

locale/gitlab.pot

@@ -4517,6 +4517,12 @@ msgstr ""
 msgid "Invite"
 msgstr ""
 
+msgid "Invoke Count"
+msgstr ""
+
+msgid "Invoke Time"
+msgstr ""
+
 msgid "Issue"
 msgstr ""
 

spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js

+/* eslint-disable
+  no-underscore-dangle
+*/
+
 import $ from 'jquery';
 import initCopyAsGFM from '~/behaviors/markdown/copy_as_gfm';
 import ShortcutsIssuable from '~/behaviors/shortcuts/shortcuts_issuable';
@@ -27,13 +31,17 @@ describe('ShortcutsIssuable', function() {
 
   describe('replyWithSelectedText', () => {
     // Stub window.gl.utils.getSelectedFragment to return a node with the provided HTML.
-    const stubSelection = html => {
-      window.gl.utils.getSelectedFragment = () => {
+    const stubSelection = (html, invalidNode) => {
+      ShortcutsIssuable.__Rewire__('getSelectedFragment', () => {
+        const documentFragment = document.createDocumentFragment();
         const node = document.createElement('div');
+
         node.innerHTML = html;
+        if (!invalidNode) node.className = 'md';
 
-        return node;
-      };
+        documentFragment.appendChild(node);
+        return documentFragment;
+      });
     };
     describe('with empty selection', () => {
       it('does not return an error', () => {
@@ -105,5 +113,133 @@ describe('ShortcutsIssuable', function() {
         );
       });
     });
+
+    describe('with an invalid selection', () => {
+      beforeEach(() => {
+        stubSelection('<p>Selected text.</p>', true);
+      });
+
+      it('does not add anything to the input', () => {
+        ShortcutsIssuable.replyWithSelectedText(true);
+
+        expect($(FORM_SELECTOR).val()).toBe('');
+      });
+
+      it('triggers `focus`', () => {
+        const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus');
+        ShortcutsIssuable.replyWithSelectedText(true);
+
+        expect(spy).toHaveBeenCalled();
+      });
+    });
+
+    describe('with a semi-valid selection', () => {
+      beforeEach(() => {
+        stubSelection('<div class="md">Selected text.</div><p>Invalid selected text.</p>', true);
+      });
+
+      it('only adds the valid part to the input', () => {
+        ShortcutsIssuable.replyWithSelectedText(true);
+
+        expect($(FORM_SELECTOR).val()).toBe('> Selected text.\n\n');
+      });
+
+      it('triggers `focus`', () => {
+        const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus');
+        ShortcutsIssuable.replyWithSelectedText(true);
+
+        expect(spy).toHaveBeenCalled();
+      });
+
+      it('triggers `input`', () => {
+        let triggered = false;
+        $(FORM_SELECTOR).on('input', () => {
+          triggered = true;
+        });
+
+        ShortcutsIssuable.replyWithSelectedText(true);
+
+        expect(triggered).toBe(true);
+      });
+    });
+
+    describe('with a selection in a valid block', () => {
+      beforeEach(() => {
+        ShortcutsIssuable.__Rewire__('getSelectedFragment', () => {
+          const documentFragment = document.createDocumentFragment();
+          const node = document.createElement('div');
+          const originalNode = document.createElement('body');
+          originalNode.innerHTML = `<div class="issue">
+            <div class="otherElem">Text...</div>
+            <div class="md"><p><em>Selected text.</em></p></div>
+          </div>`;
+          documentFragment.originalNodes = [originalNode.querySelector('em')];
+
+          node.innerHTML = '<em>Selected text.</em>';
+
+          documentFragment.appendChild(node);
+
+          return documentFragment;
+        });
+      });
+
+      it('adds the quoted selection to the input', () => {
+        ShortcutsIssuable.replyWithSelectedText(true);
+
+        expect($(FORM_SELECTOR).val()).toBe('> _Selected text._\n\n');
+      });
+
+      it('triggers `focus`', () => {
+        const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus');
+        ShortcutsIssuable.replyWithSelectedText(true);
+
+        expect(spy).toHaveBeenCalled();
+      });
+
+      it('triggers `input`', () => {
+        let triggered = false;
+        $(FORM_SELECTOR).on('input', () => {
+          triggered = true;
+        });
+
+        ShortcutsIssuable.replyWithSelectedText(true);
+
+        expect(triggered).toBe(true);
+      });
+    });
+
+    describe('with a selection in an invalid block', () => {
+      beforeEach(() => {
+        ShortcutsIssuable.__Rewire__('getSelectedFragment', () => {
+          const documentFragment = document.createDocumentFragment();
+          const node = document.createElement('div');
+          const originalNode = document.createElement('body');
+          originalNode.innerHTML = `<div class="issue">
+            <div class="otherElem"><div><b>Selected text.</b></div></div>
+            <div class="md"><p><em>Valid text</em></p></div>
+          </div>`;
+          documentFragment.originalNodes = [originalNode.querySelector('b')];
+
+          node.innerHTML = '<b>Selected text.</b>';
+
+          documentFragment.appendChild(node);
+
+          return documentFragment;
+        });
+      });
+
+      it('does not add anything to the input', () => {
+        ShortcutsIssuable.replyWithSelectedText(true);
+
+        expect($(FORM_SELECTOR).val()).toBe('');
+      });
+
+      it('triggers `focus`', () => {
+        const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus');
+        ShortcutsIssuable.replyWithSelectedText(true);
+
+        expect(spy).toHaveBeenCalled();
+      });
+    });
   });
 });

spec/javascripts/ide/stores/actions/merge_request_spec.js

@@ -262,16 +262,28 @@ describe('IDE store merge request actions', () => {
         bar: {},
       };
 
-      spyOn(store, 'dispatch').and.callFake(type => {
+      const originalDispatch = store.dispatch;
+
+      spyOn(store, 'dispatch').and.callFake((type, payload) => {
         switch (type) {
           case 'getMergeRequestData':
             return Promise.resolve(testMergeRequest);
           case 'getMergeRequestChanges':
             return Promise.resolve(testMergeRequestChanges);
-          default:
+          case 'getFiles':
+          case 'getMergeRequestVersions':
+          case 'getBranchData':
+          case 'setFileMrChange':
             return Promise.resolve();
+          default:
+            return originalDispatch(type, payload);
         }
       });
+      spyOn(service, 'getFileData').and.callFake(() =>
+        Promise.resolve({
+          headers: {},
+        }),
+      );
     });
 
     it('dispatch actions for merge request data', done => {
@@ -303,7 +315,17 @@ describe('IDE store merge request actions', () => {
     });
 
     it('updates activity bar view and gets file data, if changes are found', done => {
-      testMergeRequestChanges.changes = [{ new_path: 'foo' }, { new_path: 'bar' }];
+      store.state.entries.foo = {
+        url: 'test',
+      };
+      store.state.entries.bar = {
+        url: 'test',
+      };
+
+      testMergeRequestChanges.changes = [
+        { new_path: 'foo', path: 'foo' },
+        { new_path: 'bar', path: 'bar' },
+      ];
 
       openMergeRequest(store, mr)
         .then(() => {
@@ -321,8 +343,11 @@ describe('IDE store merge request actions', () => {
             expect(store.dispatch).toHaveBeenCalledWith('getFileData', {
               path: change.new_path,
               makeFileActive: i === 0,
+              openFile: true,
             });
           });
+
+          expect(store.state.openFiles.length).toBe(testMergeRequestChanges.changes.length);
         })
         .then(done)
         .catch(done.fail);

spec/support/helpers/features/branches_helpers.rb

@@ -20,7 +20,7 @@ module Spec
           end
 
           def select_branch(branch_name)
-            find(".git-revision-dropdown-toggle").click
+            find(".js-branch-select").click
 
             page.within("#new-branch-form .dropdown-menu") do
               click_link(branch_name)