Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-09-20

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

 <script>
-import { mapGetters } from 'vuex';
+import { mapGetters, mapActions } from 'vuex';
 import DiffTableCell from './diff_table_cell.vue';
 import {
   NEW_LINE_TYPE,
@@ -63,7 +63,11 @@ export default {
     this.linePositionLeft = LINE_POSITION_LEFT;
     this.linePositionRight = LINE_POSITION_RIGHT;
   },
+  mounted() {
+    this.scrollToLineIfNeededInline(this.line);
+  },
   methods: {
+    ...mapActions('diffs', ['scrollToLineIfNeededInline']),
     handleMouseMove(e) {
       // To show the comment icon on the gutter we need to know if we hover the line.
       // Current table structure doesn't allow us to do this with CSS in both of the diff view types

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

 <script>
+import { mapActions } from 'vuex';
 import $ from 'jquery';
 import DiffTableCell from './diff_table_cell.vue';
 import {
@@ -64,7 +65,11 @@ export default {
     this.oldLineType = OLD_LINE_TYPE;
     this.parallelDiffViewType = PARALLEL_DIFF_VIEW_TYPE;
   },
+  mounted() {
+    this.scrollToLineIfNeededParallel(this.line);
+  },
   methods: {
+    ...mapActions('diffs', ['scrollToLineIfNeededParallel']),
     handleMouseMove(e) {
       const isHover = e.type === 'mouseover';
       const hoveringCell = e.target.closest('td');

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

@@ -2,7 +2,7 @@ import Vue from 'vue';
 import axios from '~/lib/utils/axios_utils';
 import Cookies from 'js-cookie';
 import { handleLocationHash, historyPushState } from '~/lib/utils/common_utils';
-import { mergeUrlParams } from '~/lib/utils/url_utility';
+import { mergeUrlParams, getLocationHash } from '~/lib/utils/url_utility';
 import { getDiffPositionByLineCode } from './utils';
 import * as types from './mutation_types';
 import {
@@ -120,6 +120,25 @@ export const loadMoreLines = ({ commit }, options) => {
   });
 };
 
+export const scrollToLineIfNeededInline = (_, line) => {
+  const hash = getLocationHash();
+
+  if (hash && line.lineCode === hash) {
+    handleLocationHash();
+  }
+};
+
+export const scrollToLineIfNeededParallel = (_, line) => {
+  const hash = getLocationHash();
+
+  if (
+    hash &&
+    ((line.left && line.left.lineCode === hash) || (line.right && line.right.lineCode === hash))
+  ) {
+    handleLocationHash();
+  }
+};
+
 export const loadCollapsedDiff = ({ commit }, file) =>
   axios.get(file.loadCollapsedDiffUrl).then(res => {
     commit(types.ADD_COLLAPSED_DIFFS, {

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

@@ -87,6 +87,7 @@ export const handleLocationHash = () => {
   const fixedTabs = document.querySelector('.js-tabs-affix');
   const fixedDiffStats = document.querySelector('.js-diff-files-changed');
   const fixedNav = document.querySelector('.navbar-gitlab');
+  const performanceBar = document.querySelector('#js-peek');
 
   let adjustment = 0;
   if (fixedNav) adjustment -= fixedNav.offsetHeight;
@@ -103,6 +104,10 @@ export const handleLocationHash = () => {
     adjustment -= fixedDiffStats.offsetHeight;
   }
 
+  if (performanceBar) {
+    adjustment -= performanceBar.offsetHeight;
+  }
+
   window.scrollBy(0, adjustment);
 };
 
@@ -132,21 +137,20 @@ export const parseUrlPathname = url => {
   return parsedUrl.pathname.charAt(0) === '/' ? parsedUrl.pathname : `/${parsedUrl.pathname}`;
 };
 
-const splitPath = (path = '') => path
-  .replace(/^\?/, '')
-  .split('&');
+const splitPath = (path = '') => path.replace(/^\?/, '').split('&');
 
-export const urlParamsToArray = (path = '') => splitPath(path)
-  .filter(param => param.length > 0)
-  .map(param => {
-    const split = param.split('=');
-    return [decodeURI(split[0]), split[1]].join('=');
-  });
+export const urlParamsToArray = (path = '') =>
+  splitPath(path)
+    .filter(param => param.length > 0)
+    .map(param => {
+      const split = param.split('=');
+      return [decodeURI(split[0]), split[1]].join('=');
+    });
 
 export const getUrlParamsArray = () => urlParamsToArray(window.location.search);
 
-export const urlParamsToObject = (path = '') => splitPath(path)
-  .reduce((dataParam, filterParam) => {
+export const urlParamsToObject = (path = '') =>
+  splitPath(path).reduce((dataParam, filterParam) => {
     if (filterParam === '') {
       return dataParam;
     }
@@ -217,7 +221,7 @@ export const getParameterByName = (name, urlToParse) => {
   return decodeURIComponent(results[2].replace(/\+/g, ' '));
 };
 
-const handleSelectedRange = (range) => {
+const handleSelectedRange = range => {
   const container = range.commonAncestorContainer;
   // add context to fragment if needed
   if (container.tagName === 'OL') {
@@ -514,7 +518,7 @@ export const setFaviconOverlay = overlayPath => {
   );
 };
 
-export const setFavicon = (faviconPath) => {
+export const setFavicon = faviconPath => {
   const faviconEl = document.getElementById('favicon');
   if (faviconEl && faviconPath) {
     faviconEl.setAttribute('href', faviconPath);
@@ -539,7 +543,7 @@ export const setCiStatusFavicon = pageUrl =>
       }
       return resetFavicon();
     })
-    .catch((error) => {
+    .catch(error => {
       resetFavicon();
       throw error;
     });

doc/development/documentation/index.md

@@ -411,6 +411,22 @@ The following GitLab features are used among others:
 Every GitLab instance includes the documentation, which is available from `/help`
 (`http://my-instance.com/help`), e.g., <https://gitlab.com/help>.
 
+The documentation available online on docs.gitlab.com is continuously
+deployed every hour from the `master` branch of CE, EE, Omnibus, and Runner. Therefore,
+once a merge request gets merged, it will be available online on the same day,
+but they will be shipped (and available on `/help`) within the milestone assigned
+to the MR.
+
+For instance, let's say your merge request has a milestone set to 11.3, which
+will be released on 2018-09-22. If it gets merged on 2018-09-15, it will be
+available online on 2018-09-15, but, as the feature freeze date has passed, if
+the MR does not have a "pick into 11.3" label, the milestone has to be changed
+to 11.4 and it will be shipped with all GitLab packages only on 2018-10-22,
+with GitLab 11.4. Meaning, it will only be available under `/help` from GitLab
+11.4 onwards, but available on docs.gitlab.com on the same day it was merged.
+
+### Linking to `/help`
+
 When you're building a new feature, you may need to link the documentation
 from GitLab, the application. This is normally done in files inside the
 `app/views/` directory with the help of the `help_page_path` helper method.

doc/development/testing_guide/index.md

@@ -59,6 +59,12 @@ parallelization, monitoring.
 
 ---
 
+## [Review apps](review_apps.md)
+
+How review apps are set up for GitLab CE/EE and how to use them.
+
+---
+
 ## [Testing Rake tasks](testing_rake_tasks.md)
 
 Everything you should know about how to test Rake tasks.

doc/development/testing_guide/review_apps.md

+# Review apps
+
+We currently have review apps available as a manual job in EE pipelines. Here is
+[the first implementation](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/6259).
+
+That said, [the Quality team is working](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/6665)
+on making Review Apps automatically deployed by each pipeline, both in CE and EE.
+
+## How does it work?
+
+1. On every EE [pipeline][gitlab-pipeline] during the `test` stage, you can
+  start the [`review` job][review-job]
+1. The `review` job [triggers a pipeline][cng-pipeline] in the
+  [`CNG-mirror`][cng-mirror] [^1] project
+1. The `CNG-mirror` pipeline creates the Docker images of each component (e.g. `gitlab-rails-ee`,
+  `gitlab-shell`, `gitaly` etc.) based on the commit from the
+  [GitLab pipeline][gitlab-pipeline] and store them in its
+  [registry][cng-mirror-registry]
+1. Once all images are built, the review app is deployed using
+  [the official GitLab Helm chart][helm-chart] [^2] to the
+  [`review-apps-ee` Kubernetes cluster on GCP][review-apps-ee]
+  - The actual scripts used to deploy the review app can be found at
+    [`scripts/review_apps/review-apps.sh`][review-apps.sh]
+  - These scripts are basically
+    [our official Auto DevOps scripts][Auto-DevOps.gitlab-ci.yml] where the
+    default CNG images are overriden with the images built and stored in the
+    [`CNG-mirror` project's registry][cng-mirror-registry]
+1. Once the `review` job succeeds, you should be able to use your review app
+  thanks to the direct link to it from the MR widget. The default username is
+  `root` and its password can be found in the 1Password secure note named
+  **gitlab-{ce,ee} review app's root password**.
+
+**Additional notes:**
+
+- The Kubernetes cluster is connected to the `gitlab-ee` project using [GitLab's
+  Kubernetes integration][gitlab-k8s-integration]. This basically allows to have
+  a link to the review app directly from the merge request widget.
+- The manual `stop_review` in the `post-cleanup` stage can be used to stop a
+  review app manually, and is also started by GitLab once a branch is deleted
+- [TBD] Review apps are cleaned up regularly using a pipeline schedule that runs
+  the [`scripts/review_apps/automated_cleanup.rb`][automated_cleanup.rb] script
+
+[^1]: We use the `CNG-mirror` project so that the `CNG`, (**C**loud **N**ative **G**itLab), project's registry is
+  not overloaded with a lot of transient Docker images.
+[^2]: Since we're using [the official GitLab Helm chart][helm-chart], this means
+  you get the a dedicated environment for your branch that's very close to what it
+  would look in production
+
+## Frequently Asked Questions
+
+**Will it be too much to trigger CNG image builds on every test run? This could create thousands of unused docker images.**
+
+  > We have to start somewhere and improve later. If we see this getting out of hand, we will revisit.
+
+**How big is the Kubernetes cluster?**
+
+  > The cluster is currently setup with a single pool of preemptible
+    nodes, with a minimum of 1 node and a maximum of 30 nodes.
+
+**What are the machine running on the cluster?**
+
+  > We're currently using `n1-standard-4` (4 vCPUs, 15 GB memory) machines.
+
+**How do we secure this from abuse? Apps are open to the world so we need to find a way to limit it to only us.**
+
+  > This won't work for forks. We will add a root password to 1password shared vault.
+
+[gitlab-pipeline]: https://gitlab.com/gitlab-org/gitlab-ee/pipelines/29302122
+[review-job]: https://gitlab.com/gitlab-org/gitlab-ee/-/jobs/94294136
+[cng-mirror]: https://gitlab.com/gitlab-org/build/CNG-mirror
+[cng-pipeline]: https://gitlab.com/gitlab-org/build/CNG-mirror/pipelines/29307727
+[cng-mirror-registry]: https://gitlab.com/gitlab-org/build/CNG-mirror/container_registry
+[helm-chart]: https://gitlab.com/charts/gitlab/
+[review-apps-ee]: https://console.cloud.google.com/kubernetes/clusters/details/us-central1-b/review-apps-ee?project=gitlab-review-apps
+[review-apps.sh]: https://gitlab.com/gitlab-org/gitlab-ee/blob/master/scripts/review_apps/review-apps.sh
+[automated_cleanup.rb]: https://gitlab.com/gitlab-org/gitlab-ee/blob/master/scripts/review_apps/automated_cleanup.rb
+[Auto-DevOps.gitlab-ci.yml]: https://gitlab.com/gitlab-org/gitlab-ci-yml/blob/master/Auto-DevOps.gitlab-ci.yml
+[gitlab-k8s-integration]: https://docs.gitlab.com/ee/user/project/clusters/index.html
+
+---
+
+[Return to Testing documentation](index.md)

doc/update/9.4-to-9.5.md

@@ -154,7 +154,7 @@ sudo -u git -H make
 
 #### New Gitaly configuration options required
 
-In order to function Gitaly needs some additional configuration information. Below we assume you installed Gitaly in `/home/git/gitaly` and GitLab Shell in `/home/git/gitlab-shell'.
+In order to function Gitaly needs some additional configuration information. Below we assume you installed Gitaly in `/home/git/gitaly` and GitLab Shell in `/home/git/gitlab-shell`.
 
 ```shell
 echo '

doc/update/9.5-to-10.0.md

@@ -154,7 +154,7 @@ sudo -u git -H make
 
 #### New Gitaly configuration options required
 
-In order to function Gitaly needs some additional configuration information. Below we assume you installed Gitaly in `/home/git/gitaly` and GitLab Shell in `/home/git/gitlab-shell'.
+In order to function Gitaly needs some additional configuration information. Below we assume you installed Gitaly in `/home/git/gitaly` and GitLab Shell in `/home/git/gitlab-shell`.
 
 ```shell
 echo '

spec/javascripts/diffs/store/actions_spec.js

@@ -5,7 +5,23 @@ import {
   INLINE_DIFF_VIEW_TYPE,
   PARALLEL_DIFF_VIEW_TYPE,
 } from '~/diffs/constants';
-import * as actions from '~/diffs/store/actions';
+import actions, {
+  setBaseConfig,
+  fetchDiffFiles,
+  assignDiscussionsToDiff,
+  removeDiscussionsFromDiff,
+  startRenderDiffsQueue,
+  setInlineDiffViewType,
+  setParallelDiffViewType,
+  showCommentForm,
+  cancelCommentForm,
+  loadMoreLines,
+  scrollToLineIfNeededInline,
+  scrollToLineIfNeededParallel,
+  loadCollapsedDiff,
+  expandAllFiles,
+  toggleFileDiscussions,
+} from '~/diffs/store/actions';
 import * as types from '~/diffs/store/mutation_types';
 import { reduceDiscussionsToLineCodes } from '~/notes/stores/utils';
 import axios from '~/lib/utils/axios_utils';
@@ -37,7 +53,7 @@ describe('DiffsStoreActions', () => {
       const projectPath = '/root/project';
 
       testAction(
-        actions.setBaseConfig,
+        setBaseConfig,
         { endpoint, projectPath },
         { endpoint: '', projectPath: '' },
         [{ type: types.SET_BASE_CONFIG, payload: { endpoint, projectPath } }],
@@ -55,7 +71,7 @@ describe('DiffsStoreActions', () => {
       mock.onGet(endpoint).reply(200, res);
 
       testAction(
-        actions.fetchDiffFiles,
+        fetchDiffFiles,
         {},
         { endpoint },
         [
@@ -139,7 +155,7 @@ describe('DiffsStoreActions', () => {
       const discussions = reduceDiscussionsToLineCodes([singleDiscussion]);
 
       testAction(
-        actions.assignDiscussionsToDiff,
+        assignDiscussionsToDiff,
         discussions,
         state,
         [
@@ -208,7 +224,7 @@ describe('DiffsStoreActions', () => {
       };
 
       testAction(
-        actions.removeDiscussionsFromDiff,
+        removeDiscussionsFromDiff,
         singleDiscussion,
         state,
         [
@@ -252,8 +268,7 @@ describe('DiffsStoreActions', () => {
         });
       };
 
-      actions
-        .startRenderDiffsQueue({ state, commit: pseudoCommit })
+      startRenderDiffsQueue({ state, commit: pseudoCommit })
         .then(() => {
           expect(state.diffFiles[0].renderIt).toBeTruthy();
           expect(state.diffFiles[1].renderIt).toBeTruthy();
@@ -269,7 +284,7 @@ describe('DiffsStoreActions', () => {
   describe('setInlineDiffViewType', () => {
     it('should set diff view type to inline and also set the cookie properly', done => {
       testAction(
-        actions.setInlineDiffViewType,
+        setInlineDiffViewType,
         null,
         {},
         [{ type: types.SET_DIFF_VIEW_TYPE, payload: INLINE_DIFF_VIEW_TYPE }],
@@ -287,7 +302,7 @@ describe('DiffsStoreActions', () => {
   describe('setParallelDiffViewType', () => {
     it('should set diff view type to parallel and also set the cookie properly', done => {
       testAction(
-        actions.setParallelDiffViewType,
+        setParallelDiffViewType,
         null,
         {},
         [{ type: types.SET_DIFF_VIEW_TYPE, payload: PARALLEL_DIFF_VIEW_TYPE }],
@@ -307,7 +322,7 @@ describe('DiffsStoreActions', () => {
       const payload = { lineCode: 'lineCode' };
 
       testAction(
-        actions.showCommentForm,
+        showCommentForm,
         payload,
         {},
         [{ type: types.ADD_COMMENT_FORM_LINE, payload }],
@@ -322,7 +337,7 @@ describe('DiffsStoreActions', () => {
       const payload = { lineCode: 'lineCode' };
 
       testAction(
-        actions.cancelCommentForm,
+        cancelCommentForm,
         payload,
         {},
         [{ type: types.REMOVE_COMMENT_FORM_LINE, payload }],
@@ -344,7 +359,7 @@ describe('DiffsStoreActions', () => {
       mock.onGet(endpoint).reply(200, contextLines);
 
       testAction(
-        actions.loadMoreLines,
+        loadMoreLines,
         options,
         {},
         [
@@ -370,7 +385,7 @@ describe('DiffsStoreActions', () => {
       mock.onGet(file.loadCollapsedDiffUrl).reply(200, data);
 
       testAction(
-        actions.loadCollapsedDiff,
+        loadCollapsedDiff,
         file,
         {},
         [
@@ -391,7 +406,7 @@ describe('DiffsStoreActions', () => {
   describe('expandAllFiles', () => {
     it('should change the collapsed prop from the diffFiles', done => {
       testAction(
-        actions.expandAllFiles,
+        expandAllFiles,
         null,
         {},
         [
@@ -415,7 +430,7 @@ describe('DiffsStoreActions', () => {
 
       const dispatch = jasmine.createSpy('dispatch');
 
-      actions.toggleFileDiscussions({ getters, dispatch });
+      toggleFileDiscussions({ getters, dispatch });
 
       expect(dispatch).toHaveBeenCalledWith(
         'collapseDiscussion',
@@ -433,7 +448,7 @@ describe('DiffsStoreActions', () => {
 
       const dispatch = jasmine.createSpy();
 
-      actions.toggleFileDiscussions({ getters, dispatch });
+      toggleFileDiscussions({ getters, dispatch });
 
       expect(dispatch).toHaveBeenCalledWith(
         'expandDiscussion',
@@ -451,7 +466,7 @@ describe('DiffsStoreActions', () => {
 
       const dispatch = jasmine.createSpy();
 
-      actions.toggleFileDiscussions({ getters, dispatch });
+      toggleFileDiscussions({ getters, dispatch });
 
       expect(dispatch).toHaveBeenCalledWith(
         'expandDiscussion',
@@ -460,4 +475,111 @@ describe('DiffsStoreActions', () => {
       );
     });
   });
+
+  describe('scrollToLineIfNeededInline', () => {
+    const lineMock = {
+      lineCode: 'ABC_123',
+    };
+
+    it('should not call handleLocationHash when there is not hash', () => {
+      window.location.hash = '';
+
+      const handleLocationHashSpy = spyOnDependency(actions, 'handleLocationHash').and.stub();
+
+      scrollToLineIfNeededInline({}, lineMock);
+
+      expect(handleLocationHashSpy).not.toHaveBeenCalled();
+    });
+
+    it('should not call handleLocationHash when the hash does not match any line', () => {
+      window.location.hash = 'XYZ_456';
+
+      const handleLocationHashSpy = spyOnDependency(actions, 'handleLocationHash').and.stub();
+
+      scrollToLineIfNeededInline({}, lineMock);
+
+      expect(handleLocationHashSpy).not.toHaveBeenCalled();
+    });
+
+    it('should call handleLocationHash only when the hash matches a line', () => {
+      window.location.hash = 'ABC_123';
+
+      const handleLocationHashSpy = spyOnDependency(actions, 'handleLocationHash').and.stub();
+
+      scrollToLineIfNeededInline(
+        {},
+        {
+          lineCode: 'ABC_456',
+        },
+      );
+      scrollToLineIfNeededInline({}, lineMock);
+      scrollToLineIfNeededInline(
+        {},
+        {
+          lineCode: 'XYZ_456',
+        },
+      );
+
+      expect(handleLocationHashSpy).toHaveBeenCalled();
+      expect(handleLocationHashSpy).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  describe('scrollToLineIfNeededParallel', () => {
+    const lineMock = {
+      left: null,
+      right: {
+        lineCode: 'ABC_123',
+      },
+    };
+
+    it('should not call handleLocationHash when there is not hash', () => {
+      window.location.hash = '';
+
+      const handleLocationHashSpy = spyOnDependency(actions, 'handleLocationHash').and.stub();
+
+      scrollToLineIfNeededParallel({}, lineMock);
+
+      expect(handleLocationHashSpy).not.toHaveBeenCalled();
+    });
+
+    it('should not call handleLocationHash when the hash does not match any line', () => {
+      window.location.hash = 'XYZ_456';
+
+      const handleLocationHashSpy = spyOnDependency(actions, 'handleLocationHash').and.stub();
+
+      scrollToLineIfNeededParallel({}, lineMock);
+
+      expect(handleLocationHashSpy).not.toHaveBeenCalled();
+    });
+
+    it('should call handleLocationHash only when the hash matches a line', () => {
+      window.location.hash = 'ABC_123';
+
+      const handleLocationHashSpy = spyOnDependency(actions, 'handleLocationHash').and.stub();
+
+      scrollToLineIfNeededParallel(
+        {},
+        {
+          left: null,
+          right: {
+            lineCode: 'ABC_456',
+          },
+        },
+      );
+      scrollToLineIfNeededParallel({}, lineMock);
+      scrollToLineIfNeededParallel(
+        {},
+        {
+          left: null,
+          right: {
+            lineCode: 'XYZ_456',
+          },
+        },
+      );
+
+      expect(handleLocationHashSpy).toHaveBeenCalled();
+      expect(handleLocationHashSpy).toHaveBeenCalledTimes(1);
+    });
+  });
 });