Add latest changes from gitlab-org/gitlab@master

Gemfile

@@ -484,3 +484,6 @@ gem 'countries', '~> 3.0'
 gem 'retriable', '~> 3.1.2'
 gem 'liquid', '~> 4.0'
+
+# LRU cache
+gem 'lru_redux'

Gemfile.lock

@@ -598,6 +598,7 @@ GEM
     loofah (2.4.0)
       crass (~> 1.0.2)
       nokogiri (>= 1.5.9)
+    lru_redux (1.1.0)
     lumberjack (1.0.13)
     mail (2.7.1)
       mini_mime (>= 0.1.1)
@@ -1263,6 +1264,7 @@ DEPENDENCIES
   liquid (~> 4.0)
   lograge (~> 0.5)
   loofah (~> 2.2)
+  lru_redux
   mail_room (~> 0.10.0)
   marginalia (~> 1.8.0)
   memory_profiler (~> 0.9)

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

 <script>
 import Vue from 'vue';
-import { mapActions, mapState, mapGetters } from 'vuex';
+import { mapActions, mapGetters, mapState } from 'vuex';
 import { GlButton, GlLoadingIcon } from '@gitlab/ui';
 import { __ } from '~/locale';
 import FindFile from '~/vue_shared/components/file_finder/index.vue';

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

+<script>
+import { mapActions, mapState } from 'vuex';
+import _ from 'underscore';
+import tooltip from '~/vue_shared/directives/tooltip';
+import Icon from '~/vue_shared/components/icon.vue';
+import ResizablePanel from '../resizable_panel.vue';
+import { GlSkeletonLoading } from '@gitlab/ui';
+
+export default {
+  name: 'CollapsibleSidebar',
+  directives: {
+    tooltip,
+  },
+  components: {
+    Icon,
+    ResizablePanel,
+    GlSkeletonLoading,
+  },
+  props: {
+    extensionTabs: {
+      type: Array,
+      required: false,
+      default: () => [],
+    },
+    side: {
+      type: String,
+      required: true,
+    },
+    width: {
+      type: Number,
+      required: true,
+    },
+  },
+  computed: {
+    ...mapState(['loading']),
+    ...mapState({
+      isOpen(state) {
+        return state[this.namespace].isOpen;
+      },
+      currentView(state) {
+        return state[this.namespace].currentView;
+      },
+      isActiveView(state, getters) {
+        return getters[`${this.namespace}/isActiveView`];
+      },
+      isAliveView(_state, getters) {
+        return getters[`${this.namespace}/isAliveView`];
+      },
+    }),
+    namespace() {
+      // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings
+      return `${this.side}Pane`;
+    },
+    tabs() {
+      return this.extensionTabs.filter(tab => tab.show);
+    },
+    tabViews() {
+      return _.flatten(this.tabs.map(tab => tab.views));
+    },
+    aliveTabViews() {
+      return this.tabViews.filter(view => this.isAliveView(view.name));
+    },
+    otherSide() {
+      return this.side === 'right' ? 'left' : 'right';
+    },
+  },
+  methods: {
+    ...mapActions({
+      toggleOpen(dispatch) {
+        return dispatch(`${this.namespace}/toggleOpen`);
+      },
+      open(dispatch, view) {
+        return dispatch(`${this.namespace}/open`, view);
+      },
+    }),
+    clickTab(e, tab) {
+      e.target.blur();
+
+      if (this.isActiveTab(tab)) {
+        this.toggleOpen();
+      } else {
+        this.open(tab.views[0]);
+      }
+    },
+    isActiveTab(tab) {
+      return tab.views.some(view => this.isActiveView(view.name));
+    },
+    buttonClasses(tab) {
+      return [
+        this.side === 'right' ? 'is-right' : '',
+        this.isActiveTab(tab) && this.isOpen ? 'active' : '',
+        ...(tab.buttonClasses || []),
+      ];
+    },
+  },
+};
+</script>
+
+<template>
+  <div
+    :class="`ide-${side}-sidebar`"
+    :data-qa-selector="`ide_${side}_sidebar`"
+    class="multi-file-commit-panel ide-sidebar"
+  >
+    <resizable-panel
+      v-show="isOpen"
+      :collapsible="false"
+      :initial-width="width"
+      :min-size="width"
+      :class="`ide-${side}-sidebar-${currentView}`"
+      :side="side"
+      class="multi-file-commit-panel-inner"
+    >
+      <div class="h-100 d-flex flex-column align-items-stretch">
+        <slot v-if="isOpen" name="header"></slot>
+        <div
+          v-for="tabView in aliveTabViews"
+          v-show="isActiveView(tabView.name)"
+          :key="tabView.name"
+          class="flex-fill js-tab-view"
+        >
+          <component :is="tabView.component" />
+        </div>
+        <slot name="footer"></slot>
+      </div>
+    </resizable-panel>
+    <nav class="ide-activity-bar">
+      <ul class="list-unstyled">
+        <li>
+          <slot name="header-icon"></slot>
+        </li>
+        <li v-for="tab of tabs" :key="tab.title">
+          <button
+            v-tooltip
+            :title="tab.title"
+            :aria-label="tab.title"
+            :class="buttonClasses(tab)"
+            data-container="body"
+            :data-placement="otherSide"
+            :data-qa-selector="`${tab.title.toLowerCase()}_tab_button`"
+            class="ide-sidebar-link"
+            type="button"
+            @click="clickTab($event, tab)"
+          >
+            <icon :size="16" :name="tab.icon" />
+          </button>
+        </li>
+      </ul>
+    </nav>
+  </div>
+</template>

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

 <script>
