diff options
52 files changed, 1290 insertions, 512 deletions
@@ -484,3 +484,6 @@ gem 'countries', '~> 3.0' gem 'retriable', '~> 3.1.2' gem 'liquid', '~> 4.0' + +# LRU cache +gem 'lru_redux' diff --git a/Gemfile.lock b/Gemfile.lock index 2be06106f3a..0bf630b42ef 100644 --- a/Gemfile.lock +++ b/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) diff --git a/app/assets/javascripts/ide/components/ide.vue b/app/assets/javascripts/ide/components/ide.vue index 363a8f43033..6ed863c9c2e 100644 --- a/app/assets/javascripts/ide/components/ide.vue +++ b/app/assets/javascripts/ide/components/ide.vue @@ -1,6 +1,6 @@ <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'; diff --git a/app/assets/javascripts/ide/components/panes/collapsible_sidebar.vue b/app/assets/javascripts/ide/components/panes/collapsible_sidebar.vue new file mode 100644 index 00000000000..d5a123edb80 --- /dev/null +++ b/app/assets/javascripts/ide/components/panes/collapsible_sidebar.vue @@ -0,0 +1,151 @@ +<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> diff --git a/app/assets/javascripts/ide/components/panes/right.vue b/app/assets/javascripts/ide/components/panes/right.vue index 200391282e7..40ed7d9c422 100644 --- a/app/assets/javascripts/ide/components/panes/right.vue +++ b/app/assets/javascripts/ide/components/panes/right.vue @@ -1,27 +1,17 @@ <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> diff --git a/app/assets/javascripts/ide/stores/actions.js b/app/assets/javascripts/ide/stores/actions.js index cb027358d46..34e7cc304dd 100644 --- a/app/assets/javascripts/ide/stores/actions.js +++ b/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); }; diff --git a/app/assets/javascripts/ide/stores/modules/pane/actions.js b/app/assets/javascripts/ide/stores/modules/pane/actions.js index 7f5d167a14f..a8fcdf539ec 100644 --- a/app/assets/javascripts/ide/stores/modules/pane/actions.js +++ b/app/assets/javascripts/ide/stores/modules/pane/actions.js @@ -1,17 +1,17 @@ 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); diff --git a/app/assets/javascripts/ide/stores/mutation_types.js b/app/assets/javascripts/ide/stores/mutation_types.js index f0b4718d025..4dde53a9fdf 100644 --- a/app/assets/javascripts/ide/stores/mutation_types.js +++ b/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 diff --git a/app/assets/stylesheets/page_bundles/ide.scss b/app/assets/stylesheets/page_bundles/ide.scss index 977fc8329b6..420271c9a1e 100644 --- a/app/assets/stylesheets/page_bundles/ide.scss +++ b/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; } diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb index dbd11c8ddc8..daddd9dd485 100644 --- a/app/controllers/projects/snippets_controller.rb +++ b/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 diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 54774df5e76..fc073e47368 100644 --- a/app/controllers/snippets_controller.rb +++ b/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 + service_response = Snippets::DestroyService.new(current_user, @snippet).execute - redirect_to snippets_path, status: :found + 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 diff --git a/app/graphql/mutations/snippets/create.rb b/app/graphql/mutations/snippets/create.rb index c439a6cfc2f..4e0e65d09a9 100644 --- a/app/graphql/mutations/snippets/create.rb +++ b/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, diff --git a/app/graphql/mutations/snippets/destroy.rb b/app/graphql/mutations/snippets/destroy.rb index 115fcfd6488..dc9a1e82575 100644 --- a/app/graphql/mutations/snippets/destroy.rb +++ b/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 diff --git a/app/graphql/mutations/snippets/update.rb b/app/graphql/mutations/snippets/update.rb index 27c232bc7f8..b6bdcb9b67b 100644 --- a/app/graphql/mutations/snippets/update.rb +++ b/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 diff --git a/app/services/create_snippet_service.rb b/app/services/create_snippet_service.rb deleted file mode 100644 index 56c175ebdb1..00000000000 --- a/app/services/create_snippet_service.rb +++ /dev/null @@ -1,35 +0,0 @@ -# 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 diff --git a/app/services/snippets/base_service.rb b/app/services/snippets/base_service.rb new file mode 100644 index 00000000000..2b450db0b83 --- /dev/null +++ b/app/services/snippets/base_service.rb @@ -0,0 +1,15 @@ +# 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 diff --git a/app/services/snippets/create_service.rb b/app/services/snippets/create_service.rb new file mode 100644 index 00000000000..250e99c466a --- /dev/null +++ b/app/services/snippets/create_service.rb @@ -0,0 +1,40 @@ +# 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 diff --git a/app/services/snippets/destroy_service.rb b/app/services/snippets/destroy_service.rb new file mode 100644 index 00000000000..f253817d94f --- /dev/null +++ b/app/services/snippets/destroy_service.rb @@ -0,0 +1,48 @@ +# 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 diff --git a/app/services/snippets/update_service.rb b/app/services/snippets/update_service.rb new file mode 100644 index 00000000000..8d2c8cac148 --- /dev/null +++ b/app/services/snippets/update_service.rb @@ -0,0 +1,36 @@ +# 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 diff --git a/app/services/update_snippet_service.rb b/app/services/update_snippet_service.rb deleted file mode 100644 index d365e4e7b2b..00000000000 --- a/app/services/update_snippet_service.rb +++ /dev/null @@ -1,36 +0,0 @@ -# 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 diff --git a/changelogs/unreleased/197146-expose-active-field-in-the-error-tracking-api.yml b/changelogs/unreleased/197146-expose-active-field-in-the-error-tracking-api.yml new file mode 100644 index 00000000000..56860e89d0c --- /dev/null +++ b/changelogs/unreleased/197146-expose-active-field-in-the-error-tracking-api.yml @@ -0,0 +1,5 @@ +--- +title: Expose `active` field in the Error Tracking API +merge_request: 23150 +author: +type: added diff --git a/changelogs/unreleased/7132-document-go-support.yml b/changelogs/unreleased/7132-document-go-support.yml new file mode 100644 index 00000000000..e6b61300bf9 --- /dev/null +++ b/changelogs/unreleased/7132-document-go-support.yml @@ -0,0 +1,5 @@ +--- +title: Document go support for dependency scanning +merge_request: 22806 +author: +type: added diff --git a/changelogs/unreleased/lru-object-caching-group-project-object-builder.yml b/changelogs/unreleased/lru-object-caching-group-project-object-builder.yml new file mode 100644 index 00000000000..bc3f6379de6 --- /dev/null +++ b/changelogs/unreleased/lru-object-caching-group-project-object-builder.yml @@ -0,0 +1,5 @@ +--- +title: LRU object caching for GroupProjectObjectBuilder +merge_request: 21823 +author: +type: performance diff --git a/db/post_migrate/20200113151354_remove_creations_in_gitlab_subscription_histories.rb b/db/post_migrate/20200113151354_remove_creations_in_gitlab_subscription_histories.rb new file mode 100644 index 00000000000..39ca5124b32 --- /dev/null +++ b/db/post_migrate/20200113151354_remove_creations_in_gitlab_subscription_histories.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class RemoveCreationsInGitlabSubscriptionHistories < ActiveRecord::Migration[5.2] + DOWNTIME = false + GITLAB_SUBSCRIPTION_CREATED = 0 + + def up + return unless Gitlab.com? + + delete_sql = "DELETE FROM gitlab_subscription_histories WHERE change_type=#{GITLAB_SUBSCRIPTION_CREATED} RETURNING *" + + records = execute(delete_sql) + + logger = Gitlab::BackgroundMigration::Logger.build + records.to_a.each do |record| + logger.info record.as_json.merge(message: "gitlab_subscription_histories with change_type=0 was deleted") + end + end + + def down + # There's no way to restore, and the data is useless + # all the data to be deleted in case needed https://gitlab.com/gitlab-org/gitlab/uploads/7409379b0ed658624f5d33202b5668a1/gitlab_subscription_histories_change_type_0.sql.txt + end +end diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md index d5e141459b6..7d3be9e1bd3 100644 --- a/doc/administration/audit_events.md +++ b/doc/administration/audit_events.md @@ -18,7 +18,7 @@ permission level, who added a new user, or who removed a user. ## Use-cases -- Check who was the person who changed the permission level of a particular +- Check who the person was that changed the permission level of a particular user for a project in GitLab. - Use it to track which users have access to a certain group of projects in GitLab, and who gave them that permission level. diff --git a/doc/api/error_tracking.md b/doc/api/error_tracking.md index 77dc7f8629f..3c1fbb7dc7a 100644 --- a/doc/api/error_tracking.md +++ b/doc/api/error_tracking.md @@ -24,6 +24,7 @@ Example response: ```json { + "active": true, "project_name": "sample sentry project", "sentry_external_url": "https://sentry.io/myawesomeproject/project", "api_url": "https://sentry.io/api/0/projects/myawesomeproject/project" diff --git a/doc/api/merge_request_approvals.md b/doc/api/merge_request_approvals.md index f98fb7a5a0d..4552a56d808 100644 --- a/doc/api/merge_request_approvals.md +++ b/doc/api/merge_request_approvals.md @@ -762,7 +762,7 @@ will be used. You can update merge request approval rules using the following endpoint: ``` -PUT /projects/:id/merge_request/:merge_request_iid/approval_rules/:approval_rule_id +PUT /projects/:id/merge_requests/:merge_request_iid/approval_rules/:approval_rule_id ``` **Important:** Approvers and groups not in the `users`/`groups` param will be **removed** diff --git a/doc/user/application_security/dependency_scanning/index.md b/doc/user/application_security/dependency_scanning/index.md index 2a11f7a1b7c..c47fd6f1ff1 100644 --- a/doc/user/application_security/dependency_scanning/index.md +++ b/doc/user/application_security/dependency_scanning/index.md @@ -65,6 +65,7 @@ The following languages and dependency managers are supported. | Python ([poetry](https://poetry.eustace.io/)) | not currently ([issue](https://gitlab.com/gitlab-org/gitlab/issues/7006 "Support Poetry in Dependency Scanning")) | not available | | Ruby ([gem](https://rubygems.org/)) | yes | [gemnasium](https://gitlab.com/gitlab-org/security-products/gemnasium), [bundler-audit](https://github.com/rubysec/bundler-audit) | | Scala ([sbt](https://www.scala-sbt.org/)) | yes | [gemnasium](https://gitlab.com/gitlab-org/security-products/gemnasium) | +| Go ([go](https://golang.org/)) | yes (alpha) | [gemnasium](https://gitlab.com/gitlab-org/security-products/gemnasium) | ## Configuration diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index f5116f22cd8..6016837a769 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -460,11 +460,11 @@ You can also use following variables besides static text: | `%{file_path}` | The path of the file the Suggestion is applied to. | `docs/index.md` | | `%{branch_name}` | The name of the branch the Suggestion is applied on. | `my-feature-branch` | | `%{username}` | The username of the user applying the Suggestion. | `user_1` | -| `%{user_full_name}` | The full name of the user applying the Suggestion. | `**User 1** | +| `%{user_full_name}` | The full name of the user applying the Suggestion. | **User 1** | For example, to customize the commit message to output **Addresses user_1's review**, set the custom text to -`Adresses %{username}'s review`. +`Addresses %{username}'s review`. NOTE: **Note:** Custom commit messages for each applied Suggestion will be diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 2b26565018f..985c1babdb5 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -66,6 +66,7 @@ The following table depicts the various user permission levels in a project. | View confidential issues | (*2*) | ✓ | ✓ | ✓ | ✓ | | Assign issues | | ✓ | ✓ | ✓ | ✓ | | Label issues | | ✓ | ✓ | ✓ | ✓ | +| Set issue weight | | ✓ | ✓ | ✓ | ✓ | | Lock issue threads | | ✓ | ✓ | ✓ | ✓ | | Manage issue tracker | | ✓ | ✓ | ✓ | ✓ | | Manage related issues **(STARTER)** | | ✓ | ✓ | ✓ | ✓ | diff --git a/doc/user/project/settings/import_export.md b/doc/user/project/settings/import_export.md index 9449ab6d10f..f5cfa256f5d 100644 --- a/doc/user/project/settings/import_export.md +++ b/doc/user/project/settings/import_export.md @@ -100,7 +100,7 @@ For more details on the specific data persisted in a project export, see the  1. Alternatively, you can come back to the project settings and download the - file from there, or generate a new export. Once the file available, the page + file from there, or generate a new export. Once the file is available, the page should show the **Download export** button:  diff --git a/lib/api/entities/error_tracking.rb b/lib/api/entities/error_tracking.rb index 0180572a6cc..c762c274486 100644 --- a/lib/api/entities/error_tracking.rb +++ b/lib/api/entities/error_tracking.rb @@ -4,6 +4,7 @@ module API module Entities module ErrorTracking class ProjectSetting < Grape::Entity + expose :enabled, as: :active expose :project_name expose :sentry_external_url expose :api_url diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb index b4545295d54..ecada843972 100644 --- a/lib/api/project_snippets.rb +++ b/lib/api/project_snippets.rb @@ -64,7 +64,8 @@ module API snippet_params = declared_params(include_missing: false).merge(request: request, api: true) snippet_params[:content] = snippet_params.delete(:code) if snippet_params[:code].present? - snippet = CreateSnippetService.new(user_project, current_user, snippet_params).execute + service_response = ::Snippets::CreateService.new(user_project, current_user, snippet_params).execute + snippet = service_response.payload[:snippet] render_spam_error! if snippet.spam? @@ -103,8 +104,8 @@ module API snippet_params[:content] = snippet_params.delete(:code) if snippet_params[:code].present? - UpdateSnippetService.new(user_project, current_user, snippet, - snippet_params).execute + service_response = ::Snippets::UpdateService.new(user_project, current_user, snippet_params).execute(snippet) + snippet = service_response.payload[:snippet] render_spam_error! if snippet.spam? @@ -127,7 +128,14 @@ module API authorize! :admin_project_snippet, snippet - destroy_conditionally!(snippet) + destroy_conditionally!(snippet) do |snippet| + service = ::Snippets::DestroyService.new(current_user, snippet) + response = service.execute + + if response.error? + render_api_error!({ error: response.message }, response.http_status) + end + end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index fd5422f2e2c..a7dab373b7f 100644 --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -75,7 +75,8 @@ module API end post do attrs = declared_params(include_missing: false).merge(request: request, api: true) - snippet = CreateSnippetService.new(nil, current_user, attrs).execute + service_response = ::Snippets::CreateService.new(nil, current_user, attrs).execute + snippet = service_response.payload[:snippet] render_spam_error! if snippet.spam? @@ -108,8 +109,8 @@ module API authorize! :update_personal_snippet, snippet attrs = declared_params(include_missing: false).merge(request: request, api: true) - - UpdateSnippetService.new(nil, current_user, snippet, attrs).execute + service_response = ::Snippets::UpdateService.new(nil, current_user, attrs).execute(snippet) + snippet = service_response.payload[:snippet] render_spam_error! if snippet.spam? @@ -133,7 +134,14 @@ module API authorize! :admin_personal_snippet, snippet - destroy_conditionally!(snippet) + destroy_conditionally!(snippet) do |snippet| + service = ::Snippets::DestroyService.new(current_user, snippet) + response = service.execute + + if response.error? + render_api_error!({ error: response.message }, response.http_status) + end + end end desc 'Get a raw snippet' do diff --git a/lib/gitlab/import_export/group_project_object_builder.rb b/lib/gitlab/import_export/group_project_object_builder.rb index 2e7ab3d4b69..78ba4894459 100644 --- a/lib/gitlab/import_export/group_project_object_builder.rb +++ b/lib/gitlab/import_export/group_project_object_builder.rb @@ -12,6 +12,13 @@ module Gitlab # # It also adds some logic around Group Labels/Milestones for edge cases. class GroupProjectObjectBuilder + # Cache keeps 1000 entries at most, 1000 is chosen based on: + # - one cache entry uses around 0.5K memory, 1000 items uses around 500K. + # (leave some buffer it should be less than 1M). It is afforable cost for project import. + # - for projects in Gitlab.com, it seems 1000 entries for labels/milestones is enough. + # For example, gitlab has ~970 labels and 26 milestones. + LRU_CACHE_SIZE = 1000 + def self.build(*args) Project.transaction do new(*args).find @@ -23,17 +30,34 @@ module Gitlab @attributes = attributes @group = @attributes['group'] @project = @attributes['project'] + + if Gitlab::SafeRequestStore.active? + @lru_cache = cache_from_request_store + @cache_key = [klass, attributes] + end end def find return if epic? && group.nil? - find_object || klass.create(project_attributes) + find_with_cache do + find_object || klass.create(project_attributes) + end end private - attr_reader :klass, :attributes, :group, :project + attr_reader :klass, :attributes, :group, :project, :lru_cache, :cache_key + + def find_with_cache + return yield unless lru_cache && cache_key + + lru_cache[cache_key] ||= yield + end + + def cache_from_request_store + Gitlab::SafeRequestStore[:lru_cache] ||= LruRedux::Cache.new(LRU_CACHE_SIZE) + end def find_object klass.where(where_clause).first diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb index d372a94db56..ee145a62b57 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -445,4 +445,64 @@ describe Projects::SnippetsController do end end end + + describe 'DELETE #destroy' do + let!(:snippet) { create(:project_snippet, :private, project: project, author: user) } + + let(:params) do + { + namespace_id: project.namespace.to_param, + project_id: project, + id: snippet.to_param + } + end + + context 'when current user has ability to destroy the snippet' do + before do + sign_in(user) + end + + it 'removes the snippet' do + delete :destroy, params: params + + expect { snippet.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + context 'when snippet is succesfuly destroyed' do + it 'redirects to the project snippets page' do + delete :destroy, params: params + + expect(response).to redirect_to(project_snippets_path(project)) + end + end + + context 'when snippet is not destroyed' do + before do + allow(snippet).to receive(:destroy).and_return(false) + controller.instance_variable_set(:@snippet, snippet) + end + + it 'renders the snippet page with errors' do + delete :destroy, params: params + + expect(flash[:alert]).to eq('Failed to remove snippet.') + expect(response).to redirect_to(project_snippet_path(project, snippet)) + end + end + end + + context 'when current_user does not have ability to destroy the snippet' do + let(:another_user) { create(:user) } + + before do + sign_in(another_user) + end + + it 'responds with status 404' do + delete :destroy, params: params + + expect(response).to have_gitlab_http_status(404) + end + end + end end diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 510db4374c0..c8f9e4256c9 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -664,4 +664,56 @@ describe SnippetsController do expect(json_response.keys).to match_array(%w(body references)) end end + + describe 'DELETE #destroy' do + let!(:snippet) { create :personal_snippet, author: user } + + context 'when current user has ability to destroy the snippet' do + before do + sign_in(user) + end + + it 'removes the snippet' do + delete :destroy, params: { id: snippet.to_param } + + expect { snippet.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + context 'when snippet is succesfuly destroyed' do + it 'redirects to the project snippets page' do + delete :destroy, params: { id: snippet.to_param } + + expect(response).to redirect_to(dashboard_snippets_path) + end + end + + context 'when snippet is not destroyed' do + before do + allow(snippet).to receive(:destroy).and_return(false) + controller.instance_variable_set(:@snippet, snippet) + end + + it 'renders the snippet page with errors' do + delete :destroy, params: { id: snippet.to_param } + + expect(flash[:alert]).to eq('Failed to remove snippet.') + expect(response).to redirect_to(snippet_path(snippet)) + end + end + end + + context 'when current_user does not have ability to destroy the snippet' do + let(:another_user) { create(:user) } + + before do + sign_in(another_user) + end + + it 'responds with status 404' do + delete :destroy, params: { id: snippet.to_param } + + expect(response).to have_gitlab_http_status(404) + end + end + end end diff --git a/spec/frontend/ide/components/panes/collapsible_sidebar_spec.js b/spec/frontend/ide/components/panes/collapsible_sidebar_spec.js new file mode 100644 index 00000000000..3bc89996978 --- /dev/null +++ b/spec/frontend/ide/components/panes/collapsible_sidebar_spec.js @@ -0,0 +1,167 @@ +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import { createStore } from '~/ide/stores'; +import paneModule from '~/ide/stores/modules/pane'; +import CollapsibleSidebar from '~/ide/components/panes/collapsible_sidebar.vue'; +import Vuex from 'vuex'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('ide/components/panes/collapsible_sidebar.vue', () => { + let wrapper; + let store; + + const width = 350; + const fakeComponentName = 'fake-component'; + + const createComponent = props => { + wrapper = shallowMount(CollapsibleSidebar, { + localVue, + store, + propsData: { + extensionTabs: [], + side: 'right', + width, + ...props, + }, + slots: { + 'header-icon': '<div class=".header-icon-slot">SLOT ICON</div>', + header: '<div class=".header-slot"/>', + footer: '<div class=".footer-slot"/>', + }, + }); + }; + + const findTabButton = () => wrapper.find(`[data-qa-selector="${fakeComponentName}_tab_button"]`); + + beforeEach(() => { + store = createStore(); + store.registerModule('leftPane', paneModule()); + }); + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + describe('with a tab', () => { + let fakeView; + let extensionTabs; + + beforeEach(() => { + const FakeComponent = localVue.component(fakeComponentName, { + render: () => {}, + }); + + fakeView = { + name: fakeComponentName, + keepAlive: true, + component: FakeComponent, + }; + + extensionTabs = [ + { + show: true, + title: fakeComponentName, + views: [fakeView], + icon: 'text-description', + buttonClasses: ['button-class-1', 'button-class-2'], + }, + ]; + }); + + describe.each` + side + ${'left'} + ${'right'} + `('when side=$side', ({ side }) => { + it('correctly renders side specific attributes', () => { + createComponent({ extensionTabs, side }); + const button = findTabButton(); + + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.classes()).toContain('multi-file-commit-panel'); + expect(wrapper.classes()).toContain(`ide-${side}-sidebar`); + expect(wrapper.find('.multi-file-commit-panel-inner')).not.toBe(null); + expect(wrapper.find(`.ide-${side}-sidebar-${fakeComponentName}`)).not.toBe(null); + expect(button.attributes('data-placement')).toEqual(side === 'left' ? 'right' : 'left'); + if (side === 'right') { + // this class is only needed on the right side; there is no 'is-left' + expect(button.classes()).toContain('is-right'); + } else { + expect(button.classes()).not.toContain('is-right'); + } + }); + }); + }); + + describe('when default side', () => { + let button; + + beforeEach(() => { + createComponent({ extensionTabs }); + + button = findTabButton(); + }); + + it('correctly renders tab-specific classes', () => { + store.state.rightPane.currentView = fakeComponentName; + + return wrapper.vm.$nextTick().then(() => { + expect(button.classes()).toContain('button-class-1'); + expect(button.classes()).toContain('button-class-2'); + }); + }); + + it('can show an open pane tab with an active view', () => { + store.state.rightPane.isOpen = true; + store.state.rightPane.currentView = fakeComponentName; + + return wrapper.vm.$nextTick().then(() => { + expect(button.classes()).toEqual(expect.arrayContaining(['ide-sidebar-link', 'active'])); + expect(button.attributes('data-original-title')).toEqual(fakeComponentName); + expect(wrapper.find('.js-tab-view').exists()).toBe(true); + }); + }); + + it('does not show a pane which is not open', () => { + store.state.rightPane.isOpen = false; + store.state.rightPane.currentView = fakeComponentName; + + return wrapper.vm.$nextTick().then(() => { + expect(button.classes()).not.toEqual( + expect.arrayContaining(['ide-sidebar-link', 'active']), + ); + expect(wrapper.find('.js-tab-view').exists()).toBe(false); + }); + }); + + describe('when button is clicked', () => { + it('opens view', () => { + button.trigger('click'); + expect(store.state.rightPane.isOpen).toBeTruthy(); + }); + + it('toggles open view if tab is currently active', () => { + button.trigger('click'); + expect(store.state.rightPane.isOpen).toBeTruthy(); + + button.trigger('click'); + expect(store.state.rightPane.isOpen).toBeFalsy(); + }); + }); + + it('shows header-icon', () => { + expect(wrapper.find('.header-icon-slot')).not.toBeNull(); + }); + + it('shows header', () => { + expect(wrapper.find('.header-slot')).not.toBeNull(); + }); + + it('shows footer', () => { + expect(wrapper.find('.footer-slot')).not.toBeNull(); + }); + }); + }); +}); diff --git a/spec/frontend/ide/components/panes/right_spec.js b/spec/frontend/ide/components/panes/right_spec.js index 6908790aaa8..7e408be96fc 100644 --- a/spec/frontend/ide/components/panes/right_spec.js +++ b/spec/frontend/ide/components/panes/right_spec.js @@ -1,89 +1,124 @@ import Vue from 'vue'; -import '~/behaviors/markdown/render_gfm'; -import { createComponentWithStore } from 'helpers/vue_mount_component_helper'; +import Vuex from 'vuex'; +import { createLocalVue, shallowMount } from '@vue/test-utils'; import { createStore } from '~/ide/stores'; import RightPane from '~/ide/components/panes/right.vue'; +import CollapsibleSidebar from '~/ide/components/panes/collapsible_sidebar.vue'; import { rightSidebarViews } from '~/ide/constants'; -describe('IDE right pane', () => { - let Component; - let vm; +const localVue = createLocalVue(); +localVue.use(Vuex); - beforeAll(() => { - Component = Vue.extend(RightPane); - }); +describe('ide/components/panes/right.vue', () => { + let wrapper; + let store; - beforeEach(() => { - const store = createStore(); + const createComponent = props => { + wrapper = shallowMount(RightPane, { + localVue, + store, + propsData: { + ...props, + }, + }); + }; - vm = createComponentWithStore(Component, store).$mount(); + beforeEach(() => { + store = createStore(); }); afterEach(() => { - vm.$destroy(); + wrapper.destroy(); + wrapper = null; }); - describe('active', () => { - it('renders merge request button as active', done => { - vm.$store.state.rightPane.isOpen = true; - vm.$store.state.rightPane.currentView = rightSidebarViews.mergeRequestInfo.name; - vm.$store.state.currentMergeRequestId = '123'; - vm.$store.state.currentProjectId = 'gitlab-ce'; - vm.$store.state.currentMergeRequestId = 1; - vm.$store.state.projects['gitlab-ce'] = { - mergeRequests: { - 1: { - iid: 1, - title: 'Testing', - title_html: '<span class="title-html">Testing</span>', - description: 'Description', - description_html: '<p class="description-html">Description HTML</p>', - }, + it('allows tabs to be added via extensionTabs prop', () => { + createComponent({ + extensionTabs: [ + { + show: true, + title: 'FakeTab', }, - }; - - vm.$nextTick() - .then(() => { - expect(vm.$el.querySelector('.ide-sidebar-link.active')).not.toBe(null); - expect( - vm.$el.querySelector('.ide-sidebar-link.active').getAttribute('data-original-title'), - ).toBe('Merge Request'); - }) - .then(done) - .catch(done.fail); + ], }); + + expect(wrapper.find(CollapsibleSidebar).props('extensionTabs')).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + show: true, + title: 'FakeTab', + }), + ]), + ); }); - describe('click', () => { - beforeEach(() => { - jest.spyOn(vm, 'open').mockReturnValue(); - }); + describe('pipelines tab', () => { + it('is always shown', () => { + createComponent(); - it('sets view to merge request', done => { - vm.$store.state.currentMergeRequestId = '123'; + expect(wrapper.find(CollapsibleSidebar).props('extensionTabs')).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + show: true, + title: 'Pipelines', + views: expect.arrayContaining([ + expect.objectContaining({ + name: rightSidebarViews.pipelines.name, + }), + expect.objectContaining({ + name: rightSidebarViews.jobsDetail.name, + }), + ]), + }), + ]), + ); + }); + }); - vm.$nextTick(() => { - vm.$el.querySelector('.ide-sidebar-link').click(); + describe('merge request tab', () => { + it('is shown if there is a currentMergeRequestId', () => { + store.state.currentMergeRequestId = 1; - expect(vm.open).toHaveBeenCalledWith(rightSidebarViews.mergeRequestInfo); + createComponent(); - done(); - }); + expect(wrapper.find(CollapsibleSidebar).props('extensionTabs')).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + show: true, + title: 'Merge Request', + views: expect.arrayContaining([ + expect.objectContaining({ + name: rightSidebarViews.mergeRequestInfo.name, + }), + ]), + }), + ]), + ); }); }); - describe('live preview', () => { - it('renders live preview button', done => { - Vue.set(vm.$store.state.entries, 'package.json', { + describe('clientside live preview tab', () => { + it('is shown if there is a packageJson and clientsidePreviewEnabled', () => { + Vue.set(store.state.entries, 'package.json', { name: 'package.json', }); - vm.$store.state.clientsidePreviewEnabled = true; + store.state.clientsidePreviewEnabled = true; - vm.$nextTick(() => { - expect(vm.$el.querySelector('button[aria-label="Live preview"]')).not.toBeNull(); + createComponent(); - done(); - }); + expect(wrapper.find(CollapsibleSidebar).props('extensionTabs')).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + show: true, + title: 'Live preview', + views: expect.arrayContaining([ + expect.objectContaining({ + name: rightSidebarViews.clientSidePreview.name, + }), + ]), + }), + ]), + ); }); }); }); diff --git a/spec/frontend/ide/stores/modules/pane/actions_spec.js b/spec/frontend/ide/stores/modules/pane/actions_spec.js index 8c0aeaff5b3..8c56714e0ed 100644 --- a/spec/frontend/ide/stores/modules/pane/actions_spec.js +++ b/spec/frontend/ide/stores/modules/pane/actions_spec.js @@ -8,14 +8,7 @@ describe('IDE pane module actions', () => { describe('toggleOpen', () => { it('dispatches open if closed', done => { - testAction( - actions.toggleOpen, - TEST_VIEW, - { isOpen: false }, - [], - [{ type: 'open', payload: TEST_VIEW }], - done, - ); + testAction(actions.toggleOpen, TEST_VIEW, { isOpen: false }, [], [{ type: 'open' }], done); }); it('dispatches close if opened', done => { @@ -24,37 +17,48 @@ describe('IDE pane module actions', () => { }); describe('open', () => { - it('commits SET_OPEN', done => { - testAction(actions.open, null, {}, [{ type: types.SET_OPEN, payload: true }], [], done); - }); + describe('with a view specified', () => { + it('commits SET_OPEN and SET_CURRENT_VIEW', done => { + testAction( + actions.open, + TEST_VIEW, + {}, + [ + { type: types.SET_OPEN, payload: true }, + { type: types.SET_CURRENT_VIEW, payload: TEST_VIEW.name }, + ], + [], + done, + ); + }); - it('commits SET_CURRENT_VIEW if view is given', done => { - testAction( - actions.open, - TEST_VIEW, - {}, - [ - { type: types.SET_OPEN, payload: true }, - { type: types.SET_CURRENT_VIEW, payload: TEST_VIEW.name }, - ], - [], - done, - ); + it('commits KEEP_ALIVE_VIEW if keepAlive is true', done => { + testAction( + actions.open, + TEST_VIEW_KEEP_ALIVE, + {}, + [ + { type: types.SET_OPEN, payload: true }, + { type: types.SET_CURRENT_VIEW, payload: TEST_VIEW_KEEP_ALIVE.name }, + { type: types.KEEP_ALIVE_VIEW, payload: TEST_VIEW_KEEP_ALIVE.name }, + ], + [], + done, + ); + }); }); - it('commits KEEP_ALIVE_VIEW if keepAlive is true', done => { - testAction( - actions.open, - TEST_VIEW_KEEP_ALIVE, - {}, - [ - { type: types.SET_OPEN, payload: true }, - { type: types.SET_CURRENT_VIEW, payload: TEST_VIEW_KEEP_ALIVE.name }, - { type: types.KEEP_ALIVE_VIEW, payload: TEST_VIEW_KEEP_ALIVE.name }, - ], - [], - done, - ); + describe('without a view specified', () => { + it('commits SET_OPEN', done => { + testAction( + actions.open, + undefined, + {}, + [{ type: types.SET_OPEN, payload: true }], + [], + done, + ); + }); }); }); diff --git a/spec/frontend/pipelines/graph/action_component_spec.js b/spec/frontend/pipelines/graph/action_component_spec.js index cbb1de4d87a..43da6388efa 100644 --- a/spec/frontend/pipelines/graph/action_component_spec.js +++ b/spec/frontend/pipelines/graph/action_component_spec.js @@ -19,7 +19,6 @@ describe('pipeline graph action component', () => { link: 'foo', actionIcon: 'cancel', }, - attachToDocument: true, }); }); diff --git a/spec/frontend/pipelines/graph/job_item_spec.js b/spec/frontend/pipelines/graph/job_item_spec.js index abeb538e390..0c64d5c9fa8 100644 --- a/spec/frontend/pipelines/graph/job_item_spec.js +++ b/spec/frontend/pipelines/graph/job_item_spec.js @@ -7,7 +7,6 @@ describe('pipeline graph job item', () => { const createWrapper = propsData => { wrapper = mount(JobItem, { - attachToDocument: true, propsData, }); }; diff --git a/spec/frontend/pipelines/graph/linked_pipeline_spec.js b/spec/frontend/pipelines/graph/linked_pipeline_spec.js index de1f9142001..7f49b21100d 100644 --- a/spec/frontend/pipelines/graph/linked_pipeline_spec.js +++ b/spec/frontend/pipelines/graph/linked_pipeline_spec.js @@ -10,7 +10,6 @@ describe('Linked pipeline', () => { const createWrapper = propsData => { wrapper = mount(LinkedPipelineComponent, { - attachToDocument: true, propsData, }); }; diff --git a/spec/frontend/pipelines/pipeline_triggerer_spec.js b/spec/frontend/pipelines/pipeline_triggerer_spec.js index b633d711699..a8eec274487 100644 --- a/spec/frontend/pipelines/pipeline_triggerer_spec.js +++ b/spec/frontend/pipelines/pipeline_triggerer_spec.js @@ -24,7 +24,6 @@ describe('Pipelines Triggerer', () => { const createComponent = () => { wrapper = shallowMount(pipelineTriggerer, { propsData: mockData, - attachToDocument: true, }); }; diff --git a/spec/frontend/pipelines/pipeline_url_spec.js b/spec/frontend/pipelines/pipeline_url_spec.js index 6587cc8b318..70b94f2c8e1 100644 --- a/spec/frontend/pipelines/pipeline_url_spec.js +++ b/spec/frontend/pipelines/pipeline_url_spec.js @@ -10,7 +10,6 @@ describe('Pipeline Url Component', () => { const createComponent = props => { wrapper = shallowMount(PipelineUrlComponent, { - attachToDocument: true, propsData: props, }); }; diff --git a/spec/lib/gitlab/import_export/group_project_object_builder_spec.rb b/spec/lib/gitlab/import_export/group_project_object_builder_spec.rb index 0d0a2df4423..355757654da 100644 --- a/spec/lib/gitlab/import_export/group_project_object_builder_spec.rb +++ b/spec/lib/gitlab/import_export/group_project_object_builder_spec.rb @@ -12,6 +12,59 @@ describe Gitlab::ImportExport::GroupProjectObjectBuilder do group: create(:group)) end + let(:lru_cache) { subject.send(:lru_cache) } + let(:cache_key) { subject.send(:cache_key) } + + context 'request store is not active' do + subject do + described_class.new(Label, + 'title' => 'group label', + 'project' => project, + 'group' => project.group) + end + + it 'ignore cache initialize' do + expect(lru_cache).to be_nil + expect(cache_key).to be_nil + end + end + + context 'request store is active', :request_store do + subject do + described_class.new(Label, + 'title' => 'group label', + 'project' => project, + 'group' => project.group) + end + + it 'initialize cache in memory' do + expect(lru_cache).not_to be_nil + expect(cache_key).not_to be_nil + end + + it 'cache object when first time find the object' do + group_label = create(:group_label, name: 'group label', group: project.group) + + expect(subject).to receive(:find_object).and_call_original + expect { subject.find } + .to change { lru_cache[cache_key] } + .from(nil).to(group_label) + + expect(subject.find).to eq(group_label) + end + + it 'read from cache when object has been cached' do + group_label = create(:group_label, name: 'group label', group: project.group) + + subject.find + + expect(subject).not_to receive(:find_object) + expect { subject.find }.not_to change { lru_cache[cache_key] } + + expect(subject.find).to eq(group_label) + end + end + context 'labels' do it 'finds the existing group label' do group_label = create(:group_label, name: 'group label', group: project.group) diff --git a/spec/requests/api/error_tracking_spec.rb b/spec/requests/api/error_tracking_spec.rb index af337f34a68..48ddc7f5a75 100644 --- a/spec/requests/api/error_tracking_spec.rb +++ b/spec/requests/api/error_tracking_spec.rb @@ -22,6 +22,7 @@ describe API::ErrorTracking do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to eq( + 'active' => setting.enabled, 'project_name' => setting.project_name, 'sentry_external_url' => setting.sentry_external_url, 'api_url' => setting.api_url diff --git a/spec/services/create_snippet_service_spec.rb b/spec/services/create_snippet_service_spec.rb deleted file mode 100644 index 1751029a78c..00000000000 --- a/spec/services/create_snippet_service_spec.rb +++ /dev/null @@ -1,117 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe CreateSnippetService do - let(:user) { create(:user) } - let(:admin) { create(:user, :admin) } - let(:opts) { base_opts.merge(extra_opts) } - let(:base_opts) do - { - title: 'Test snippet', - file_name: 'snippet.rb', - content: 'puts "hello world"', - visibility_level: Gitlab::VisibilityLevel::PRIVATE - } - end - let(:extra_opts) { {} } - - context 'When public visibility is restricted' do - let(:extra_opts) { { visibility_level: Gitlab::VisibilityLevel::PUBLIC } } - - before do - stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) - end - - it 'non-admins are not able to create a public snippet' do - snippet = create_snippet(nil, user, opts) - expect(snippet.errors.messages).to have_key(:visibility_level) - expect(snippet.errors.messages[:visibility_level].first).to( - match('has been restricted') - ) - end - - it 'admins are able to create a public snippet' do - snippet = create_snippet(nil, admin, opts) - expect(snippet.errors.any?).to be_falsey - expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC) - end - - describe "when visibility level is passed as a string" do - let(:extra_opts) { { visibility: 'internal' } } - - before do - base_opts.delete(:visibility_level) - end - - it "assigns the correct visibility level" do - snippet = create_snippet(nil, user, opts) - expect(snippet.errors.any?).to be_falsey - expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) - end - end - end - - context 'checking spam' do - shared_examples 'marked as spam' do - let(:snippet) { create_snippet(nil, admin, opts) } - - it 'marks a snippet as a spam ' do - expect(snippet).to be_spam - end - - it 'invalidates the snippet' do - expect(snippet).to be_invalid - end - - it 'creates a new spam_log' do - expect { snippet } - .to log_spam(title: snippet.title, noteable_type: 'PersonalSnippet') - end - - it 'assigns a spam_log to an issue' do - expect(snippet.spam_log).to eq(SpamLog.last) - end - end - - let(:extra_opts) do - { visibility_level: Gitlab::VisibilityLevel::PUBLIC, request: double(:request, env: {}) } - end - - before do - expect_next_instance_of(AkismetService) do |akismet_service| - expect(akismet_service).to receive_messages(spam?: true) - end - end - - [true, false, nil].each do |allow_possible_spam| - context "when recaptcha_disabled flag is #{allow_possible_spam.inspect}" do - before do - stub_feature_flags(allow_possible_spam: allow_possible_spam) unless allow_possible_spam.nil? - end - - it_behaves_like 'marked as spam' - end - end - end - - describe 'usage counter' do - let(:counter) { Gitlab::UsageDataCounters::SnippetCounter } - - it 'increments count' do - expect do - create_snippet(nil, admin, opts) - end.to change { counter.read(:create) }.by 1 - end - - it 'does not increment count if create fails' do - expect do - create_snippet(nil, admin, {}) - end.not_to change { counter.read(:create) } - end - end - - def create_snippet(project, user, opts) - CreateSnippetService.new(project, user, opts).execute - end -end diff --git a/spec/services/snippets/create_service_spec.rb b/spec/services/snippets/create_service_spec.rb new file mode 100644 index 00000000000..6f7ce7959ff --- /dev/null +++ b/spec/services/snippets/create_service_spec.rb @@ -0,0 +1,170 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Snippets::CreateService do + describe '#execute' do + let_it_be(:user) { create(:user) } + let_it_be(:admin) { create(:user, :admin) } + let(:opts) { base_opts.merge(extra_opts) } + let(:base_opts) do + { + title: 'Test snippet', + file_name: 'snippet.rb', + content: 'puts "hello world"', + visibility_level: Gitlab::VisibilityLevel::PRIVATE + } + end + let(:extra_opts) { {} } + let(:creator) { admin } + + subject { Snippets::CreateService.new(project, creator, opts).execute } + + let(:snippet) { subject.payload[:snippet] } + + shared_examples 'a service that creates a snippet' do + it 'creates a snippet with the provided attributes' do + expect(snippet.title).to eq(opts[:title]) + expect(snippet.file_name).to eq(opts[:file_name]) + expect(snippet.content).to eq(opts[:content]) + expect(snippet.visibility_level).to eq(opts[:visibility_level]) + end + end + + shared_examples 'public visibility level restrictions apply' do + let(:extra_opts) { { visibility_level: Gitlab::VisibilityLevel::PUBLIC } } + + before do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + end + + context 'when user is not an admin' do + let(:creator) { user } + + it 'responds with an error' do + expect(subject).to be_error + end + + it 'does not create a public snippet' do + expect(subject.message).to match('has been restricted') + end + end + + context 'when user is an admin' do + it 'responds with success' do + expect(subject).to be_success + end + + it 'creates a public snippet' do + expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC) + end + end + + describe 'when visibility level is passed as a string' do + let(:extra_opts) { { visibility: 'internal' } } + + before do + base_opts.delete(:visibility_level) + end + + it 'assigns the correct visibility level' do + expect(subject).to be_success + expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) + end + end + end + + shared_examples 'spam check is performed' do + shared_examples 'marked as spam' do + it 'marks a snippet as spam ' do + expect(snippet).to be_spam + end + + it 'invalidates the snippet' do + expect(snippet).to be_invalid + end + + it 'creates a new spam_log' do + expect { snippet } + .to log_spam(title: snippet.title, noteable_type: snippet.class.name) + end + + it 'assigns a spam_log to an issue' do + expect(snippet.spam_log).to eq(SpamLog.last) + end + end + + let(:extra_opts) do + { visibility_level: Gitlab::VisibilityLevel::PUBLIC, request: double(:request, env: {}) } + end + + before do + expect_next_instance_of(AkismetService) do |akismet_service| + expect(akismet_service).to receive_messages(spam?: true) + end + end + + [true, false, nil].each do |allow_possible_spam| + context "when recaptcha_disabled flag is #{allow_possible_spam.inspect}" do + before do + stub_feature_flags(allow_possible_spam: allow_possible_spam) unless allow_possible_spam.nil? + end + + it_behaves_like 'marked as spam' + end + end + end + + shared_examples 'snippet create data is tracked' do + let(:counter) { Gitlab::UsageDataCounters::SnippetCounter } + + it 'increments count when create succeeds' do + expect { subject }.to change { counter.read(:create) }.by 1 + end + + context 'when create fails' do + let(:opts) { {} } + + it 'does not increment count' do + expect { subject }.not_to change { counter.read(:create) } + end + end + end + + shared_examples 'an error service response when save fails' do + let(:extra_opts) { { content: nil } } + + it 'responds with an error' do + expect(subject).to be_error + end + + it 'does not create the snippet' do + expect { subject }.not_to change { Snippet.count } + end + end + + context 'when Project Snippet' do + let_it_be(:project) { create(:project) } + + before do + project.add_developer(user) + end + + it_behaves_like 'a service that creates a snippet' + it_behaves_like 'public visibility level restrictions apply' + it_behaves_like 'spam check is performed' + it_behaves_like 'snippet create data is tracked' + it_behaves_like 'an error service response when save fails' + end + + context 'when PersonalSnippet' do + let(:project) { nil } + + it_behaves_like 'a service that creates a snippet' + it_behaves_like 'public visibility level restrictions apply' + it_behaves_like 'spam check is performed' + it_behaves_like 'snippet create data is tracked' + it_behaves_like 'an error service response when save fails' + end + end +end diff --git a/spec/services/snippets/destroy_service_spec.rb b/spec/services/snippets/destroy_service_spec.rb new file mode 100644 index 00000000000..bb035d275ab --- /dev/null +++ b/spec/services/snippets/destroy_service_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Snippets::DestroyService do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:other_user) { create(:user) } + + describe '#execute' do + subject { Snippets::DestroyService.new(user, snippet).execute } + + context 'when snippet is nil' do + let(:snippet) { nil } + + it 'returns a ServiceResponse error' do + expect(subject).to be_error + end + end + + shared_examples 'a successful destroy' do + it 'deletes the snippet' do + expect { subject }.to change { Snippet.count }.by(-1) + end + + it 'returns ServiceResponse success' do + expect(subject).to be_success + end + end + + shared_examples 'an unsuccessful destroy' do + it 'does not delete the snippet' do + expect { subject }.to change { Snippet.count }.by(0) + end + + it 'returns ServiceResponse error' do + expect(subject).to be_error + end + end + + context 'when ProjectSnippet' do + let!(:snippet) { create(:project_snippet, project: project, author: author) } + + context 'when user is able to admin_project_snippet' do + let(:author) { user } + + before do + project.add_developer(user) + end + + it_behaves_like 'a successful destroy' + end + + context 'when user is not able to admin_project_snippet' do + let(:author) { other_user } + + it_behaves_like 'an unsuccessful destroy' + end + end + + context 'when PersonalSnippet' do + let!(:snippet) { create(:personal_snippet, author: author) } + + context 'when user is able to admin_personal_snippet' do + let(:author) { user } + + it_behaves_like 'a successful destroy' + end + + context 'when user is not able to admin_personal_snippet' do + let(:author) { other_user } + + it_behaves_like 'an unsuccessful destroy' + end + end + end +end diff --git a/spec/services/snippets/update_service_spec.rb b/spec/services/snippets/update_service_spec.rb new file mode 100644 index 00000000000..b8215f9779d --- /dev/null +++ b/spec/services/snippets/update_service_spec.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Snippets::UpdateService do + describe '#execute' do + let_it_be(:user) { create(:user) } + let_it_be(:admin) { create :user, admin: true } + let(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE } + let(:options) do + { + title: 'Test snippet', + file_name: 'snippet.rb', + content: 'puts "hello world"', + visibility_level: visibility_level + } + end + let(:updater) { user } + + subject do + Snippets::UpdateService.new( + project, + updater, + options + ).execute(snippet) + end + + shared_examples 'a service that updates a snippet' do + it 'updates a snippet with the provided attributes' do + expect { subject }.to change { snippet.title }.from(snippet.title).to(options[:title]) + .and change { snippet.file_name }.from(snippet.file_name).to(options[:file_name]) + .and change { snippet.content }.from(snippet.content).to(options[:content]) + end + end + + shared_examples 'public visibility level restrictions apply' do + let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } + + before do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + end + + context 'when user is not an admin' do + it 'responds with an error' do + expect(subject).to be_error + end + + it 'does not update snippet to public visibility' do + original_visibility = snippet.visibility_level + + expect(subject.message).to match('has been restricted') + expect(snippet.visibility_level).to eq(original_visibility) + end + end + + context 'when user is an admin' do + let(:updater) { admin } + + it 'responds with success' do + expect(subject).to be_success + end + + it 'updates the snippet to public visibility' do + old_visibility = snippet.visibility_level + + expect(subject.payload[:snippet]).not_to be_nil + expect(snippet.visibility_level).not_to eq(old_visibility) + expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC) + end + end + + context 'when visibility level is passed as a string' do + before do + options[:visibility] = 'internal' + options.delete(:visibility_level) + end + + it 'assigns the correct visibility level' do + expect(subject).to be_success + expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) + end + end + end + + shared_examples 'snippet update data is tracked' do + let(:counter) { Gitlab::UsageDataCounters::SnippetCounter } + + it 'increments count when create succeeds' do + expect { subject }.to change { counter.read(:update) }.by 1 + end + + context 'when update fails' do + let(:options) { { title: '' } } + + it 'does not increment count' do + expect { subject }.not_to change { counter.read(:update) } + end + end + end + + context 'when Project Snippet' do + let_it_be(:project) { create(:project) } + let!(:snippet) { create(:project_snippet, author: user, project: project) } + + before do + project.add_developer(user) + end + + it_behaves_like 'a service that updates a snippet' + it_behaves_like 'public visibility level restrictions apply' + it_behaves_like 'snippet update data is tracked' + end + + context 'when PersonalSnippet' do + let(:project) { nil } + let!(:snippet) { create(:personal_snippet, author: user) } + + it_behaves_like 'a service that updates a snippet' + it_behaves_like 'public visibility level restrictions apply' + it_behaves_like 'snippet update data is tracked' + end + end +end diff --git a/spec/services/update_snippet_service_spec.rb b/spec/services/update_snippet_service_spec.rb deleted file mode 100644 index 19b35dca6a7..00000000000 --- a/spec/services/update_snippet_service_spec.rb +++ /dev/null @@ -1,80 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe UpdateSnippetService do - before do - @user = create :user - @admin = create :user, admin: true - @opts = { - title: 'Test snippet', - file_name: 'snippet.rb', - content: 'puts "hello world"', - visibility_level: Gitlab::VisibilityLevel::PRIVATE - } - end - - context 'When public visibility is restricted' do - before do - stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) - - @snippet = create_snippet(@project, @user, @opts) - @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - end - - it 'non-admins should not be able to update to public visibility' do - old_visibility = @snippet.visibility_level - update_snippet(@project, @user, @snippet, @opts) - expect(@snippet.errors.messages).to have_key(:visibility_level) - expect(@snippet.errors.messages[:visibility_level].first).to( - match('has been restricted') - ) - expect(@snippet.visibility_level).to eq(old_visibility) - end - - it 'admins should be able to update to public visibility' do - old_visibility = @snippet.visibility_level - update_snippet(@project, @admin, @snippet, @opts) - expect(@snippet.visibility_level).not_to eq(old_visibility) - expect(@snippet.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC) - end - - describe "when visibility level is passed as a string" do - before do - @opts[:visibility] = 'internal' - @opts.delete(:visibility_level) - end - - it "assigns the correct visibility level" do - update_snippet(@project, @user, @snippet, @opts) - expect(@snippet.errors.any?).to be_falsey - expect(@snippet.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) - end - end - end - - describe 'usage counter' do - let(:counter) { Gitlab::UsageDataCounters::SnippetCounter } - let(:snippet) { create_snippet(nil, @user, @opts) } - - it 'increments count' do - expect do - update_snippet(nil, @admin, snippet, @opts) - end.to change { counter.read(:update) }.by 1 - end - - it 'does not increment count if create fails' do - expect do - update_snippet(nil, @admin, snippet, { title: '' }) - end.not_to change { counter.read(:update) } - end - end - - def create_snippet(project, user, opts) - CreateSnippetService.new(project, user, opts).execute - end - - def update_snippet(project, user, snippet, opts) - UpdateSnippetService.new(project, user, snippet, opts).execute - end -end |