Merge branch 'ce-to-ee-2018-04-04' into 'master'

CE upstream - 2018-04-04 15:28 UTC

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

.gitlab-ci.yml

@@ -708,7 +708,8 @@ karma:
 
 codequality:
   <<: *dedicated-no-docs-no-db-pull-cache-job
-  image: docker:latest
+  image: docker:stable
+  allow_failure: true
   # gitlab-org runners set `privileged: false` but we need to have it set to true
   # since we're using Docker in Docker
   tags: []
@@ -718,14 +719,15 @@ codequality:
   variables:
     SETUP_DB: "false"
     DOCKER_DRIVER: overlay2
-    CODECLIMATE_FORMAT: json
   cache: {}
   dependencies: []
   script:
-    - apk update && apk add jq
-    - ./scripts/codequality analyze -f json > raw_codeclimate.json || true
-    # The following line keeps only the fields used in the MR widget, reducing the JSON artifact size
-    - jq -c 'map({check_name,description,fingerprint,location})' raw_codeclimate.json > codeclimate.json
+    # Get the custom rubocop codeclimate image (https://gitlab.com/gitlab-org/codeclimate-rubocop/wikis/home)
+    - docker pull dev.gitlab.org:5005/gitlab/gitlab-build-images:gitlab-codeclimate-rubocop-0-52-1
+    - docker tag dev.gitlab.org:5005/gitlab/gitlab-build-images:gitlab-codeclimate-rubocop-0-52-1 codeclimate/codeclimate-rubocop:gitlab-codeclimate-rubocop-0-52-1
+    # Extract "MAJOR.MINOR" from CI_SERVER_VERSION and generate "MAJOR-MINOR-stable" for Security Products
+    - export SP_VERSION=$(echo "$CI_SERVER_VERSION" | sed 's/^\([0-9]*\)\.\([0-9]*\).*/\1-\2-stable/')
+    - docker run --env SOURCE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock "registry.gitlab.com/gitlab-org/security-products/codequality:$SP_VERSION" /code
   artifacts:
     paths: [codeclimate.json]
     expire_in: 1 week

CHANGELOG.md