-import { mapActions, mapState, mapGetters } from 'vuex';
-import _ from 'underscore';
+import { mapGetters, mapState } from 'vuex';
 import { __ } from '~/locale';
-import tooltip from '../../../vue_shared/directives/tooltip';
-import Icon from '../../../vue_shared/components/icon.vue';
+import CollapsibleSidebar from './collapsible_sidebar.vue';
 import { rightSidebarViews } from '../../constants';
+import MergeRequestInfo from '../merge_requests/info.vue';
 import PipelinesList from '../pipelines/list.vue';
 import JobsDetail from '../jobs/detail.vue';
-import MergeRequestInfo from '../merge_requests/info.vue';
-import ResizablePanel from '../resizable_panel.vue';
 import Clientside from '../preview/clientside.vue';
 export default {
-  directives: {
-    tooltip,
-  },
+  name: 'RightPane',
   components: {
-    Icon,
-    PipelinesList,
-    JobsDetail,
-    ResizablePanel,
-    MergeRequestInfo,
-    Clientside,
+    CollapsibleSidebar,
   },
   props: {
     extensionTabs: {
@@ -32,103 +22,40 @@ export default {
   },
   computed: {
     ...mapState(['currentMergeRequestId', 'clientsidePreviewEnabled']),
-    ...mapState('rightPane', ['isOpen', 'currentView']),
     ...mapGetters(['packageJson']),
-    ...mapGetters('rightPane', ['isActiveView', 'isAliveView']),
     showLivePreview() {
       return this.packageJson && this.clientsidePreviewEnabled;
     },
-    defaultTabs() {
+    rightExtensionTabs() {
       return [
         {
-          show: this.currentMergeRequestId,
+          show: Boolean(this.currentMergeRequestId),
           title: __('Merge Request'),
-          views: [rightSidebarViews.mergeRequestInfo],
+          views: [{ component: MergeRequestInfo, ...rightSidebarViews.mergeRequestInfo }],
           icon: 'text-description',
         },
         {
           show: true,
           title: __('Pipelines'),
-          views: [rightSidebarViews.pipelines, rightSidebarViews.jobsDetail],
+          views: [
+            { component: PipelinesList, ...rightSidebarViews.pipelines },
+            { component: JobsDetail, ...rightSidebarViews.jobsDetail },
+          ],
           icon: 'rocket',
         },
         {
           show: this.showLivePreview,
           title: __('Live preview'),
-          views: [rightSidebarViews.clientSidePreview],
+          views: [{ component: Clientside, ...rightSidebarViews.clientSidePreview }],
           icon: 'live-preview',
         },
+        ...this.extensionTabs,
       ];
     },
-    tabs() {
-      return this.defaultTabs.concat(this.extensionTabs).filter(tab => tab.show);
-    },
-    tabViews() {
-      return _.flatten(this.tabs.map(tab => tab.views));
-    },
-    aliveTabViews() {
-      return this.tabViews.filter(view => this.isAliveView(view.name));
-    },
-  },
-  methods: {
-    ...mapActions('rightPane', ['toggleOpen', 'open']),
-    clickTab(e, tab) {
-      e.target.blur();
-
-      if (this.isActiveTab(tab)) {
-        this.toggleOpen();
-      } else {
-        this.open(tab.views[0]);
-      }
-    },
-    isActiveTab(tab) {
-      return tab.views.some(view => this.isActiveView(view.name));
-    },
   },
 };
 </script>
 <template>
-  <div class="multi-file-commit-panel ide-right-sidebar" data-qa-selector="ide_right_sidebar">
-    <resizable-panel
-      v-show="isOpen"
-      :collapsible="false"
-      :initial-width="350"
-      :min-size="350"
-      :class="`ide-right-sidebar-${currentView}`"
-      side="right"
-      class="multi-file-commit-panel-inner"
-    >
-      <div
-        v-for="tabView in aliveTabViews"
-        v-show="isActiveView(tabView.name)"
-        :key="tabView.name"
-        class="h-100"
-      >
-        <component :is="tabView.component || tabView.name" />
-      </div>
-    </resizable-panel>
-    <nav class="ide-activity-bar">
-      <ul class="list-unstyled">
-        <li v-for="tab of tabs" :key="tab.title">
-          <button
-            v-tooltip
-            :title="tab.title"
-            :aria-label="tab.title"
-            :class="{
-              active: isActiveTab(tab) && isOpen,
-            }"
-            data-container="body"
-            data-placement="left"
-            :data-qa-selector="`${tab.title.toLowerCase()}_tab_button`"
-            class="ide-sidebar-link is-right"
-            type="button"
-            @click="clickTab($event, tab)"
-          >
-            <icon :size="16" :name="tab.icon" />
-          </button>
-        </li>
-      </ul>
-    </nav>
-  </div>
+  <collapsible-sidebar :extension-tabs="rightExtensionTabs" side="right" :width="350" />
 </template>

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

@@ -33,19 +33,6 @@ export const setPanelCollapsedStatus = ({ commit }, { side, collapsed }) => {
   }
 };
-export const toggleRightPanelCollapsed = ({ dispatch, state }, e = undefined) => {
-  if (e) {
-    $(e.currentTarget)
-      .tooltip('hide')
-      .blur();
-  }
-
-  dispatch('setPanelCollapsedStatus', {
-    side: 'right',
-    collapsed: !state.rightPanelCollapsed,
-  });
-};
-
 export const setResizingStatus = ({ commit }, resizing) => {
   commit(types.SET_RESIZING_STATUS, resizing);
 };

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

 import * as types from './mutation_types';
-export const toggleOpen = ({ dispatch, state }, view) => {
+export const toggleOpen = ({ dispatch, state }) => {
   if (state.isOpen) {
     dispatch('close');
   } else {
-    dispatch('open', view);
+    dispatch('open');
   }
 };
-export const open = ({ commit }, view) => {
+export const open = ({ state, commit }, view) => {
   commit(types.SET_OPEN, true);
-  if (view) {
+  if (view && view.name !== state.currentView) {
     const { name, keepAlive } = view;
     commit(types.SET_CURRENT_VIEW, name);

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

@@ -11,7 +11,6 @@ export const SET_LINKS = 'SET_LINKS';
 // Project Mutation Types
 export const SET_PROJECT = 'SET_PROJECT';
 export const SET_CURRENT_PROJECT = 'SET_CURRENT_PROJECT';
-export const TOGGLE_PROJECT_OPEN = 'TOGGLE_PROJECT_OPEN';
 export const TOGGLE_EMPTY_STATE = 'TOGGLE_EMPTY_STATE';
 // Merge Request Mutation Types

app/assets/stylesheets/page_bundles/ide.scss

@@ -740,6 +740,7 @@ $ide-commit-header-height: 48px;
 .ide-sidebar-link {
   display: flex;
   align-items: center;
+  justify-content: center;
   position: relative;
   height: 60px;
   width: 100%;
@@ -1076,10 +1077,12 @@ $ide-commit-header-height: 48px;
   }
 }
-.ide-right-sidebar {
+.ide-sidebar {
   width: auto;
   min-width: 60px;
+}
+.ide-right-sidebar {
   .ide-activity-bar {
     border-left: 1px solid $white-dark;
   }

app/controllers/projects/snippets_controller.rb

@@ -46,8 +46,8 @@ class Projects::SnippetsController < Projects::ApplicationController
   def create
     create_params = snippet_params.merge(spammable_params)
-
-    @snippet = CreateSnippetService.new(@project, current_user, create_params).execute
+    service_response = Snippets::CreateService.new(project, current_user, create_params).execute
+    @snippet = service_response.payload[:snippet]
     recaptcha_check_with_fallback { render :new }
   end
@@ -55,7 +55,8 @@ class Projects::SnippetsController < Projects::ApplicationController
   def update
     update_params = snippet_params.merge(spammable_params)
-    UpdateSnippetService.new(project, current_user, @snippet, update_params).execute
+    service_response = Snippets::UpdateService.new(project, current_user, update_params).execute(@snippet)
+    @snippet = service_response.payload[:snippet]
     recaptcha_check_with_fallback { render :edit }
   end
@@ -89,11 +90,17 @@ class Projects::SnippetsController < Projects::ApplicationController
   end
   def destroy
-    return access_denied! unless can?(current_user, :admin_project_snippet, @snippet)
-
-    @snippet.destroy
-
-    redirect_to project_snippets_path(@project), status: :found
+    service_response = Snippets::DestroyService.new(current_user, @snippet).execute
+
+    if service_response.success?
+      redirect_to project_snippets_path(project), status: :found
+    elsif service_response.http_status == 403
+      access_denied!
+    else
+      redirect_to project_snippet_path(project, @snippet),
+                  status: :found,
+                  alert: service_response.message
+    end
   end
   protected

app/controllers/snippets_controller.rb

@@ -50,8 +50,8 @@ class SnippetsController < ApplicationController
   def create
     create_params = snippet_params.merge(spammable_params)
-
-    @snippet = CreateSnippetService.new(nil, current_user, create_params).execute
+    service_response = Snippets::CreateService.new(nil, current_user, create_params).execute
+    @snippet = service_response.payload[:snippet]
     move_temporary_files if @snippet.valid? && params[:files]
@@ -61,7 +61,8 @@ class SnippetsController < ApplicationController
   def update
     update_params = snippet_params.merge(spammable_params)
-    UpdateSnippetService.new(nil, current_user, @snippet, update_params).execute
+    service_response = Snippets::UpdateService.new(nil, current_user, update_params).execute(@snippet)
+    @snippet = service_response.payload[:snippet]
     recaptcha_check_with_fallback { render :edit }
   end
@@ -96,11 +97,17 @@ class SnippetsController < ApplicationController
   end
   def destroy
-    return access_denied! unless can?(current_user, :admin_personal_snippet, @snippet)
-
-    @snippet.destroy
-    redirect_to snippets_path, status: :found
+    service_response = Snippets::DestroyService.new(current_user, @snippet).execute
+    if service_response.success?
+      redirect_to dashboard_snippets_path, status: :found
+    elsif service_response.http_status == 403
+      access_denied!
+    else
+      redirect_to snippet_path(@snippet),
+                  status: :found,
+                  alert: service_response.message
+    end
   end
   protected

app/graphql/mutations/snippets/create.rb

@@ -45,9 +45,10 @@ module Mutations
           raise_resource_not_available_error!
         end
-        snippet = CreateSnippetService.new(project,
+        service_response = ::Snippets::CreateService.new(project,
                                            context[:current_user],
                                            args).execute
+        snippet = service_response.payload[:snippet]
         {
           snippet: snippet.valid? ? snippet : nil,

app/graphql/mutations/snippets/destroy.rb

@@ -15,8 +15,8 @@ module Mutations
       def resolve(id:)
         snippet = authorized_find!(id: id)
-        result = snippet.destroy
-        errors = result ? [] : [ERROR_MSG]
+        response = ::Snippets::DestroyService.new(current_user, snippet).execute
+        errors = response.success? ? [] : [ERROR_MSG]
         {
           errors: errors

app/graphql/mutations/snippets/update.rb

@@ -33,13 +33,13 @@ module Mutations
       def resolve(args)
         snippet = authorized_find!(id: args.delete(:id))
-        result = UpdateSnippetService.new(snippet.project,
+        result = ::Snippets::UpdateService.new(snippet.project,
                                           context[:current_user],
-                                          snippet,
-                                          args).execute
+                                          args).execute(snippet)
+        snippet = result.payload[:snippet]
         {
-          snippet: result ? snippet : snippet.reset,
+          snippet: result.success? ? snippet : snippet.reset,
           errors: errors_on_object(snippet)
         }
       end

app/services/create_snippet_service.rb

-# frozen_string_literal: true
-
-class CreateSnippetService < BaseService
-  include SpamCheckMethods
-
-  def execute
-    filter_spam_check_params
-
-    snippet = if project
-                project.snippets.build(params)
-              else
-                PersonalSnippet.new(params)
-              end
-
-    unless Gitlab::VisibilityLevel.allowed_for?(current_user, snippet.visibility_level)
-      deny_visibility_level(snippet)
-      return snippet
-    end
-
-    snippet.author = current_user
-
-    spam_check(snippet, current_user)
-
-    snippet_saved = snippet.with_transaction_returning_status do
-      snippet.save && snippet.store_mentions!
-    end
-
-    if snippet_saved
-      UserAgentDetailService.new(snippet, @request).create
-      Gitlab::UsageDataCounters::SnippetCounter.count(:create)
-    end
-
-    snippet
-  end
-end

app/services/snippets/base_service.rb

+# frozen_string_literal: true
+
+module Snippets
+  class BaseService < ::BaseService
+    private
+
+    def snippet_error_response(snippet, http_status)
+      ServiceResponse.error(
+        message: snippet.errors.full_messages.to_sentence,
+        http_status: http_status,
+        payload: { snippet: snippet }
+      )
+    end
+  end
+end

app/services/snippets/create_service.rb

+# frozen_string_literal: true
+
+module Snippets
+  class CreateService < Snippets::BaseService
+    include SpamCheckMethods
+
+    def execute
+      filter_spam_check_params
+
+      snippet = if project
+                  project.snippets.build(params)
+                else
+                  PersonalSnippet.new(params)
+                end
+
+      unless Gitlab::VisibilityLevel.allowed_for?(current_user, snippet.visibility_level)
+        deny_visibility_level(snippet)
+
+        return snippet_error_response(snippet, 403)
+      end
+
+      snippet.author = current_user
+
+      spam_check(snippet, current_user)
+
+      snippet_saved = snippet.with_transaction_returning_status do
+        snippet.save && snippet.store_mentions!
+      end
+
+      if snippet_saved
+        UserAgentDetailService.new(snippet, @request).create
+        Gitlab::UsageDataCounters::SnippetCounter.count(:create)
+
+        ServiceResponse.success(payload: { snippet: snippet } )
+      else
+        snippet_error_response(snippet, 400)
+      end
+    end
+  end
+end

app/services/snippets/destroy_service.rb

+# frozen_string_literal: true
+
+module Snippets
+  class DestroyService
+    include Gitlab::Allowable
+
+    attr_reader :current_user, :project
+
+    def initialize(user, snippet)
+      @current_user = user
+      @snippet = snippet
+      @project = snippet&.project
+    end
+
+    def execute
+      if snippet.nil?
+        return service_response_error('No snippet found.', 404)
+      end
+
+      unless user_can_delete_snippet?
+        return service_response_error(
+          "You don't have access to delete this snippet.",
+          403
+        )
+      end
+
+      if snippet.destroy
+        ServiceResponse.success(message: 'Snippet was deleted.')
+      else
+        service_response_error('Failed to remove snippet.', 400)
+      end
+    end
+
+    private
+
+    attr_reader :snippet
+
+    def user_can_delete_snippet?
+      return can?(current_user, :admin_project_snippet, snippet) if project
+
+      can?(current_user, :admin_personal_snippet, snippet)
+    end
+
+    def service_response_error(message, http_status)
+      ServiceResponse.error(message: message, http_status: http_status)
+    end
+  end
+end

app/services/snippets/update_service.rb

+# frozen_string_literal: true
+
+module Snippets
+  class UpdateService < Snippets::BaseService
+    include SpamCheckMethods
+
+    def execute(snippet)
+      # check that user is allowed to set specified visibility_level
+      new_visibility = visibility_level
+
+      if new_visibility && new_visibility.to_i != snippet.visibility_level
+        unless Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
+          deny_visibility_level(snippet, new_visibility)
+
+          return snippet_error_response(snippet, 403)
+        end
+      end
+
+      filter_spam_check_params
+      snippet.assign_attributes(params)
+      spam_check(snippet, current_user)
+
+      snippet_saved = snippet.with_transaction_returning_status do
+        snippet.save && snippet.store_mentions!
+      end
+
+      if snippet_saved
+        Gitlab::UsageDataCounters::SnippetCounter.count(:update)
+
+        ServiceResponse.success(payload: { snippet: snippet } )
+      else
+        snippet_error_response(snippet, 400)
+      end
+    end
+  end
+end

app/services/update_snippet_service.rb

-# frozen_string_literal: true
-
-class UpdateSnippetService < BaseService
-  include SpamCheckMethods
-
-  attr_accessor :snippet
-
-  def initialize(project, user, snippet, params)
-    super(project, user, params)
-    @snippet = snippet
-  end
-
-  def execute
-    # check that user is allowed to set specified visibility_level
-    new_visibility = visibility_level
-
-    if new_visibility && new_visibility.to_i != snippet.visibility_level
-      unless Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
-        deny_visibility_level(snippet, new_visibility)
-        return snippet
-      end
-    end
-
-    filter_spam_check_params
-    snippet.assign_attributes(params)
-    spam_check(snippet, current_user)
-
-    snippet_saved = snippet.with_transaction_returning_status do
-      snippet.save && snippet.store_mentions!
-    end
-
-    if snippet_saved
-      Gitlab::UsageDataCounters::SnippetCounter.count(:update)
-    end
-  end
-end