@@ -192,7 +192,6 @@ entry.
 - Enable privileged mode for GitLab Runner. !17528
 - Expose GITLAB_FEATURES as CI/CD variable (fixes #40994).
 - Upgrade GitLab Workhorse to 4.0.0.
-- Allow CI/CD Jobs being grouped on version strings.
 - Add discussions API for Issues and Snippets.
 - Add one group board to Libre.
 - Add support for filtering by source and target branch to merge requests API.

app/assets/javascripts/ide/components/commit_sidebar/list_item.vue

 <script>
-  import { mapActions } from 'vuex';
-  import icon from '~/vue_shared/components/icon.vue';
-  import router from '../../ide_router';
+import { mapActions } from 'vuex';
+import Icon from '~/vue_shared/components/icon.vue';
 
-  export default {
+export default {
   components: {
-      icon,
+    Icon,
   },
   props: {
     file: {
@@ -22,17 +21,16 @@
     },
   },
   methods: {
-      ...mapActions([
-        'discardFileChanges',
-        'updateViewer',
-      ]),
+    ...mapActions(['discardFileChanges', 'updateViewer', 'openPendingTab']),
     openFileInEditor(file) {
+      return this.openPendingTab(file).then(changeViewer => {
+        if (changeViewer) {
           this.updateViewer('diff');
-
-        router.push(`/project${file.url}`);
+        }
+      });
     },
   },
-  };
+};
 </script>
 
 <template>

app/assets/javascripts/ide/components/ide.vue

@@ -60,6 +60,7 @@ export default {
         v-if="activeFile"
       >
         <repo-tabs
+          :active-file="activeFile"
           :files="openFiles"
           :viewer="viewer"
           :has-changes="hasChanges"

app/assets/javascripts/ide/components/repo_editor.vue

@@ -21,7 +21,8 @@ export default {
   },
   watch: {
     file(oldVal, newVal) {
-      if (newVal.path !== this.file.path) {
+      // Compare key to allow for files opened in review mode to be cached differently
+      if (newVal.key !== this.file.key) {
         this.initMonaco();
       }
     },
@@ -70,7 +71,7 @@ export default {
       })
         .then(() => {
           const viewerPromise = this.delayViewerUpdated
-            ? this.updateViewer('editor')
+            ? this.updateViewer(this.file.pending ? 'diff' : 'editor')
             : Promise.resolve();
 
           return viewerPromise;

app/assets/javascripts/ide/components/repo_file.vue

@@ -62,11 +62,7 @@ export default {
         this.toggleTreeOpen(this.file.path);
       }
 
-      const delayPromise = this.file.changed
-        ? Promise.resolve()
-        : this.updateDelayViewerUpdated(true);
-
-      return delayPromise.then(() => {
+      return this.updateDelayViewerUpdated(true).then(() => {
         router.push(`/project${this.file.url}`);
       });
     },

app/assets/javascripts/ide/components/repo_tab.vue

 <script>
-  import { mapActions } from 'vuex';
+import { mapActions } from 'vuex';
 
-  import fileIcon from '~/vue_shared/components/file_icon.vue';
-  import icon from '~/vue_shared/components/icon.vue';
-  import fileStatusIcon from './repo_file_status_icon.vue';
-  import changedFileIcon from './changed_file_icon.vue';
+import FileIcon from '~/vue_shared/components/file_icon.vue';
+import Icon from '~/vue_shared/components/icon.vue';
+import FileStatusIcon from './repo_file_status_icon.vue';
+import ChangedFileIcon from './changed_file_icon.vue';
 
-  export default {
+export default {
   components: {
-      fileStatusIcon,
-      fileIcon,
-      icon,
-      changedFileIcon,
+    FileStatusIcon,
+    FileIcon,
+    Icon,
+    ChangedFileIcon,
   },
   props: {
     tab: {
@@ -37,11 +37,15 @@
   },
 
   methods: {
-      ...mapActions([
-        'closeFile',
-      ]),
+    ...mapActions(['closeFile', 'updateDelayViewerUpdated', 'openPendingTab']),
     clickFile(tab) {
+      this.updateDelayViewerUpdated(true);
+
+      if (tab.pending) {
+        this.openPendingTab(tab);
+      } else {
         this.$router.push(`/project${tab.url}`);
+      }
     },
     mouseOverTab() {
       if (this.tab.changed) {
@@ -54,7 +58,7 @@
       }
     },
   },
-  };
+};
 </script>
 
 <template>
@@ -66,7 +70,7 @@
     <button
       type="button"
       class="multi-file-tab-close"
-      @click.stop.prevent="closeFile(tab.path)"
+      @click.stop.prevent="closeFile(tab)"
       :aria-label="closeLabel"
     >
       <icon
@@ -82,7 +86,9 @@
 
     <div
       class="multi-file-tab"
-      :class="{active : tab.active }"
+      :class="{
+        active: tab.active
+      }"
       :title="tab.url"
     >
       <file-icon

app/assets/javascripts/ide/components/repo_tabs.vue

@@ -2,6 +2,7 @@
 import { mapActions } from 'vuex';
 import RepoTab from './repo_tab.vue';
 import EditorMode from './editor_mode_dropdown.vue';
+import router from '../ide_router';
 
 export default {
   components: {
@@ -9,6 +10,10 @@ export default {
     EditorMode,
   },
   props: {
+    activeFile: {
+      type: Object,
+      required: true,
+    },
     files: {
       type: Array,
       required: true,
@@ -38,7 +43,18 @@ export default {
     this.showShadow = this.$refs.tabsScroller.scrollWidth > this.$refs.tabsScroller.offsetWidth;
   },
   methods: {
-    ...mapActions(['updateViewer']),
+    ...mapActions(['updateViewer', 'removePendingTab']),
+    openFileViewer(viewer) {
+      this.updateViewer(viewer);
+
+      if (this.activeFile.pending) {
+        return this.removePendingTab(this.activeFile).then(() => {
+          router.push(`/project${this.activeFile.url}`);
+        });
+      }
+
+      return null;
+    },
   },
 };
 </script>
@@ -60,7 +76,7 @@ export default {
       :show-shadow="showShadow"
       :has-changes="hasChanges"
       :merge-request-id="mergeRequestId"
-      @click="updateViewer"
+      @click="openFileViewer"
     />
   </div>
 </template>

app/assets/javascripts/ide/ide_router.js

@@ -77,7 +77,11 @@ router.beforeEach((to, from, next) => {
               if (to.params[0]) {
                 const path =
                   to.params[0].slice(-1) === '/' ? to.params[0].slice(0, -1) : to.params[0];
-                const treeEntry = store.state.entries[path];
+                const treeEntryKey = Object.keys(store.state.entries).find(
+                  key => key === path && !store.state.entries[key].pending,
+                );
+                const treeEntry = store.state.entries[treeEntryKey];
+
                 if (treeEntry) {
                   store.dispatch('handleTreeEntryAction', treeEntry);
                 }

app/assets/javascripts/ide/lib/common/model.js

@@ -13,12 +13,12 @@ export default class Model {
       (this.originalModel = this.monaco.editor.createModel(
         this.file.raw,
         undefined,
-        new this.monaco.Uri(null, null, `original/${this.file.path}`),
+        new this.monaco.Uri(null, null, `original/${this.file.key}`),
       )),
       (this.model = this.monaco.editor.createModel(
         this.content,
         undefined,
-        new this.monaco.Uri(null, null, this.file.path),
+        new this.monaco.Uri(null, null, this.file.key),
       )),
     );
     if (this.file.mrChange) {
@@ -36,7 +36,7 @@ export default class Model {
     this.updateContent = this.updateContent.bind(this);
     this.dispose = this.dispose.bind(this);
 
-    eventHub.$on(`editor.update.model.dispose.${this.file.path}`, this.dispose);
+    eventHub.$on(`editor.update.model.dispose.${this.file.key}`, this.dispose);
     eventHub.$on(`editor.update.model.content.${this.file.path}`, this.updateContent);
   }
 
@@ -53,7 +53,7 @@ export default class Model {
   }
 
   get path() {
-    return this.file.path;
+    return this.file.key;
   }
 
   getModel() {
@@ -88,7 +88,7 @@ export default class Model {
     this.disposable.dispose();
     this.events.clear();
 
-    eventHub.$off(`editor.update.model.dispose.${this.file.path}`, this.dispose);
+    eventHub.$off(`editor.update.model.dispose.${this.file.key}`, this.dispose);
     eventHub.$off(`editor.update.model.content.${this.file.path}`, this.updateContent);
   }
 }

app/assets/javascripts/ide/lib/common/model_manager.js

@@ -9,17 +9,17 @@ export default class ModelManager {
     this.models = new Map();
   }
 
-  hasCachedModel(path) {
-    return this.models.has(path);
+  hasCachedModel(key) {
+    return this.models.has(key);
   }
 
-  getModel(path) {
-    return this.models.get(path);
+  getModel(key) {
+    return this.models.get(key);
   }
 
   addModel(file) {
-    if (this.hasCachedModel(file.path)) {
-      return this.getModel(file.path);
+    if (this.hasCachedModel(file.key)) {
+      return this.getModel(file.key);
     }
 
     const model = new Model(this.monaco, file);
@@ -27,7 +27,7 @@ export default class ModelManager {
     this.disposable.add(model);
 
     eventHub.$on(
-      `editor.update.model.dispose.${file.path}`,
+      `editor.update.model.dispose.${file.key}`,
       this.removeCachedModel.bind(this, file),
     );
 
@@ -35,12 +35,9 @@ export default class ModelManager {
   }
 
   removeCachedModel(file) {
-    this.models.delete(file.path);
+    this.models.delete(file.key);
 
-    eventHub.$off(
-      `editor.update.model.dispose.${file.path}`,
-      this.removeCachedModel,
-    );
+    eventHub.$off(`editor.update.model.dispose.${file.key}`, this.removeCachedModel);
   }
 
   dispose() {

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

@@ -21,7 +21,7 @@ export const discardAllChanges = ({ state, commit, dispatch }) => {
 };
 
 export const closeAllFiles = ({ state, dispatch }) => {
-  state.openFiles.forEach(file => dispatch('closeFile', file.path));
+  state.openFiles.forEach(file => dispatch('closeFile', file));
 };
 
 export const setPanelCollapsedStatus = ({ commit }, { side, collapsed }) => {

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

@@ -6,24 +6,34 @@ import * as types from '../mutation_types';
 import router from '../../ide_router';
 import { setPageTitle } from '../utils';
 
-export const closeFile = ({ commit, state, getters, dispatch }, path) => {
-  const indexOfClosedFile = state.openFiles.findIndex(f => f.path === path);
-  const file = state.entries[path];
+export const closeFile = ({ commit, state, dispatch }, file) => {
+  const path = file.path;
+  const indexOfClosedFile = state.openFiles.findIndex(f => f.key === file.key);
   const fileWasActive = file.active;
 
+  if (file.pending) {
+    commit(types.REMOVE_PENDING_TAB, file);
+  } else {
     commit(types.TOGGLE_FILE_OPEN, path);
     commit(types.SET_FILE_ACTIVE, { path, active: false });
+  }
 
   if (state.openFiles.length > 0 && fileWasActive) {
     const nextIndexToOpen = indexOfClosedFile === 0 ? 0 : indexOfClosedFile - 1;
-    const nextFileToOpen = state.entries[state.openFiles[nextIndexToOpen].path];
+    const nextFileToOpen = state.openFiles[nextIndexToOpen];
 
+    if (nextFileToOpen.pending) {
+      dispatch('updateViewer', 'diff');
+      dispatch('openPendingTab', nextFileToOpen);
+    } else {
+      dispatch('updateDelayViewerUpdated', true);
       router.push(`/project${nextFileToOpen.url}`);
+    }
   } else if (!state.openFiles.length) {
     router.push(`/project/${file.projectId}/tree/${file.branchId}/`);
   }
 
-  eventHub.$emit(`editor.update.model.dispose.${file.path}`);
+  eventHub.$emit(`editor.update.model.dispose.${file.key}`);
 };
 
 export const setFileActive = ({ commit, state, getters, dispatch }, path) => {
@@ -151,3 +161,23 @@ export const discardFileChanges = ({ state, commit }, path) => {
 
   eventHub.$emit(`editor.update.model.content.${file.path}`, file.raw);
 };
+
+export const openPendingTab = ({ commit, getters, dispatch, state }, file) => {
+  if (getters.activeFile && getters.activeFile.path === file.path && state.viewer === 'diff') {
+    return false;
+  }
+
+  commit(types.ADD_PENDING_TAB, { file });
+
+  dispatch('scrollToTab');
+
+  router.push(`/project/${file.projectId}/tree/${state.currentBranchId}/`);
+
+  return true;
+};
+
+export const removePendingTab = ({ commit }, file) => {
+  commit(types.REMOVE_PENDING_TAB, file);
+
+  eventHub.$emit(`editor.update.model.dispose.${file.key}`);
+};

app/assets/javascripts/ide/stores/mutation_types.js

@@ -49,3 +49,6 @@ export const CREATE_TMP_ENTRY = 'CREATE_TMP_ENTRY';
 export const SET_FILE_MERGE_REQUEST_CHANGE = 'SET_FILE_MERGE_REQUEST_CHANGE';
 export const UPDATE_VIEWER = 'UPDATE_VIEWER';
 export const UPDATE_DELAY_VIEWER_CHANGE = 'UPDATE_DELAY_VIEWER_CHANGE';
+
+export const ADD_PENDING_TAB = 'ADD_PENDING_TAB';
+export const REMOVE_PENDING_TAB = 'REMOVE_PENDING_TAB';

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

@@ -5,6 +5,14 @@ export default {
     Object.assign(state.entries[path], {
       active,
     });
+
+    if (active && !state.entries[path].pending) {
+      Object.assign(state, {
+        openFiles: state.openFiles.map(f =>
+          Object.assign(f, { active: f.pending ? false : f.active }),
+        ),
+      });
+    }
   },
   [types.TOGGLE_FILE_OPEN](state, path) {
     Object.assign(state.entries[path], {
@@ -12,10 +20,14 @@ export default {
     });
 
     if (state.entries[path].opened) {
-      state.openFiles.push(state.entries[path]);
+      Object.assign(state, {
+        openFiles: state.openFiles.filter(f => f.path !== path).concat(state.entries[path]),
+      });
     } else {
+      const file = state.entries[path];
+
       Object.assign(state, {
-        openFiles: state.openFiles.filter(f => f.path !== path),
+        openFiles: state.openFiles.filter(f => f.key !== file.key),
       });
     }
   },
@@ -92,4 +104,37 @@ export default {
       changed,
     });
   },
+  [types.ADD_PENDING_TAB](state, { file, keyPrefix = 'pending' }) {
+    const pendingTab = state.openFiles.find(f => f.path === file.path && f.pending);
+    let openFiles = state.openFiles.map(f =>
+      Object.assign(f, { active: f.path === file.path, opened: false }),
+    );
+
+    if (!pendingTab) {
+      const openFile = openFiles.find(f => f.path === file.path);
+
+      openFiles = openFiles.concat(openFile ? null : file).reduce((acc, f) => {
+        if (!f) return acc;
+
+        if (f.path === file.path) {
+          return acc.concat({
+            ...f,
+            active: true,
+            pending: true,
+            opened: true,
+            key: `${keyPrefix}-${f.key}`,
+          });
+        }
+
+        return acc.concat(f);
+      }, []);
+    }
+
+    Object.assign(state, { openFiles });
+  },
+  [types.REMOVE_PENDING_TAB](state, file) {
+    Object.assign(state, {
+      openFiles: state.openFiles.filter(f => f.key !== file.key),
+    });
+  },
 };

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

 export const dataStructure = () => ({
   id: '',
+  // Key will contain a mixture of ID and path
+  // it can also contain a prefix `pending-` for files opened in review mode
   key: '',
   type: '',
   projectId: '',

app/controllers/projects/settings/repository_controller.rb

@@ -20,6 +20,10 @@ module Projects
         @protected_tags = @project.protected_tags.order(:name).page(params[:page])
         @protected_branch = @project.protected_branches.new
         @protected_tag = @project.protected_tags.new
+
+        @protected_branches_count = @protected_branches.reduce(0) { |sum, branch| sum + branch.matching(@project.repository.branches).size }
+        @protected_tags_count = @protected_tags.reduce(0) { |sum, tag| sum + tag.matching(@project.repository.tags).size }
+
         load_gon_index
       end
 

app/models/commit.rb

@@ -32,7 +32,8 @@ class Commit
   COMMIT_SHA_PATTERN = /\h{#{MIN_SHA_LENGTH},40}/.freeze
 
   def banzai_render_context(field)
-    context = { pipeline: :single_line, project: self.project }
+    pipeline = field == :description ? :commit_description : :single_line
+    context = { pipeline: pipeline, project: self.project }
     context[:author] = self.author if self.author
 
     context

app/models/commit_status.rb

@@ -141,7 +141,7 @@ class CommitStatus < ActiveRecord::Base
   end
 
   def group_name
-    name.to_s.gsub(%r{\d+[\.\s:/\\]+\d+\s*}, '').strip
+    name.to_s.gsub(%r{\d+[\s:/\\]+\d+\s*}, '').strip
   end
 
   def failed_but_allowed?

app/views/projects/commit/_commit_box.html.haml

@@ -49,10 +49,10 @@
 
 .commit-box{ data: { project_path: project_path(@project) } }
   %h3.commit-title
-    = markdown(@commit.title, pipeline: :single_line, author: @commit.author)
+    = markdown_field(@commit, :title)
   - if @commit.description.present?
     %pre.commit-description
-      = preserve(markdown(@commit.description, pipeline: :single_line, author: @commit.author))
+      = preserve(markdown_field(@commit, :description))
 
 .info-well
   .well-segment.branch-info

app/views/projects/commits/_commit.atom.builder

@@ -10,5 +10,5 @@ xml.entry do
     xml.email commit.author_email
   end
 
-  xml.summary markdown(commit.description, pipeline: :single_line), type: 'html'
+  xml.summary markdown_field(commit, :description), type: 'html'
 end

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

@@ -2,7 +2,7 @@
   - if @protected_branches.empty?
     .panel-heading
       %h3.panel-title
-        Protected branch (#{@protected_branches.size})
+        Protected branch (#{@protected_branches_count})
     %p.settings-message.text-center
       There are currently no protected branches, protect a branch with the form above.
   - else
@@ -16,7 +16,7 @@
           %col
       %thead
         %tr
-          %th Protected branch (#{@protected_branches.size})
+          %th Protected branch (#{@protected_branches_count})
           %th Last commit
           %th Allowed to merge
           %th Allowed to push

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

@@ -2,7 +2,7 @@
   - if @protected_tags.empty?
     .panel-heading
       %h3.panel-title
-        Protected tag (#{@protected_tags.size})
+        Protected tag (#{@protected_tags_count})
     %p.settings-message.text-center
       There are currently no protected tags, protect a tag with the form above.
   - else
@@ -17,7 +17,7 @@
           %col
       %thead
         %tr
-          %th Protected tag (#{@protected_tags.size})
+          %th Protected tag (#{@protected_tags_count})
           %th Last commit
           %th Allowed to create
           - if can_admin_project

changelogs/unreleased/20394-protected-branches-wildcard.yml

+---
+title: Include matching branches and tags in protected branches / tags count
+merge_request:
+author: Jan Beckmann
+type: fixed

changelogs/unreleased/39880-merge-method-api.yml

 ---
 title: 'API: Add parameter merge_method to projects'
-merge_request:
+merge_request: 18031
 author: Jan Beckmann
 type: added

changelogs/unreleased/feature_detect_co_authored_commits.yml

+---
+title: Detect commit message trailers and link users properly to their accounts
+  on Gitlab
+merge_request: 17919
+author: cousine
+type: added

changelogs/unreleased/sh-gitlab-sidekiq-logger.yml

+---
+title: Add support for Sidekiq JSON logging
+merge_request:
+author:
+type: added

changelogs/unreleased/zj-feature-gate-remove-http-api.yml

+---
+title: Allow feature gates to be removed through the API
+merge_request:
+author:
+type: added

config/gitlab.yml.example

@@ -226,6 +226,10 @@ production: &base
     # plain_url: "http://..."     # default: https://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon
     # ssl_url:   "https://..."    # default: https://secure.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon
 
+  ## Sidekiq
+  sidekiq:
+    log_format: default # (json is also supported)
+
   ## Auxiliary jobs
   # Periodically executed jobs, to self-heal GitLab, do external synchronizations, etc.
   # Please read here for more information: https://github.com/ondrejbartas/sidekiq-cron#adding-cron-job

config/initializers/1_settings.rb

@@ -534,6 +534,12 @@ Settings.cron_jobs['pages_domain_verification_cron_worker'] ||= Settingslogic.ne
 Settings.cron_jobs['pages_domain_verification_cron_worker']['cron'] ||= '*/15 * * * *'
 Settings.cron_jobs['pages_domain_verification_cron_worker']['job_class'] = 'PagesDomainVerificationCronWorker'
 
+#
+# Sidekiq
+#
+Settings['sidekiq'] ||= Settingslogic.new({})
+Settings['sidekiq']['log_format'] ||= 'default'
+
 #
 # GitLab Shell
 #

config/initializers/sidekiq.rb

@@ -5,16 +5,23 @@ queues_config_hash[:namespace] = Gitlab::Redis::Queues::SIDEKIQ_NAMESPACE
 # Default is to retry 25 times with exponential backoff. That's too much.
 Sidekiq.default_worker_options = { retry: 3 }
 
+enable_json_logs = Gitlab.config.sidekiq.log_format == 'json'
+
 Sidekiq.configure_server do |config|
   config.redis = queues_config_hash
 
   config.server_middleware do |chain|
-    chain.add Gitlab::SidekiqMiddleware::ArgumentsLogger if ENV['SIDEKIQ_LOG_ARGUMENTS']
+    chain.add Gitlab::SidekiqMiddleware::ArgumentsLogger if ENV['SIDEKIQ_LOG_ARGUMENTS'] && !enable_json_logs
     chain.add Gitlab::SidekiqMiddleware::Shutdown
     chain.add Gitlab::SidekiqMiddleware::RequestStoreMiddleware unless ENV['SIDEKIQ_REQUEST_STORE'] == '0'
     chain.add Gitlab::SidekiqStatus::ServerMiddleware
   end
 
+  if enable_json_logs
+    Sidekiq.logger.formatter = Gitlab::SidekiqLogging::JSONFormatter.new
+    config.options[:job_logger] = Gitlab::SidekiqLogging::StructuredLogger
+  end
+
   config.client_middleware do |chain|
     chain.add Gitlab::SidekiqStatus::ClientMiddleware
   end

doc/administration/logs.md

@@ -146,6 +146,28 @@ this file. For example:
 2014-06-10T18:18:26Z 14299 TID-55uqo INFO: Booting Sidekiq 3.0.0 with redis options {:url=>"redis://localhost:6379/0", :namespace=>"sidekiq"}
 ```
 
+Instead of the format above, you can opt to generate JSON logs for
+Sidekiq. For example:
+
+```json
+{"severity":"INFO","time":"2018-04-03T22:57:22.071Z","queue":"cronjob:update_all_mirrors","args":[],"class":"UpdateAllMirrorsWorker","retry":false,"queue_namespace":"cronjob","jid":"06aeaa3b0aadacf9981f368e","created_at":"2018-04-03T22:57:21.930Z","enqueued_at":"2018-04-03T22:57:21.931Z","pid":10077,"message":"UpdateAllMirrorsWorker JID-06aeaa3b0aadacf9981f368e: done: 0.139 sec","job_status":"done","duration":0.139,"completed_at":"2018-04-03T22:57:22.071Z"}
+```
+
+For Omnibus GitLab installations, add the configuration option:
+
+```ruby
+sidekiq['log_format'] = 'json'
+```
+
+For source installations, edit the `gitlab.yml` and set the Sidekiq
+`log_format` configuration option:
+
+```yaml
+  ## Sidekiq
+  sidekiq:
+    log_format: json
+```
+
 ## `gitlab-shell.log`
 
 This file lives in `/var/log/gitlab/gitlab-shell/gitlab-shell.log` for

doc/api/features.md

@@ -86,3 +86,11 @@ Example response:
   ]
 }
 ```
+
+## Delete a feature
+
+Removes a feature gate. Response is equal when the gate exists, or doesn't.
+
+```
+DELETE /features/:name
+```

doc/api/projects.md

@@ -31,6 +31,21 @@ There are currently three options for `merge_method` to choose from:
   No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch could be fast-forwarded.
 
 
+## Project merge method
+
+There are currently three options for `merge_method` to choose from:
+
+* `merge`:
+  A merge commit is created for every merge, and merging is allowed as long as there are no conflicts.
+
+* `rebase_merge`:
+  A merge commit is created for every merge, but merging is only allowed if fast-forward merge is possible.
+  This way you could make sure that if this merge request would build, after merging to target branch it would also build.
+
+* `ff`:
+  No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch could be fast-forwarded.
+
+
 ## List all projects
 
 Get a list of all visible projects across GitLab for the authenticated user.
@@ -109,8 +124,8 @@ GET /projects
     "only_allow_merge_if_pipeline_succeeds": false,
     "only_allow_merge_if_all_discussions_are_resolved": false,
     "request_access_enabled": false,
-    "approvals_before_merge": 0,
     "merge_method": "merge",
+    "approvals_before_merge": 0,
     "statistics": {
       "commit_count": 37,
       "storage_size": 1038090,
@@ -190,8 +205,8 @@ GET /projects
     "only_allow_merge_if_pipeline_succeeds": false,
     "only_allow_merge_if_all_discussions_are_resolved": false,
     "request_access_enabled": false,
-    "approvals_before_merge": 0,
     "merge_method": "merge",
+    "approvals_before_merge": 0,
     "statistics": {
       "commit_count": 12,
       "storage_size": 2066080,
@@ -489,8 +504,8 @@ GET /projects/:id
   "only_allow_merge_if_all_discussions_are_resolved": false,
   "printing_merge_requests_link_enabled": true,
   "request_access_enabled": false,
-  "approvals_before_merge": 0,
   "merge_method": "merge",
+  "approvals_before_merge": 0,
   "statistics": {
     "commit_count": 37,
     "storage_size": 1038090,

lib/api/features.rb

@@ -65,6 +65,13 @@ module API
 
         present feature, with: Entities::Feature, current_user: current_user
       end
+
+      desc 'Remove the gate value for the given feature'
+      delete ':name' do
+        Feature.get(params[:name]).remove
+
+        status 204
+      end
     end
   end
 end

lib/banzai/filter/commit_trailers_filter.rb

+module Banzai
+  module Filter
+    # HTML filter that replaces users' names and emails in commit trailers
+    # with links to their GitLab accounts or mailto links to their mentioned
+    # emails.
+    #
+    # Commit trailers are special labels in the form of `*-by:` and fall on a
+    # single line, ex:
+    #
+    #   Reported-By: John S. Doe <john.doe@foo.bar>
+    #
+    # More info about this can be found here:
+    # * https://git.wiki.kernel.org/index.php/CommitMessageConventions
+    class CommitTrailersFilter < HTML::Pipeline::Filter
+      include ActionView::Helpers::TagHelper
+      include ApplicationHelper
+      include AvatarsHelper
+
+      TRAILER_REGEXP = /(?<label>[[:alpha:]-]+-by:)/i.freeze
+      AUTHOR_REGEXP = /(?<author_name>.+)/.freeze
+      # Devise.email_regexp wouldn't work here since its designed to match
+      # against strings that only contains email addresses; the \A and \z
+      # around the expression will only match if the string being matched
+      # contains just the email nothing else.
+      MAIL_REGEXP = /&lt;(?<author_email>[^@\s]+@[^@\s]+)&gt;/.freeze
+      FILTER_REGEXP = /(?<trailer>^\s*#{TRAILER_REGEXP}\s*#{AUTHOR_REGEXP}\s+#{MAIL_REGEXP}$)/mi.freeze
+
+      def call
+        doc.xpath('descendant-or-self::text()').each do |node|
+          content = node.to_html
+
+          next unless content.match(FILTER_REGEXP)
+
+          html = trailer_filter(content)
+
+          next if html == content
+
+          node.replace(html)
+        end
+
+        doc
+      end
+
+      private
+
+      # Replace trailer lines with links to GitLab users or mailto links to
+      # non GitLab users.
+      #
+      # text - String text to replace trailers in.
+      #
+      # Returns a String with all trailer lines replaced with links to GitLab
+      # users and mailto links to non GitLab users. All links have `data-trailer`
+      # and `data-user` attributes attached.
+      def trailer_filter(text)
+        text.gsub(FILTER_REGEXP) do |author_match|
+          label = $~[:label]
+          "#{label} #{parse_user($~[:author_name], $~[:author_email], label)}"
+        end
+      end
+
+      # Find a GitLab user using the supplied email and generate
+      # a valid link to them, otherwise, generate a mailto link.
+      #
+      # name - String name used in the commit message for the user
+      # email - String email used in the commit message for the user
+      # trailer - String trailer used in the commit message
+      #
+      # Returns a String with a link to the user.
+      def parse_user(name, email, trailer)
+        link_to_user User.find_by_any_email(email),
+          name: name,
+          email: email,
+          trailer: trailer
+      end
+
+      def urls
+        Gitlab::Routing.url_helpers
+      end
+
+      def link_to_user(user, name:, email:, trailer:)
+        wrapper = link_wrapper(data: {
+          trailer: trailer,
+          user: user.try(:id)
+        })
+
+        avatar = user_avatar_without_link(
+          user: user,
+          user_email: email,
+          css_class: 'avatar-inline',
+          has_tooltip: false
+        )
+
+        link_href = user.nil? ? "mailto:#{email}" : urls.user_url(user)
+
+        avatar_link = link_tag(
+          link_href,
+          content: avatar,
+          title: email
+        )
+
+        name_link = link_tag(
+          link_href,
+          content: name,
+          title: email
+        )
+
+        email_link = link_tag(
+          "mailto:#{email}",
+          content: email,
+          title: email
+        )
+
+        wrapper << "#{avatar_link}#{name_link} <#{email_link}>"
+      end
+
+      def link_wrapper(data: {})
+        data_attributes = data_attributes_from_hash(data)
+
+        doc.document.create_element(
+          'span',
+          data_attributes
+        )
+      end
+
+      def link_tag(url, title: "", content: "", data: {})
+        data_attributes = data_attributes_from_hash(data)
+
+        attributes = data_attributes.merge(
+          href: url,
+          title: title
+        )
+
+        link = doc.document.create_element('a', attributes)
+
+        if content.html_safe?
+          link << content
+        else
+          link.content = content # make sure we escape content using nokogiri's #content=
+        end
+
+        link
+      end
+
+      def data_attributes_from_hash(data = {})
+        data.reject! {|_, value| value.nil?}
+        data.map do |key, value|
+          [%(data-#{key.to_s.dasherize}), value]
+        end.to_h
+      end
+    end
+  end
+end

lib/banzai/pipeline/commit_description_pipeline.rb

+module Banzai
+  module Pipeline
+    class CommitDescriptionPipeline < SingleLinePipeline
+      def self.filters
+        @filters ||= super.concat FilterArray[
+          Filter::CommitTrailersFilter,
+        ]
+      end
+    end
+  end
+end

lib/gitlab/sidekiq_logging/json_formatter.rb

+module Gitlab
+  module SidekiqLogging
+    class JSONFormatter
+      def call(severity, timestamp, progname, data)
+        output = {
+          severity: severity,
+          time: timestamp.utc.iso8601(3)
+        }
+
+        case data
+        when String
+          output[:message] = data
+        when Hash
+          output.merge!(data)
+        end
+
+        output.to_json + "\n"
+      end
+    end
+  end
+end

lib/gitlab/sidekiq_logging/structured_logger.rb

+module Gitlab
+  module SidekiqLogging
+    class StructuredLogger
+      START_TIMESTAMP_FIELDS = %w[created_at enqueued_at].freeze
+      DONE_TIMESTAMP_FIELDS = %w[started_at retried_at failed_at completed_at].freeze
+
+      def call(job, queue)
+        started_at = current_time
+        base_payload = parse_job(job)
+
+        Sidekiq.logger.info log_job_start(started_at, base_payload)
+
+        yield
+
+        Sidekiq.logger.info log_job_done(started_at, base_payload)
+      rescue => job_exception
+        Sidekiq.logger.warn log_job_done(started_at, base_payload, job_exception)
+
+        raise
+      end
+
+      private
+
+      def base_message(payload)
+        "#{payload['class']} JID-#{payload['jid']}"
+      end
+
+      def log_job_start(started_at, payload)
+        payload['message'] = "#{base_message(payload)}: start"
+        payload['job_status'] = 'start'
+
+        payload
+      end
+
+      def log_job_done(started_at, payload, job_exception = nil)
+        payload = payload.dup
+        payload['duration'] = elapsed(started_at)
+        payload['completed_at'] = Time.now.utc
+
+        message = base_message(payload)
+
+        if job_exception
+          payload['message'] = "#{message}: fail: #{payload['duration']} sec"
+          payload['job_status'] = 'fail'
+          payload['error_message'] = job_exception.message
+          payload['error'] = job_exception.class
+          payload['error_backtrace'] = backtrace_cleaner.clean(job_exception.backtrace)
+        else
+          payload['message'] = "#{message}: done: #{payload['duration']} sec"
+          payload['job_status'] = 'done'
+        end
+
+        convert_to_iso8601(payload, DONE_TIMESTAMP_FIELDS)
+
+        payload
+      end
+
+      def parse_job(job)
+        job = job.dup
+
+        # Add process id params
+        job['pid'] = ::Process.pid
+
+        job.delete('args') unless ENV['SIDEKIQ_LOG_ARGUMENTS']
+
+        convert_to_iso8601(job, START_TIMESTAMP_FIELDS)
+
+        job
+      end
+
+      def convert_to_iso8601(payload, keys)
+        keys.each do |key|
+          payload[key] = format_time(payload[key]) if payload[key]
+        end
+      end
+
+      def elapsed(start)
+        (current_time - start).round(3)
+      end
+
+      def current_time
+        Gitlab::Metrics::System.monotonic_time
+      end
+
+      def backtrace_cleaner
+        @backtrace_cleaner ||= ActiveSupport::BacktraceCleaner.new
+      end
+
+      def format_time(timestamp)
+        return timestamp if timestamp.is_a?(String)
+
+        Time.at(timestamp).utc.iso8601(3)
+      end
+    end
+  end
+end

scripts/codequality

-#!/bin/sh
-
-set -eo pipefail
-
-code_path=$(pwd)
-
-# docker run --tty will merge stderr and stdout, we don't need this on CI or
-# it will break codequality json file
-[ "$CI" != "" ] || docker_tty="--tty"
-
-# The codebase and instructions for the following image can be found at https://gitlab.com/gitlab-org/codeclimate-rubocop/wikis/home
-docker pull dev.gitlab.org:5005/gitlab/gitlab-build-images:gitlab-codeclimate-rubocop-0-52-1 > /dev/null
-docker tag dev.gitlab.org:5005/gitlab/gitlab-build-images:gitlab-codeclimate-rubocop-0-52-1 codeclimate/codeclimate-rubocop:gitlab-codeclimate-rubocop-0-52-1 > /dev/null
-
-exec docker run --rm $docker_tty --env CODECLIMATE_CODE="$code_path" \
-	--volume "$code_path":/code \
-	--volume /var/run/docker.sock:/var/run/docker.sock \
-	--volume /tmp/cc:/tmp/cc \
-	"codeclimate/codeclimate:${CODECLIMATE_VERSION:-0.71.1}" "$@"

spec/features/protected_branches_spec.rb

@@ -159,7 +159,10 @@ feature 'Protected Branches', :js do
         set_allowed_to('push')
         click_on "Protect"
 
-        within(".protected-branches-list") { expect(page).to have_content("2 matching branches") }
+        within(".protected-branches-list") do
+          expect(page).to have_content("Protected branch (2)")
+          expect(page).to have_content("2 matching branches")
+        end
       end
 
       it "displays all the branches matching the wildcard" do

spec/features/protected_tags_spec.rb

@@ -81,7 +81,10 @@ feature 'Protected Tags', :js do
       set_allowed_to('create')
       click_on "Protect"
 
-      within(".protected-tags-list") { expect(page).to have_content("2 matching tags") }
+      within(".protected-tags-list") do
+        expect(page).to have_content("Protected tag (2)")
+        expect(page).to have_content("2 matching tags")
+      end
     end
 
     it "displays all the tags matching the wildcard" do

spec/javascripts/ide/components/commit_sidebar/list_item_spec.js

 import Vue from 'vue';
 import listItem from '~/ide/components/commit_sidebar/list_item.vue';
 import router from '~/ide/ide_router';
-import mountComponent from 'spec/helpers/vue_mount_component_helper';
-import { file } from '../../helpers';
+import store from '~/ide/stores';
+import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
+import { file, resetStore } from '../../helpers';
 
 describe('Multi-file editor commit sidebar list item', () => {
   let vm;
@@ -13,19 +14,21 @@ describe('Multi-file editor commit sidebar list item', () => {
 
     f = file('test-file');
 
-    vm = mountComponent(Component, {
+    store.state.entries[f.path] = f;
+
+    vm = createComponentWithStore(Component, store, {
       file: f,
-    });
+    }).$mount();
   });
 
   afterEach(() => {
     vm.$destroy();
+
+    resetStore(store);
   });
 
   it('renders file path', () => {
-    expect(
-      vm.$el.querySelector('.multi-file-commit-list-path').textContent.trim(),
-    ).toBe(f.path);
+    expect(vm.$el.querySelector('.multi-file-commit-list-path').textContent.trim()).toBe(f.path);
   });
 
   it('calls discardFileChanges when clicking discard button', () => {
@@ -36,25 +39,32 @@ describe('Multi-file editor commit sidebar list item', () => {
     expect(vm.discardFileChanges).toHaveBeenCalled();
   });
 
-  it('opens a closed file in the editor when clicking the file path', () => {
+  it('opens a closed file in the editor when clicking the file path', done => {
     spyOn(vm, 'openFileInEditor').and.callThrough();
-    spyOn(vm, 'updateViewer');
     spyOn(router, 'push');
 
     vm.$el.querySelector('.multi-file-commit-list-path').click();
 
+    setTimeout(() => {
       expect(vm.openFileInEditor).toHaveBeenCalled();
       expect(router.push).toHaveBeenCalled();
+
+      done();
+    });
   });
 
-  it('calls updateViewer with diff when clicking file', () => {
+  it('calls updateViewer with diff when clicking file', done => {
     spyOn(vm, 'openFileInEditor').and.callThrough();
-    spyOn(vm, 'updateViewer');
+    spyOn(vm, 'updateViewer').and.callThrough();
     spyOn(router, 'push');
 
     vm.$el.querySelector('.multi-file-commit-list-path').click();
 
+    setTimeout(() => {
       expect(vm.updateViewer).toHaveBeenCalledWith('diff');
+
+      done();
+    });
   });
 
   describe('computed', () => {

spec/javascripts/ide/components/repo_tab_spec.js

@@ -59,7 +59,7 @@ describe('RepoTab', () => {
 
     vm.$el.querySelector('.multi-file-tab-close').click();
 
-    expect(vm.closeFile).toHaveBeenCalledWith(vm.tab.path);
+    expect(vm.closeFile).toHaveBeenCalledWith(vm.tab);
   });
 
   it('changes icon on hover', done => {

spec/javascripts/ide/components/repo_tabs_spec.js

@@ -17,6 +17,7 @@ describe('RepoTabs', () => {
       files: openedFiles,
       viewer: 'editor',
       hasChanges: false,
+      activeFile: file('activeFile'),
       hasMergeRequest: false,
     });
     openedFiles[0].active = true;
@@ -57,6 +58,7 @@ describe('RepoTabs', () => {
           files: [],
           viewer: 'editor',
           hasChanges: false,
+          activeFile: file('activeFile'),
           hasMergeRequest: false,
         },
         '#test-app',

spec/javascripts/ide/lib/common/model_manager_spec.js

@@ -27,9 +27,10 @@ describe('Multi-file editor library model manager', () => {
     });
 
     it('caches model by file path', () => {
-      instance.addModel(file('path-name'));
+      const f = file('path-name');
+      instance.addModel(f);
 
-      expect(instance.models.keys().next().value).toBe('path-name');
+      expect(instance.models.keys().next().value).toBe(f.key);
     });
 
     it('adds model into disposable', () => {
@@ -56,7 +57,7 @@ describe('Multi-file editor library model manager', () => {
       instance.addModel(f);
 
       expect(eventHub.$on).toHaveBeenCalledWith(
-        `editor.update.model.dispose.${f.path}`,
+        `editor.update.model.dispose.${f.key}`,
         jasmine.anything(),
       );
     });
@@ -68,9 +69,11 @@ describe('Multi-file editor library model manager', () => {
     });
 
     it('returns true when model exists', () => {
-      instance.addModel(file('path-name'));
+      const f = file('path-name');
+
+      instance.addModel(f);
 
-      expect(instance.hasCachedModel('path-name')).toBeTruthy();
+      expect(instance.hasCachedModel(f.key)).toBeTruthy();
     });
   });
 
@@ -103,7 +106,7 @@ describe('Multi-file editor library model manager', () => {
       instance.removeCachedModel(f);
 
       expect(eventHub.$off).toHaveBeenCalledWith(
-        `editor.update.model.dispose.${f.path}`,
+        `editor.update.model.dispose.${f.key}`,
         jasmine.anything(),
       );
     });

spec/javascripts/ide/lib/common/model_spec.js

@@ -32,14 +32,14 @@ describe('Multi-file editor library model', () => {
 
   it('adds eventHub listener', () => {
     expect(eventHub.$on).toHaveBeenCalledWith(
-      `editor.update.model.dispose.${model.file.path}`,
+      `editor.update.model.dispose.${model.file.key}`,
       jasmine.anything(),
     );
   });
 
   describe('path', () => {
     it('returns file path', () => {
-      expect(model.path).toBe('path');
+      expect(model.path).toBe(model.file.key);
     });
   });
 
@@ -74,7 +74,7 @@ describe('Multi-file editor library model', () => {
       model.onChange(() => {});
 
       expect(model.events.size).toBe(1);
-      expect(model.events.keys().next().value).toBe('path');
+      expect(model.events.keys().next().value).toBe(model.file.key);
     });
 
     it('calls callback on change', done => {
@@ -115,7 +115,7 @@ describe('Multi-file editor library model', () => {
       model.dispose();
 
       expect(eventHub.$off).toHaveBeenCalledWith(
-        `editor.update.model.dispose.${model.file.path}`,
+        `editor.update.model.dispose.${model.file.key}`,
         jasmine.anything(),
       );
     });

spec/javascripts/ide/lib/decorations/controller_spec.js

@@ -36,9 +36,7 @@ describe('Multi-file editor library decorations controller', () => {
     });
 
     it('returns decorations by model URL', () => {
-      controller.addDecorations(model, 'key', [
-        { decoration: 'decorationValue' },
-      ]);
+      controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]);
 
       const decorations = controller.getAllDecorationsForModel(model);
 
@@ -48,39 +46,29 @@ describe('Multi-file editor library decorations controller', () => {
 
   describe('addDecorations', () => {
     it('caches decorations in a new map', () => {
-      controller.addDecorations(model, 'key', [
-        { decoration: 'decorationValue' },
-      ]);
+      controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]);
 
       expect(controller.decorations.size).toBe(1);
     });
 
     it('does not create new cache model', () => {
-      controller.addDecorations(model, 'key', [
-        { decoration: 'decorationValue' },
-      ]);
-      controller.addDecorations(model, 'key', [
-        { decoration: 'decorationValue2' },
-      ]);
+      controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]);
+      controller.addDecorations(model, 'key', [{ decoration: 'decorationValue2' }]);
 
       expect(controller.decorations.size).toBe(1);
     });
 
     it('caches decorations by model URL', () => {
-      controller.addDecorations(model, 'key', [
-        { decoration: 'decorationValue' },
-      ]);
+      controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]);
 
       expect(controller.decorations.size).toBe(1);
-      expect(controller.decorations.keys().next().value).toBe('path');
+      expect(controller.decorations.keys().next().value).toBe('path--path');
     });
 
     it('calls decorate method', () => {
       spyOn(controller, 'decorate');
 
-      controller.addDecorations(model, 'key', [
-        { decoration: 'decorationValue' },
-      ]);
+      controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]);
 
       expect(controller.decorate).toHaveBeenCalled();
     });
@@ -92,10 +80,7 @@ describe('Multi-file editor library decorations controller', () => {
 
       controller.decorate(model);
 
-      expect(controller.editor.instance.deltaDecorations).toHaveBeenCalledWith(
-        [],
-        [],
-      );
+      expect(controller.editor.instance.deltaDecorations).toHaveBeenCalledWith([], []);
     });
 
     it('caches decorations', () => {
@@ -111,15 +96,13 @@ describe('Multi-file editor library decorations controller', () => {
 
       controller.decorate(model);
 
-      expect(controller.editorDecorations.keys().next().value).toBe('path');
+      expect(controller.editorDecorations.keys().next().value).toBe('path--path');
     });
   });
 
   describe('dispose', () => {
     it('clears cached decorations', () => {
-      controller.addDecorations(model, 'key', [
-        { decoration: 'decorationValue' },
-      ]);
+      controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]);
 
       controller.dispose();
 
@@ -127,9 +110,7 @@ describe('Multi-file editor library decorations controller', () => {
     });
 
     it('clears cached editorDecorations', () => {
-      controller.addDecorations(model, 'key', [
-        { decoration: 'decorationValue' },
-      ]);
+      controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]);
 
       controller.dispose();
 

spec/javascripts/ide/lib/diff/controller_spec.js

@@ -131,7 +131,7 @@ describe('Multi-file editor library dirty diff controller', () => {
     it('adds decorations into decorations controller', () => {
       spyOn(controller.decorationsController, 'addDecorations');
 
-      controller.decorate({ data: { changes: [], path: 'path' } });
+      controller.decorate({ data: { changes: [], path: model.path } });
 
       expect(
         controller.decorationsController.addDecorations,
@@ -145,7 +145,7 @@ describe('Multi-file editor library dirty diff controller', () => {
       );
 
       controller.decorate({
-        data: { changes: computeDiff('123', '1234'), path: 'path' },
+        data: { changes: computeDiff('123', '1234'), path: model.path },
       });
 
       expect(spy).toHaveBeenCalledWith(

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

@@ -29,7 +29,7 @@ describe('IDE store file actions', () => {
 
     it('closes open files', done => {
       store
-        .dispatch('closeFile', localFile.path)
+        .dispatch('closeFile', localFile)
         .then(() => {
           expect(localFile.opened).toBeFalsy();
           expect(localFile.active).toBeFalsy();
@@ -44,7 +44,7 @@ describe('IDE store file actions', () => {
       store.state.changedFiles.push(localFile);
 
       store
-        .dispatch('closeFile', localFile.path)
+        .dispatch('closeFile', localFile)
         .then(Vue.nextTick)
         .then(() => {
           expect(store.state.openFiles.length).toBe(0);
@@ -65,7 +65,7 @@ describe('IDE store file actions', () => {
       store.state.entries[f.path] = f;
 
       store
-        .dispatch('closeFile', localFile.path)
+        .dispatch('closeFile', localFile)
         .then(Vue.nextTick)
         .then(() => {
           expect(router.push).toHaveBeenCalledWith(`/project${f.url}`);
@@ -74,6 +74,22 @@ describe('IDE store file actions', () => {
         })
         .catch(done.fail);
     });
+
+    it('removes file if it pending', done => {
+      store.state.openFiles.push({
+        ...localFile,
+        pending: true,
+      });
+
+      store
+        .dispatch('closeFile', localFile)
+        .then(() => {
+          expect(store.state.openFiles.length).toBe(0);
+
+          done();
+        })
+        .catch(done.fail);
+    });
   });
 
   describe('setFileActive', () => {
@@ -445,4 +461,113 @@ describe('IDE store file actions', () => {
         .catch(done.fail);
     });
   });
+
+  describe('openPendingTab', () => {
+    let f;
+
+    beforeEach(() => {
+      f = {
+        ...file(),
+        projectId: '123',
+      };
+
+      store.state.entries[f.path] = f;
+    });
+
+    it('makes file pending in openFiles', done => {
+      store
+        .dispatch('openPendingTab', f)
+        .then(() => {
+          expect(store.state.openFiles[0].pending).toBe(true);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('returns true when opened', done => {
+      store
+        .dispatch('openPendingTab', f)
+        .then(added => {
+          expect(added).toBe(true);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('pushes router URL when added', done => {
+      store.state.currentBranchId = 'master';
+
+      store
+        .dispatch('openPendingTab', f)
+        .then(() => {
+          expect(router.push).toHaveBeenCalledWith('/project/123/tree/master/');
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('calls scrollToTab', done => {
+      const scrollToTabSpy = jasmine.createSpy('scrollToTab');
+      const oldScrollToTab = store._actions.scrollToTab; // eslint-disable-line
+      store._actions.scrollToTab = [scrollToTabSpy]; // eslint-disable-line
+
+      store
+        .dispatch('openPendingTab', f)
+        .then(() => {
+          expect(scrollToTabSpy).toHaveBeenCalled();
+          store._actions.scrollToTab = oldScrollToTab; // eslint-disable-line
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('returns false when passed in file is active & viewer is diff', done => {
+      f.active = true;
+      store.state.openFiles.push(f);
+      store.state.viewer = 'diff';
+
+      store
+        .dispatch('openPendingTab', f)
+        .then(added => {
+          expect(added).toBe(false);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+
+  describe('removePendingTab', () => {
+    let f;
+
+    beforeEach(() => {
+      spyOn(eventHub, '$emit');
+
+      f = {
+        ...file('pendingFile'),
+        pending: true,
+      };
+    });
+
+    it('removes pending file from open files', done => {
+      store.state.openFiles.push(f);
+
+      store
+        .dispatch('removePendingTab', f)
+        .then(() => {
+          expect(store.state.openFiles.length).toBe(0);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('emits event to dispose model', done => {
+      store
+        .dispatch('removePendingTab', f)
+        .then(() => {
+          expect(eventHub.$emit).toHaveBeenCalledWith(`editor.update.model.dispose.${f.key}`);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
 });

spec/javascripts/ide/stores/mutations/file_spec.js

@@ -22,6 +22,21 @@ describe('IDE store file mutations', () => {
 
       expect(localFile.active).toBeTruthy();
     });
+
+    it('sets pending tab as not active', () => {
+      localState.openFiles.push({
+        ...localFile,
+        pending: true,
+        active: true,
+      });
+
+      mutations.SET_FILE_ACTIVE(localState, {
+        path: localFile.path,
+        active: true,
+      });
+
+      expect(localState.openFiles[0].active).toBe(false);
+    });
   });
 
   describe('TOGGLE_FILE_OPEN', () => {
@@ -178,4 +193,69 @@ describe('IDE store file mutations', () => {
       expect(localFile.changed).toBeTruthy();
     });
   });
+
+  describe('ADD_PENDING_TAB', () => {
+    beforeEach(() => {
+      const f = {
+        ...file('openFile'),
+        path: 'openFile',
+        active: true,
+        opened: true,
+      };
+
+      localState.entries[f.path] = f;
+      localState.openFiles.push(f);
+    });
+
+    it('adds file into openFiles as pending', () => {
+      mutations.ADD_PENDING_TAB(localState, { file: localFile });
+
+      expect(localState.openFiles.length).toBe(2);
+      expect(localState.openFiles[1].pending).toBe(true);
+      expect(localState.openFiles[1].key).toBe(`pending-${localFile.key}`);
+    });
+
+    it('updates open file to pending', () => {
+      mutations.ADD_PENDING_TAB(localState, { file: localState.openFiles[0] });
+
+      expect(localState.openFiles.length).toBe(1);
+    });
+
+    it('updates pending open file to active', () => {
+      localState.openFiles.push({
+        ...localFile,
+        pending: true,
+      });
+
+      mutations.ADD_PENDING_TAB(localState, { file: localFile });
+
+      expect(localState.openFiles[1].pending).toBe(true);
+      expect(localState.openFiles[1].active).toBe(true);
+    });
+
+    it('sets all openFiles to not active', () => {
+      mutations.ADD_PENDING_TAB(localState, { file: localFile });
+
+      expect(localState.openFiles.length).toBe(2);
+
+      localState.openFiles.forEach(f => {
+        if (f.pending) {
+          expect(f.active).toBe(true);
+        } else {
+          expect(f.active).toBe(false);
+        }
+      });
+    });
+  });
+
+  describe('REMOVE_PENDING_TAB', () => {
+    it('removes pending tab from openFiles', () => {
+      localFile.key = 'testing';
+      localState.openFiles.push(localFile);
+
+      mutations.REMOVE_PENDING_TAB(localState, localFile);
+
+      expect(localState.openFiles.length).toBe(0);
+    });
+  });
 });

spec/lib/banzai/filter/commit_trailers_filter_spec.rb

+require 'spec_helper'
+require 'ffaker'
+
+describe Banzai::Filter::CommitTrailersFilter do
+  include FilterSpecHelper
+  include CommitTrailersSpecHelper
+
+  let(:secondary_email)     { create(:email, :confirmed) }
+  let(:user)                { create(:user) }
+
+  let(:trailer)             { "#{FFaker::Lorem.word}-by:"}
+
+  let(:commit_message)      { trailer_line(trailer, user.name, user.email) }
+  let(:commit_message_html) { commit_html(commit_message) }
+
+  context 'detects' do
+    let(:email) { FFaker::Internet.email }
+
+    it 'trailers in the form of *-by and replace users with links' do
+      doc = filter(commit_message_html)
+
+      expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+    end
+
+    it 'trailers prefixed with whitespaces' do
+      message_html = commit_html("\n\r  #{commit_message}")
+
+      doc = filter(message_html)
+
+      expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+    end
+
+    it 'GitLab users via a secondary email' do
+      _, message_html = build_commit_message(
+        trailer: trailer,
+        name: secondary_email.user.name,
+        email: secondary_email.email
+      )
+
+      doc = filter(message_html)
+
+      expect_to_have_user_link_with_avatar(
+        doc,
+        user: secondary_email.user,
+        trailer: trailer,
+        email: secondary_email.email
+      )
+    end
+
+    it 'non GitLab users and replaces them with mailto links' do
+      _, message_html = build_commit_message(
+        trailer: trailer,
+        name: FFaker::Name.name,
+        email: email
+      )
+
+      doc = filter(message_html)
+
+      expect_to_have_mailto_link(doc, email: email, trailer: trailer)
+    end
+
+    it 'multiple trailers in the same message' do
+      different_trailer = "#{FFaker::Lorem.word}-by:"
+      message = commit_html %(
+        #{commit_message}
+        #{trailer_line(different_trailer, FFaker::Name.name, email)}
+      )
+
+      doc = filter(message)
+
+      expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+      expect_to_have_mailto_link(doc, email: email, trailer: different_trailer)
+    end
+
+    context 'special names' do
+      where(:name) do
+        [
+          'John S. Doe',
+          'L33t H@x0r'
+        ]
+      end
+
+      with_them do
+        it do
+          message, message_html = build_commit_message(
+            trailer: trailer,
+            name: name,
+            email: email
+          )
+
+          doc = filter(message_html)
+
+          expect_to_have_mailto_link(doc, email: email, trailer: trailer)
+          expect(doc.text).to match Regexp.escape(message)
+        end
+      end
+    end
+  end
+
+  context "ignores" do
+    it 'commit messages without trailers' do
+      exp = message = commit_html(FFaker::Lorem.sentence)
+      doc = filter(message)
+
+      expect(doc.to_html).to match Regexp.escape(exp)
+    end
+
+    it 'trailers that are inline the commit message body' do
+      message = commit_html %(
+        #{FFaker::Lorem.sentence} #{commit_message} #{FFaker::Lorem.sentence}
+      )
+
+      doc = filter(message)
+
+      expect(doc.css('a').size).to eq 0
+    end
+  end
+
+  context "structure" do
+    it 'preserves the commit trailer structure' do
+      doc = filter(commit_message_html)
+
+      expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+      expect(doc.text).to match Regexp.escape(commit_message)
+    end
+
+    it 'preserves the original name used in the commit message' do
+      message, message_html = build_commit_message(
+        trailer: trailer,
+        name: FFaker::Name.name,
+        email: user.email
+      )
+
+      doc = filter(message_html)
+
+      expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+      expect(doc.text).to match Regexp.escape(message)
+    end
+
+    it 'preserves the original email used in the commit message' do
+      message, message_html = build_commit_message(
+        trailer: trailer,
+        name: secondary_email.user.name,
+        email: secondary_email.email
+      )
+
+      doc = filter(message_html)
+
+      expect_to_have_user_link_with_avatar(
+        doc,
+        user: secondary_email.user,
+        trailer: trailer,
+        email: secondary_email.email
+      )
+      expect(doc.text).to match Regexp.escape(message)
+    end
+
+    it 'only replaces trailer lines not the full commit message' do
+      commit_body = FFaker::Lorem.paragraph
+      message = commit_html %(
+        #{commit_body}
+        #{commit_message}
+      )
+
+      doc = filter(message)
+
+      expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+      expect(doc.text).to include(commit_body)
+    end
+  end
+end

spec/lib/gitlab/sidekiq_logging/json_formatter_spec.rb

+require 'spec_helper'
+
+describe Gitlab::SidekiqLogging::JSONFormatter do
+  let(:hash_input) { { foo: 1, bar: 'test' } }
+  let(:message) { 'This is a test' }
+  let(:timestamp) { Time.now }
+
+  it 'wraps a Hash' do
+    result = subject.call('INFO', timestamp, 'my program', hash_input)
+
+    data = JSON.parse(result)
+    expected_output = hash_input.stringify_keys
+    expected_output['severity'] = 'INFO'
+    expected_output['time'] = timestamp.utc.iso8601(3)
+
+    expect(data).to eq(expected_output)
+  end
+
+  it 'wraps a String' do
+    result = subject.call('DEBUG', timestamp, 'my string', message)
+
+    data = JSON.parse(result)
+    expected_output = {
+      severity: 'DEBUG',
+      time: timestamp.utc.iso8601(3),
+      message: message
+    }
+
+    expect(data).to eq(expected_output.stringify_keys)
+  end
+end

spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb

+require 'spec_helper'
+
+describe Gitlab::SidekiqLogging::StructuredLogger do
+  describe '#call' do
+    let(:timestamp) { Time.new('2018-01-01 12:00:00').utc }
+    let(:job) do
+      {
+        "class" => "TestWorker",
+        "args" => [1234, 'hello'],
+        "retry" => false,
+        "queue" => "cronjob:test_queue",
+        "queue_namespace" => "cronjob",
+        "jid" => "da883554ee4fe414012f5f42",
+        "created_at" => timestamp.to_f,
+        "enqueued_at" => timestamp.to_f
+      }
+    end
+    let(:logger) { double() }
+    let(:start_payload) do
+      job.merge(
+        'message' => 'TestWorker JID-da883554ee4fe414012f5f42: start',
+        'job_status' => 'start',
+        'pid' => Process.pid,
+        'created_at' => timestamp.iso8601(3),
+        'enqueued_at' => timestamp.iso8601(3)
+      )
+    end
+    let(:end_payload) do
+      start_payload.merge(
+        'message' => 'TestWorker JID-da883554ee4fe414012f5f42: done: 0.0 sec',
+        'job_status' => 'done',
+        'duration' => 0.0,
+        "completed_at" => timestamp.iso8601(3)
+      )
+    end
+    let(:exception_payload) do
+      end_payload.merge(
+        'message' => 'TestWorker JID-da883554ee4fe414012f5f42: fail: 0.0 sec',
+        'job_status' => 'fail',
+        'error' => ArgumentError,
+        'error_message' => 'some exception'
+      )
+    end
+
+    before do
+      allow(Sidekiq).to receive(:logger).and_return(logger)
+
+      allow(subject).to receive(:current_time).and_return(timestamp.to_f)
+    end
+
+    subject { described_class.new }
+
+    context 'with SIDEKIQ_LOG_ARGUMENTS enabled' do
+      before do
+        stub_env('SIDEKIQ_LOG_ARGUMENTS', '1')
+      end
+
+      it 'logs start and end of job' do
+        Timecop.freeze(timestamp) do
+          expect(logger).to receive(:info).with(start_payload).ordered
+          expect(logger).to receive(:info).with(end_payload).ordered
+          expect(subject).to receive(:log_job_start).and_call_original
+          expect(subject).to receive(:log_job_done).and_call_original
+
+          subject.call(job, 'test_queue') { }
+        end
+      end
+
+      it 'logs an exception in job' do
+        Timecop.freeze(timestamp) do
+          expect(logger).to receive(:info).with(start_payload)
+          # This excludes the exception_backtrace
+          expect(logger).to receive(:warn).with(hash_including(exception_payload))
+          expect(subject).to receive(:log_job_start).and_call_original
+          expect(subject).to receive(:log_job_done).and_call_original
+
+          expect do
+            subject.call(job, 'test_queue') do
+              raise ArgumentError, 'some exception'
+            end
+          end.to raise_error(ArgumentError)
+        end
+      end
+    end
+
+    context 'with SIDEKIQ_LOG_ARGUMENTS disabled' do
+      it 'logs start and end of job' do
+        Timecop.freeze(timestamp) do
+          start_payload.delete('args')
+
+          expect(logger).to receive(:info).with(start_payload).ordered
+          expect(logger).to receive(:info).with(end_payload).ordered
+          expect(subject).to receive(:log_job_start).and_call_original
+          expect(subject).to receive(:log_job_done).and_call_original
+
+          subject.call(job, 'test_queue') { }
+        end
+      end
+    end
+  end
+end

spec/models/commit_status_spec.rb

@@ -368,9 +368,7 @@ describe CommitStatus do
       'rspec:windows 0 : / 1' => 'rspec:windows',
       'rspec:windows 0 : / 1 name' => 'rspec:windows name',
       '0 1 name ruby' => 'name ruby',
-      '0 :/ 1 name ruby' => 'name ruby',
-      'golang test 1.8' => 'golang test',
-      '1.9 golang test' => 'golang test'
+      '0 :/ 1 name ruby' => 'name ruby'
     }
 
     tests.each do |name, group_name|

spec/requests/api/features_spec.rb

 require 'spec_helper'
 
 describe API::Features do
-  let(:user)  { create(:user) }
-  let(:admin) { create(:admin) }
+  set(:user)  { create(:user) }
+  set(:admin) { create(:admin) }
 
   before do
     Flipper.unregister_groups
@@ -249,4 +249,43 @@ describe API::Features do
       end
     end
   end
+
+  describe 'DELETE /feature/:name' do
+    let(:feature_name) { 'my_feature' }
+
+    context 'when the user has no access' do
+      it 'returns a 401 for anonymous users' do
+        delete api("/features/#{feature_name}")
+
+        expect(response).to have_gitlab_http_status(401)
+      end
+
+      it 'returns a 403 for users' do
+        delete api("/features/#{feature_name}", user)
+
+        expect(response).to have_gitlab_http_status(403)
+      end
+    end
+
+    context 'when the user has access' do
+      it 'returns 204 when the value is not set' do
+        delete api("/features/#{feature_name}", admin)
+
+        expect(response).to have_gitlab_http_status(204)
+      end
+
+      context 'when the gate value was set' do
+        before do
+          Feature.get(feature_name).enable
+        end
+
+        it 'deletes an enabled feature' do
+          delete api("/features/#{feature_name}", admin)
+
+          expect(response).to have_gitlab_http_status(204)
+          expect(Feature.get(feature_name)).not_to be_enabled
+        end
+      end
+    end
+  end
 end

spec/requests/api/projects_spec.rb

@@ -889,8 +889,8 @@ describe API::Projects do
         expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access)
         expect(json_response['only_allow_merge_if_pipeline_succeeds']).to eq(project.only_allow_merge_if_pipeline_succeeds)
         expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved)
-        expect(json_response).not_to have_key('repository_storage')
         expect(json_response['merge_method']).to eq(project.merge_method.to_s)
+        expect(json_response).not_to have_key('repository_storage')
       end
 
       it 'returns a project by path name' do

spec/support/commit_trailers_spec_helper.rb

+module CommitTrailersSpecHelper
+  extend ActiveSupport::Concern
+
+  def expect_to_have_user_link_with_avatar(doc, user:, trailer:, email: nil)
+    wrapper = find_user_wrapper(doc, trailer)
+
+    expect_to_have_links_with_url_and_avatar(wrapper, urls.user_url(user), email || user.email)
+    expect(wrapper.attribute('data-user').value).to eq user.id.to_s
+  end
+
+  def expect_to_have_mailto_link(doc, email:, trailer:)
+    wrapper = find_user_wrapper(doc, trailer)
+
+    expect_to_have_links_with_url_and_avatar(wrapper, "mailto:#{CGI.escape_html(email)}", email)
+  end
+
+  def expect_to_have_links_with_url_and_avatar(doc, url, email)
+    expect(doc).not_to be_nil
+    expect(doc.xpath("a[position()<3 and @href='#{url}']").size).to eq 2
+    expect(doc.xpath("a[position()=3 and @href='mailto:#{CGI.escape_html(email)}']").size).to eq 1
+    expect(doc.css('img').size).to eq 1
+  end
+
+  def find_user_wrapper(doc, trailer)
+    doc.xpath("descendant-or-self::node()[@data-trailer='#{trailer}']").first
+  end
+
+  def build_commit_message(trailer:, name:, email:)
+    message = trailer_line(trailer, name, email)
+
+    [message, commit_html(message)]
+  end
+
+  def trailer_line(trailer, name, email)
+    "#{trailer} #{name} <#{email}>"
+  end
+
+  def commit_html(message)
+    "<pre>#{CGI.escape_html(message)}</pre>"
+  end
+end