diff options
47 files changed, 1480 insertions, 709 deletions
diff --git a/app/assets/javascripts/flash.js b/app/assets/javascripts/flash.js index 74c00d21535..262e7c4e412 100644 --- a/app/assets/javascripts/flash.js +++ b/app/assets/javascripts/flash.js @@ -1,3 +1,4 @@ +import * as Sentry from '@sentry/browser'; import { escape } from 'lodash'; import { spriteIcon } from './lib/utils/common_utils'; @@ -109,8 +110,65 @@ const createFlash = function createFlash( return flashContainer; }; +/* + * Flash banner supports different types of Flash configurations + * along with ability to provide actionConfig which can be used to show + * additional action or link on banner next to message + * + * @param {Object} options Options to control the flash message + * @param {String} options.message Flash message text + * @param {String} options.type Type of Flash, it can be `notice`, `success`, `warning` or `alert` (default) + * @param {Object} options.parent Reference to parent element under which Flash needs to appear + * @param {Object} options.actonConfig Map of config to show action on banner + * @param {String} href URL to which action config should point to (default: '#') + * @param {String} title Title of action + * @param {Function} clickHandler Method to call when action is clicked on + * @param {Boolean} options.fadeTransition Boolean to determine whether to fade the alert out + * @param {Boolean} options.captureError Boolean to determine whether to send error to sentry + * @param {Object} options.error Error to be captured in sentry + */ +const newCreateFlash = function newCreateFlash({ + message, + type = FLASH_TYPES.ALERT, + parent = document, + actionConfig = null, + fadeTransition = true, + addBodyClass = false, + captureError = false, + error = null, +}) { + const flashContainer = parent.querySelector('.flash-container'); + + if (!flashContainer) return null; + + flashContainer.innerHTML = createFlashEl(message, type); + + const flashEl = flashContainer.querySelector(`.flash-${type}`); + + if (actionConfig) { + flashEl.insertAdjacentHTML('beforeend', createAction(actionConfig)); + + if (actionConfig.clickHandler) { + flashEl + .querySelector('.flash-action') + .addEventListener('click', e => actionConfig.clickHandler(e)); + } + } + + removeFlashClickListener(flashEl, fadeTransition); + + flashContainer.classList.add('gl-display-block'); + + if (addBodyClass) document.body.classList.add('flash-shown'); + + if (captureError && error) Sentry.captureException(error); + + return flashContainer; +}; + export { createFlash as default, + newCreateFlash, createFlashEl, createAction, hideFlash, diff --git a/app/assets/javascripts/monitoring/components/dashboard_actions_menu.vue b/app/assets/javascripts/monitoring/components/dashboard_actions_menu.vue new file mode 100644 index 00000000000..54586c67fef --- /dev/null +++ b/app/assets/javascripts/monitoring/components/dashboard_actions_menu.vue @@ -0,0 +1,249 @@ +<script> +import { mapState, mapGetters, mapActions } from 'vuex'; +import { + GlDeprecatedButton, + GlNewDropdown, + GlNewDropdownDivider, + GlNewDropdownItem, + GlModal, + GlIcon, + GlModalDirective, + GlTooltipDirective, +} from '@gitlab/ui'; +import CustomMetricsFormFields from '~/custom_metrics/components/custom_metrics_form_fields.vue'; +import DuplicateDashboardModal from './duplicate_dashboard_modal.vue'; +import CreateDashboardModal from './create_dashboard_modal.vue'; +import { s__ } from '~/locale'; +import invalidUrl from '~/lib/utils/invalid_url'; +import { redirectTo } from '~/lib/utils/url_utility'; +import TrackEventDirective from '~/vue_shared/directives/track_event'; +import { getAddMetricTrackingOptions } from '../utils'; + +export default { + components: { + GlDeprecatedButton, + GlNewDropdown, + GlNewDropdownDivider, + GlNewDropdownItem, + GlModal, + GlIcon, + DuplicateDashboardModal, + CreateDashboardModal, + CustomMetricsFormFields, + }, + directives: { + GlModal: GlModalDirective, + GlTooltip: GlTooltipDirective, + TrackEvent: TrackEventDirective, + }, + props: { + addingMetricsAvailable: { + type: Boolean, + required: false, + default: false, + }, + customMetricsPath: { + type: String, + required: false, + default: invalidUrl, + }, + validateQueryPath: { + type: String, + required: false, + default: invalidUrl, + }, + defaultBranch: { + type: String, + required: true, + }, + }, + data() { + return { customMetricsFormIsValid: null }; + }, + computed: { + ...mapState('monitoringDashboard', [ + 'projectPath', + 'isUpdatingStarredValue', + 'addDashboardDocumentationPath', + ]), + ...mapGetters('monitoringDashboard', ['selectedDashboard']), + isOutOfTheBoxDashboard() { + return this.selectedDashboard?.out_of_the_box_dashboard; + }, + isMenuItemEnabled() { + return { + createDashboard: Boolean(this.projectPath), + editDashboard: this.selectedDashboard?.can_edit, + }; + }, + isMenuItemShown() { + return { + duplicateDashboard: this.isOutOfTheBoxDashboard, + }; + }, + }, + methods: { + ...mapActions('monitoringDashboard', ['toggleStarredValue']), + setFormValidity(isValid) { + this.customMetricsFormIsValid = isValid; + }, + hideAddMetricModal() { + this.$refs.addMetricModal.hide(); + }, + getAddMetricTrackingOptions, + submitCustomMetricsForm() { + this.$refs.customMetricsForm.submit(); + }, + selectDashboard(dashboard) { + // Once the sidebar See metrics link is updated to the new URL, + // this sort of hardcoding will not be necessary. + // https://gitlab.com/gitlab-org/gitlab/-/issues/229277 + const baseURL = `${this.projectPath}/-/metrics`; + const dashboardPath = encodeURIComponent( + dashboard.out_of_the_box_dashboard ? dashboard.path : dashboard.display_name, + ); + redirectTo(`${baseURL}/${dashboardPath}`); + }, + }, + + modalIds: { + addMetric: 'addMetric', + createDashboard: 'createDashboard', + duplicateDashboard: 'duplicateDashboard', + }, + i18n: { + actionsMenu: s__('Metrics|More actions'), + duplicateDashboard: s__('Metrics|Duplicate current dashboard'), + starDashboard: s__('Metrics|Star dashboard'), + unstarDashboard: s__('Metrics|Unstar dashboard'), + addMetric: s__('Metrics|Add metric'), + editDashboardInfo: s__('Metrics|Duplicate this dashboard to edit dashboard YAML'), + editDashboard: s__('Metrics|Edit dashboard YAML'), + createDashboard: s__('Metrics|Create new dashboard'), + }, +}; +</script> + +<template> + <gl-new-dropdown + v-gl-tooltip + data-testid="actions-menu" + data-qa-selector="actions_menu_dropdown" + right + no-caret + toggle-class="gl-px-3!" + :title="$options.i18n.actionsMenu" + > + <template #button-content> + <gl-icon class="gl-mr-0!" name="ellipsis_v" /> + </template> + + <template v-if="addingMetricsAvailable"> + <gl-new-dropdown-item + v-gl-modal="$options.modalIds.addMetric" + data-qa-selector="add_metric_button" + data-testid="add-metric-item" + > + {{ $options.i18n.addMetric }} + </gl-new-dropdown-item> + <gl-modal + ref="addMetricModal" + :modal-id="$options.modalIds.addMetric" + :title="$options.i18n.addMetric" + data-testid="add-metric-modal" + > + <form ref="customMetricsForm" :action="customMetricsPath" method="post"> + <custom-metrics-form-fields + :validate-query-path="validateQueryPath" + form-operation="post" + @formValidation="setFormValidity" + /> + </form> + <div slot="modal-footer"> + <gl-deprecated-button @click="hideAddMetricModal"> + {{ __('Cancel') }} + </gl-deprecated-button> + <gl-deprecated-button + v-track-event="getAddMetricTrackingOptions()" + data-testid="add-metric-modal-submit-button" + :disabled="!customMetricsFormIsValid" + variant="success" + @click="submitCustomMetricsForm" + > + {{ __('Save changes') }} + </gl-deprecated-button> + </div> + </gl-modal> + </template> + + <gl-new-dropdown-item + v-if="isMenuItemEnabled.editDashboard" + :href="selectedDashboard ? selectedDashboard.project_blob_path : null" + data-qa-selector="edit_dashboard_button_enabled" + data-testid="edit-dashboard-item-enabled" + > + {{ $options.i18n.editDashboard }} + </gl-new-dropdown-item> + + <!-- + wrapper for tooltip as button can be `disabled` + https://bootstrap-vue.org/docs/components/tooltip#disabled-elements + --> + <div v-else v-gl-tooltip :title="$options.i18n.editDashboardInfo"> + <gl-new-dropdown-item + :alt="$options.i18n.editDashboardInfo" + :href="selectedDashboard ? selectedDashboard.project_blob_path : null" + data-testid="edit-dashboard-item-disabled" + disabled + class="gl-cursor-not-allowed" + > + <span class="gl-text-gray-400">{{ $options.i18n.editDashboard }}</span> + </gl-new-dropdown-item> + </div> + + <template v-if="isMenuItemShown.duplicateDashboard"> + <gl-new-dropdown-item + v-gl-modal="$options.modalIds.duplicateDashboard" + data-testid="duplicate-dashboard-item" + > + {{ $options.i18n.duplicateDashboard }} + </gl-new-dropdown-item> + + <duplicate-dashboard-modal + :default-branch="defaultBranch" + :modal-id="$options.modalIds.duplicateDashboard" + data-testid="duplicate-dashboard-modal" + @dashboardDuplicated="selectDashboard" + /> + </template> + + <gl-new-dropdown-item + v-if="selectedDashboard" + data-testid="star-dashboard-item" + :disabled="isUpdatingStarredValue" + @click="toggleStarredValue()" + > + {{ selectedDashboard.starred ? $options.i18n.unstarDashboard : $options.i18n.starDashboard }} + </gl-new-dropdown-item> + + <gl-new-dropdown-divider /> + + <gl-new-dropdown-item + v-gl-modal="$options.modalIds.createDashboard" + data-testid="create-dashboard-item" + :disabled="!isMenuItemEnabled.createDashboard" + :class="{ 'monitoring-actions-item-disabled': !isMenuItemEnabled.createDashboard }" + > + {{ $options.i18n.createDashboard }} + </gl-new-dropdown-item> + + <template v-if="isMenuItemEnabled.createDashboard"> + <create-dashboard-modal + data-testid="create-dashboard-modal" + :add-dashboard-documentation-path="addDashboardDocumentationPath" + :modal-id="$options.modalIds.createDashboard" + :project-path="projectPath" + /> + </template> + </gl-new-dropdown> +</template> diff --git a/app/assets/javascripts/monitoring/components/dashboard_header.vue b/app/assets/javascripts/monitoring/components/dashboard_header.vue index a7e23be98b3..1c921548ce7 100644 --- a/app/assets/javascripts/monitoring/components/dashboard_header.vue +++ b/app/assets/javascripts/monitoring/components/dashboard_header.vue @@ -7,17 +7,11 @@ import { GlDeprecatedDropdownItem, GlDeprecatedDropdownHeader, GlDeprecatedDropdownDivider, - GlNewDropdown, - GlNewDropdownDivider, - GlNewDropdownItem, - GlModal, GlLoadingIcon, GlSearchBoxByType, GlModalDirective, GlTooltipDirective, } from '@gitlab/ui'; -import { s__ } from '~/locale'; -import CustomMetricsFormFields from '~/custom_metrics/components/custom_metrics_form_fields.vue'; import { mergeUrlParams, redirectTo } from '~/lib/utils/url_utility'; import invalidUrl from '~/lib/utils/invalid_url'; import Icon from '~/vue_shared/components/icon.vue'; @@ -25,11 +19,9 @@ import DateTimePicker from '~/vue_shared/components/date_time_picker/date_time_p import DashboardsDropdown from './dashboards_dropdown.vue'; import RefreshButton from './refresh_button.vue'; -import CreateDashboardModal from './create_dashboard_modal.vue'; -import DuplicateDashboardModal from './duplicate_dashboard_modal.vue'; +import ActionsMenu from './dashboard_actions_menu.vue'; -import TrackEventDirective from '~/vue_shared/directives/track_event'; -import { getAddMetricTrackingOptions, timeRangeToUrl } from '../utils'; +import { timeRangeToUrl } from '../utils'; import { timeRanges } from '~/vue_shared/constants'; import { timezones } from '../format_date'; @@ -42,23 +34,17 @@ export default { GlDeprecatedDropdownItem, GlDeprecatedDropdownHeader, GlDeprecatedDropdownDivider, - GlNewDropdown, - GlNewDropdownDivider, - GlNewDropdownItem, GlSearchBoxByType, - GlModal, - CustomMetricsFormFields, DateTimePicker, DashboardsDropdown, RefreshButton, - DuplicateDashboardModal, - CreateDashboardModal, + + ActionsMenu, }, directives: { GlModal: GlModalDirective, GlTooltip: GlTooltipDirective, - TrackEvent: TrackEventDirective, }, props: { defaultBranch: { @@ -94,29 +80,19 @@ export default { required: true, }, }, - data() { - return { - formIsValid: null, - }; - }, computed: { ...mapState('monitoringDashboard', [ 'emptyState', 'environmentsLoading', 'currentEnvironmentName', - 'isUpdatingStarredValue', 'dashboardTimezone', 'projectPath', 'canAccessOperationsSettings', 'operationsSettingsPath', 'currentDashboard', - 'addDashboardDocumentationPath', 'externalDashboardUrl', ]), ...mapGetters('monitoringDashboard', ['selectedDashboard', 'filteredEnvironments']), - isOutOfTheBoxDashboard() { - return this.selectedDashboard?.out_of_the_box_dashboard; - }, shouldShowEmptyState() { return Boolean(this.emptyState); }, @@ -130,7 +106,7 @@ export default { // Custom metrics only avaialble on system dashboards because // they are stored in the database. This can be improved. See: // https://gitlab.com/gitlab-org/gitlab/-/issues/28241 - this.selectedDashboard?.system_dashboard + this.selectedDashboard?.out_of_the_box_dashboard ); }, showRearrangePanelsBtn() { @@ -139,15 +115,12 @@ export default { displayUtc() { return this.dashboardTimezone === timezones.UTC; }, - shouldShowActionsMenu() { - return Boolean(this.projectPath); - }, shouldShowSettingsButton() { return this.canAccessOperationsSettings && this.operationsSettingsPath; }, }, methods: { - ...mapActions('monitoringDashboard', ['filterEnvironments', 'toggleStarredValue']), + ...mapActions('monitoringDashboard', ['filterEnvironments']), selectDashboard(dashboard) { // Once the sidebar See metrics link is updated to the new URL, // this sort of hardcoding will not be necessary. @@ -171,16 +144,6 @@ export default { toggleRearrangingPanels() { this.$emit('setRearrangingPanels', !this.isRearrangingPanels); }, - setFormValidity(isValid) { - this.formIsValid = isValid; - }, - hideAddMetricModal() { - this.$refs.addMetricModal.hide(); - }, - getAddMetricTrackingOptions, - submitCustomMetricsForm() { - this.$refs.customMetricsForm.submit(); - }, getEnvironmentPath(environment) { // Once the sidebar See metrics link is updated to the new URL, // this sort of hardcoding will not be necessary. @@ -193,16 +156,6 @@ export default { return mergeUrlParams({ environment }, url); }, }, - modalIds: { - addMetric: 'addMetric', - createDashboard: 'createDashboard', - duplicateDashboard: 'duplicateDashboard', - }, - i18n: { - starDashboard: s__('Metrics|Star dashboard'), - unstarDashboard: s__('Metrics|Unstar dashboard'), - addMetric: s__('Metrics|Add metric'), - }, timeRanges, }; </script> @@ -280,29 +233,6 @@ export default { <div class="flex-grow-1"></div> <div class="d-sm-flex"> - <div v-if="selectedDashboard" class="mb-2 mr-2 d-flex"> - <!-- - wrapper for tooltip as button can be `disabled` - https://bootstrap-vue.org/docs/components/tooltip#disabled-elements - --> - <div - v-gl-tooltip - class="flex-grow-1" - :title=" - selectedDashboard.starred ? $options.i18n.unstarDashboard : $options.i18n.starDashboard - " - > - <gl-button - ref="toggleStarBtn" - class="w-100" - :disabled="isUpdatingStarredValue" - variant="default" - :icon="selectedDashboard.starred ? 'star' : 'star-o'" - @click="toggleStarredValue()" - /> - </div> - </div> - <div v-if="showRearrangePanelsBtn" class="mb-2 mr-2 d-flex"> <gl-button :pressed="isRearrangingPanels" @@ -313,58 +243,6 @@ export default { {{ __('Arrange charts') }} </gl-button> </div> - <div v-if="addingMetricsAvailable" class="mb-2 mr-2 d-flex d-sm-block"> - <gl-button - ref="addMetricBtn" - v-gl-modal="$options.modalIds.addMetric" - variant="default" - data-qa-selector="add_metric_button" - class="flex-grow-1" - > - {{ $options.i18n.addMetric }} - </gl-button> - <gl-modal - ref="addMetricModal" - :modal-id="$options.modalIds.addMetric" - :title="$options.i18n.addMetric" - > - <form ref="customMetricsForm" :action="customMetricsPath" method="post"> - <custom-metrics-form-fields - :validate-query-path="validateQueryPath" - form-operation="post" - @formValidation="setFormValidity" - /> - </form> - <div slot="modal-footer"> - <gl-button @click="hideAddMetricModal"> - {{ __('Cancel') }} - </gl-button> - <gl-button - ref="submitCustomMetricsFormBtn" - v-track-event="getAddMetricTrackingOptions()" - :disabled="!formIsValid" - variant="success" - category="primary" - @click="submitCustomMetricsForm" - > - {{ __('Save changes') }} - </gl-button> - </div> - </gl-modal> - </div> - - <div - v-if="selectedDashboard && selectedDashboard.can_edit" - class="mb-2 mr-2 d-flex d-sm-block" - > - <gl-button - class="flex-grow-1 js-edit-link" - :href="selectedDashboard.project_blob_path" - data-qa-selector="edit_dashboard_button" - > - {{ __('Edit dashboard') }} - </gl-button> - </div> <div v-if="externalDashboardUrl && externalDashboardUrl.length" @@ -382,65 +260,28 @@ export default { </gl-button> </div> - <!-- This separator should be displayed only if at least one of the action menu or settings button are displayed --> - <span - v-if="shouldShowActionsMenu || shouldShowSettingsButton" - aria-hidden="true" - class="gl-pl-3 border-left gl-mb-3 d-none d-sm-block" - ></span> + <div class="gl-mb-3 gl-mr-3 d-flex d-sm-block"> + <actions-menu + :adding-metrics-available="addingMetricsAvailable" + :custom-metrics-path="customMetricsPath" + :validate-query-path="validateQueryPath" + :default-branch="defaultBranch" + /> + </div> - <div v-if="shouldShowActionsMenu" class="gl-mb-3 gl-mr-3 d-flex d-sm-block"> - <gl-new-dropdown - v-gl-tooltip - right - class="gl-flex-grow-1" - data-testid="actions-menu" - data-qa-selector="actions_menu_dropdown" - :title="s__('Metrics|Create dashboard')" - :icon="'plus-square'" - > - <gl-new-dropdown-item - v-gl-modal="$options.modalIds.createDashboard" - data-testid="action-create-dashboard" - >{{ s__('Metrics|Create new dashboard') }}</gl-new-dropdown-item - > + <template v-if="shouldShowSettingsButton"> + <span aria-hidden="true" class="gl-pl-3 border-left gl-mb-3 d-none d-sm-block"></span> - <create-dashboard-modal - data-testid="create-dashboard-modal" - :add-dashboard-documentation-path="addDashboardDocumentationPath" - :modal-id="$options.modalIds.createDashboard" - :project-path="projectPath" + <div class="mb-2 mr-2 d-flex d-sm-block"> + <gl-button + v-gl-tooltip + data-testid="metrics-settings-button" + icon="settings" + :href="operationsSettingsPath" + :title="s__('Metrics|Metrics Settings')" /> - - <template v-if="isOutOfTheBoxDashboard"> - <gl-new-dropdown-divider /> - - <gl-new-dropdown-item - ref="duplicateDashboardItem" - v-gl-modal="$options.modalIds.duplicateDashboard" - data-testid="action-duplicate-dashboard" - > - {{ s__('Metrics|Duplicate current dashboard') }} - </gl-new-dropdown-item> - - <duplicate-dashboard-modal - :default-branch="defaultBranch" - :modal-id="$options.modalIds.duplicateDashboard" - @dashboardDuplicated="selectDashboard" - /> - </template> - </gl-new-dropdown> - </div> - - <div v-if="shouldShowSettingsButton" class="mb-2 mr-2 d-flex d-sm-block"> - <gl-button - v-gl-tooltip - data-testid="metrics-settings-button" - icon="settings" - :href="operationsSettingsPath" - :title="s__('Metrics|Metrics Settings')" - /> - </div> + </div> + </template> </div> </div> </template> diff --git a/app/assets/javascripts/packages/details/components/composer_installation.vue b/app/assets/javascripts/packages/details/components/composer_installation.vue new file mode 100644 index 00000000000..c295995935f --- /dev/null +++ b/app/assets/javascripts/packages/details/components/composer_installation.vue @@ -0,0 +1,59 @@ +<script> +import { GlLink, GlSprintf } from '@gitlab/ui'; +import { s__ } from '~/locale'; +import CodeInstruction from './code_instruction.vue'; +import { TrackingActions } from '../constants'; +import { mapGetters, mapState } from 'vuex'; + +export default { + name: 'ComposerInstallation', + components: { + CodeInstruction, + GlLink, + GlSprintf, + }, + computed: { + ...mapState(['composerHelpPath']), + ...mapGetters(['composerRegistryInclude', 'composerPackageInclude']), + }, + i18n: { + registryInclude: s__('PackageRegistry|composer.json registry include'), + copyRegistryInclude: s__('PackageRegistry|Copy registry include'), + packageInclude: s__('PackageRegistry|composer.json require package include'), + copyPackageInclude: s__('PackageRegistry|Copy require package include'), + infoLine: s__( + 'PackageRegistry|For more information on Composer packages in GitLab, %{linkStart}see the documentation.%{linkEnd}', + ), + }, + trackingActions: { ...TrackingActions }, +}; +</script> + +<template> + <div> + <p class="gl-mt-3 gl-font-weight-bold" data-testid="registry-include-title"> + {{ $options.i18n.registryInclude }} + </p> + <code-instruction + :instruction="composerRegistryInclude" + :copy-text="$options.i18n.copyRegistryInclude" + :tracking-action="$options.trackingActions.COPY_COMPOSER_REGISTRY_INCLUDE_COMMAND" + /> + + <p class="gl-mt-3 gl-font-weight-bold" data-testid="package-include-title"> + {{ $options.i18n.packageInclude }} + </p> + <code-instruction + :instruction="composerPackageInclude" + :copy-text="$options.i18n.copyPackageInclude" + :tracking-action="$options.trackingActions.COPY_COMPOSER_PACKAGE_INCLUDE_COMMAND" + /> + <span data-testid="help-text"> + <gl-sprintf :message="$options.i18n.infoLine"> + <template #link="{ content }"> + <gl-link :href="composerHelpPath" target="_blank">{{ content }}</gl-link> + </template> + </gl-sprintf> + </span> + </div> +</template> diff --git a/app/assets/javascripts/packages/details/components/installation_commands.vue b/app/assets/javascripts/packages/details/components/installation_commands.vue index 8ed1c0f267f..219e72df9dc 100644 --- a/app/assets/javascripts/packages/details/components/installation_commands.vue +++ b/app/assets/javascripts/packages/details/components/installation_commands.vue @@ -4,6 +4,7 @@ import MavenInstallation from './maven_installation.vue'; import NpmInstallation from './npm_installation.vue'; import NugetInstallation from './nuget_installation.vue'; import PypiInstallation from './pypi_installation.vue'; +import ComposerInstallation from './composer_installation.vue'; import { PackageType } from '../../shared/constants'; export default { @@ -14,6 +15,7 @@ export default { [PackageType.NPM]: NpmInstallation, [PackageType.NUGET]: NugetInstallation, [PackageType.PYPI]: PypiInstallation, + [PackageType.COMPOSER]: ComposerInstallation, }, props: { packageEntity: { diff --git a/app/assets/javascripts/packages/details/constants.js b/app/assets/javascripts/packages/details/constants.js index 88469656eb2..c6e1b388132 100644 --- a/app/assets/javascripts/packages/details/constants.js +++ b/app/assets/javascripts/packages/details/constants.js @@ -7,6 +7,7 @@ export const TrackingLabels = { NPM_INSTALLATION: 'npm_installation', NUGET_INSTALLATION: 'nuget_installation', PYPI_INSTALLATION: 'pypi_installation', + COMPOSER_INSTALLATION: 'composer_installation', }; export const TrackingActions = { @@ -31,6 +32,9 @@ export const TrackingActions = { COPY_PIP_INSTALL_COMMAND: 'copy_pip_install_command', COPY_PYPI_SETUP_COMMAND: 'copy_pypi_setup_command', + + COPY_COMPOSER_REGISTRY_INCLUDE_COMMAND: 'copy_composer_registry_include_command', + COPY_COMPOSER_PACKAGE_INCLUDE_COMMAND: 'copy_composer_package_include_command', }; export const NpmManager = { diff --git a/app/assets/javascripts/packages/details/store/getters.js b/app/assets/javascripts/packages/details/store/getters.js index bcf74713f03..77dc24ff169 100644 --- a/app/assets/javascripts/packages/details/store/getters.js +++ b/app/assets/javascripts/packages/details/store/getters.js @@ -104,3 +104,12 @@ export const pypiSetupCommand = ({ pypiSetupPath }) => `[gitlab] repository = ${pypiSetupPath} username = __token__ password = <your personal access token>`; + +export const composerRegistryInclude = ({ composerPath }) => { + const base = { type: 'composer', url: composerPath }; + return JSON.stringify(base); +}; +export const composerPackageInclude = ({ packageEntity }) => { + const base = { package_name: packageEntity.name }; + return JSON.stringify(base); +}; diff --git a/app/controllers/import/gitea_controller.rb b/app/controllers/import/gitea_controller.rb index efeff8439e4..4785a71b8a1 100644 --- a/app/controllers/import/gitea_controller.rb +++ b/app/controllers/import/gitea_controller.rb @@ -54,6 +54,16 @@ class Import::GiteaController < Import::GithubController end end + override :client_repos + def client_repos + @client_repos ||= filtered(client.repos) + end + + override :client + def client + @client ||= Gitlab::LegacyGithubImport::Client.new(session[access_token_key], client_options) + end + override :client_options def client_options { host: provider_url, api_version: 'v1' } diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb index ac6b8c06d66..29fe34f0734 100644 --- a/app/controllers/import/github_controller.rb +++ b/app/controllers/import/github_controller.rb @@ -10,6 +10,9 @@ class Import::GithubController < Import::BaseController before_action :provider_auth, only: [:status, :realtime_changes, :create] before_action :expire_etag_cache, only: [:status, :create] + OAuthConfigMissingError = Class.new(StandardError) + + rescue_from OAuthConfigMissingError, with: :missing_oauth_config rescue_from Octokit::Unauthorized, with: :provider_unauthorized rescue_from Octokit::TooManyRequests, with: :provider_rate_limit @@ -22,7 +25,7 @@ class Import::GithubController < Import::BaseController end def callback - session[access_token_key] = client.get_token(params[:code]) + session[access_token_key] = get_token(params[:code]) redirect_to status_import_url end @@ -77,9 +80,7 @@ class Import::GithubController < Import::BaseController override :provider_url def provider_url strong_memoize(:provider_url) do - provider = Gitlab::Auth::OAuth::Provider.config_for('github') - - provider&.dig('url').presence || 'https://github.com' + oauth_config&.dig('url').presence || 'https://github.com' end end @@ -104,11 +105,66 @@ class Import::GithubController < Import::BaseController end def client - @client ||= Gitlab::LegacyGithubImport::Client.new(session[access_token_key], client_options) + @client ||= if Feature.enabled?(:remove_legacy_github_client) + Gitlab::GithubImport::Client.new(session[access_token_key]) + else + Gitlab::LegacyGithubImport::Client.new(session[access_token_key], client_options) + end end def client_repos - @client_repos ||= filtered(client.repos) + @client_repos ||= if Feature.enabled?(:remove_legacy_github_client) + filtered(concatenated_repos) + else + filtered(client.repos) + end + end + + def concatenated_repos + return [] unless client.respond_to?(:each_page) + + client.each_page(:repos).flat_map(&:objects) + end + + def oauth_client + raise OAuthConfigMissingError unless oauth_config + + @oauth_client ||= ::OAuth2::Client.new( + oauth_config.app_id, + oauth_config.app_secret, + oauth_options.merge(ssl: { verify: oauth_config['verify_ssl'] }) + ) + end + + def oauth_config + @oauth_config ||= Gitlab::Auth::OAuth::Provider.config_for('github') + end + + def oauth_options + if oauth_config + oauth_config.dig('args', 'client_options').deep_symbolize_keys + else + OmniAuth::Strategies::GitHub.default_options[:client_options].symbolize_keys + end + end + + def authorize_url + if Feature.enabled?(:remove_legacy_github_client) + oauth_client.auth_code.authorize_url( + redirect_uri: callback_import_url, + scope: 'repo, user, user:email' + ) + else + client.authorize_url(callback_import_url) + end + end + + def get_token(code) + if Feature.enabled?(:remove_legacy_github_client) + oauth_client.auth_code.get_token(code).token + else + client.get_token(code) + end end def verify_import_enabled @@ -116,7 +172,7 @@ class Import::GithubController < Import::BaseController end def go_to_provider_for_permissions - redirect_to client.authorize_url(callback_import_url) + redirect_to authorize_url end def import_enabled? @@ -152,6 +208,12 @@ class Import::GithubController < Import::BaseController alert: _("GitHub API rate limit exceeded. Try again after %{reset_time}") % { reset_time: reset_time } end + def missing_oauth_config + session[access_token_key] = nil + redirect_to new_import_url, + alert: _('Missing OAuth configuration for GitHub.') + end + def access_token_key :"#{provider_name}_access_token" end diff --git a/app/helpers/packages_helper.rb b/app/helpers/packages_helper.rb index a0434284ce6..e6ecc403a88 100644 --- a/app/helpers/packages_helper.rb +++ b/app/helpers/packages_helper.rb @@ -30,6 +30,10 @@ module PackagesHelper full_url.sub!('://', '://__token__:<your_personal_token>@') end + def composer_registry_url(group_id) + expose_url(api_v4_group___packages_composer_packages_path(id: group_id, format: '.json')) + end + def packages_coming_soon_enabled?(resource) ::Feature.enabled?(:packages_coming_soon, resource) && ::Gitlab.dev_env_or_com? end diff --git a/app/policies/personal_access_token_policy.rb b/app/policies/personal_access_token_policy.rb index aa87550fd6b..1e5404b7822 100644 --- a/app/policies/personal_access_token_policy.rb +++ b/app/policies/personal_access_token_policy.rb @@ -3,7 +3,7 @@ class PersonalAccessTokenPolicy < BasePolicy condition(:is_owner) { user && subject.user_id == user.id } - rule { is_owner | admin & ~blocked }.policy do + rule { (is_owner | admin) & ~blocked }.policy do enable :read_token enable :revoke_token end diff --git a/app/services/import/github_service.rb b/app/services/import/github_service.rb index 0cf17568c78..a2923b1e4f9 100644 --- a/app/services/import/github_service.rb +++ b/app/services/import/github_service.rb @@ -33,7 +33,7 @@ module Import end def repo - @repo ||= client.repo(params[:repo_id].to_i) + @repo ||= client.repository(params[:repo_id].to_i) end def project_name diff --git a/app/views/projects/packages/packages/show.html.haml b/app/views/projects/packages/packages/show.html.haml index 2f547a4811f..a66ae466d9d 100644 --- a/app/views/projects/packages/packages/show.html.haml +++ b/app/views/projects/packages/packages/show.html.haml @@ -20,4 +20,6 @@ pypi_path: pypi_registry_url(@project.id), pypi_setup_path: package_registry_project_url(@project.id, :pypi), pypi_help_path: help_page_path('user/packages/pypi_repository/index'), + composer_path: composer_registry_url(@project&.group&.id), + composer_help_path: help_page_path('user/packages/composer_repository/index'), project_name: @project.name} } diff --git a/changelogs/unreleased/232794-include-the-installation-instructions-for-composer-dependencies-in.yml b/changelogs/unreleased/232794-include-the-installation-instructions-for-composer-dependencies-in.yml new file mode 100644 index 00000000000..a781dd15804 --- /dev/null +++ b/changelogs/unreleased/232794-include-the-installation-instructions-for-composer-dependencies-in.yml @@ -0,0 +1,5 @@ +--- +title: Add installation instructions for Composer +merge_request: 38779 +author: +type: changed diff --git a/changelogs/unreleased/astoicescu-actions_menu_update.yml b/changelogs/unreleased/astoicescu-actions_menu_update.yml new file mode 100644 index 00000000000..02b85ac7c6b --- /dev/null +++ b/changelogs/unreleased/astoicescu-actions_menu_update.yml @@ -0,0 +1,5 @@ +--- +title: Change UI and add new actions to monitor dashboard actions menu +merge_request: 38946 +author: +type: changed diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 61adc3283b9..ee55360e5f5 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -2391,7 +2391,7 @@ input DastOnDemandScanCreateInput { """ ID of the site profile to be used for the scan. """ - dastSiteProfileId: ID! + dastSiteProfileId: DastSiteProfileID! """ The project the site profile belongs to. diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index f61025aedc7..3fe75338d84 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -6374,7 +6374,7 @@ "name": null, "ofType": { "kind": "SCALAR", - "name": "ID", + "name": "DastSiteProfileID", "ofType": null } }, diff --git a/doc/operations/metrics/dashboards/img/actions_menu_create_new_dashboard_v13_2.png b/doc/operations/metrics/dashboards/img/actions_menu_create_new_dashboard_v13_2.png Binary files differdeleted file mode 100644 index 5d530a80421..00000000000 --- a/doc/operations/metrics/dashboards/img/actions_menu_create_new_dashboard_v13_2.png +++ /dev/null diff --git a/doc/operations/metrics/dashboards/img/actions_menu_create_new_dashboard_v13_3.png b/doc/operations/metrics/dashboards/img/actions_menu_create_new_dashboard_v13_3.png Binary files differnew file mode 100644 index 00000000000..4b7a8418eef --- /dev/null +++ b/doc/operations/metrics/dashboards/img/actions_menu_create_new_dashboard_v13_3.png diff --git a/doc/operations/metrics/dashboards/img/metrics_settings_button_v13_2.png b/doc/operations/metrics/dashboards/img/metrics_settings_button_v13_2.png Binary files differdeleted file mode 100644 index d649f77eded..00000000000 --- a/doc/operations/metrics/dashboards/img/metrics_settings_button_v13_2.png +++ /dev/null diff --git a/doc/operations/metrics/dashboards/img/metrics_settings_button_v13_3.png b/doc/operations/metrics/dashboards/img/metrics_settings_button_v13_3.png Binary files differnew file mode 100644 index 00000000000..9c0eac12a3f --- /dev/null +++ b/doc/operations/metrics/dashboards/img/metrics_settings_button_v13_3.png diff --git a/doc/operations/metrics/dashboards/index.md b/doc/operations/metrics/dashboards/index.md index 9207a6bd951..b9008c18e13 100644 --- a/doc/operations/metrics/dashboards/index.md +++ b/doc/operations/metrics/dashboards/index.md @@ -20,7 +20,7 @@ The metrics as defined below do not support alerts, unlike ## Add a new dashboard to your project -> UI option [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/223204) in GitLab 13.2. +> UI option [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/228856) in GitLab 13.3. You can configure a custom dashboard by adding a new YAML file into your project's `.gitlab/dashboards/` directory. For the dashboard to display on your project's **Operations > Metrics** page, the files must have a `.yml` @@ -31,9 +31,9 @@ To create a new dashboard from the GitLab user interface: 1. Sign in to GitLab as a user with Maintainer or Owner [permissions](../../../user/permissions.md#project-members-permissions). 1. Navigate to your dashboard at **Operations > Metrics**. -1. In the top-right corner of your dashboard, click the **{file-addition-solid}** **Actions** menu, +1. In the top-right corner of your dashboard, click the **{{ellipsis_v}}** **More actions** menu, and select **Create new**: -  +  1. In the modal window, click **Open Repository**, then follow the instructions for creating a new dashboard from the command line. @@ -82,7 +82,7 @@ The resulting `.yml` file can be customized and adapted to your project. You can decide to save the dashboard `.yml` file in the project's **default** branch or in a new branch. -1. Click **Duplicate dashboard** in the actions menu. +1. Click **Duplicate current dashboard** in the **{{ellipsis_v}}** **More actions** menu. NOTE: **Note:** You can duplicate only GitLab-defined dashboards. @@ -105,7 +105,7 @@ To manage the settings for your metrics dashboard: 1. Navigate to your dashboard at **Operations > Metrics**. 1. In the top-right corner of your dashboard, click **Metrics Settings**: -  +  ## Chart Context Menu diff --git a/doc/operations/metrics/img/example-dashboard_v13_1.png b/doc/operations/metrics/img/example-dashboard_v13_1.png Binary files differdeleted file mode 100644 index 0805346b916..00000000000 --- a/doc/operations/metrics/img/example-dashboard_v13_1.png +++ /dev/null diff --git a/doc/operations/metrics/img/example-dashboard_v13_3.png b/doc/operations/metrics/img/example-dashboard_v13_3.png Binary files differnew file mode 100644 index 00000000000..1178b4a9be7 --- /dev/null +++ b/doc/operations/metrics/img/example-dashboard_v13_3.png diff --git a/doc/operations/metrics/img/prometheus_monitoring_dashboard_v13_1.png b/doc/operations/metrics/img/prometheus_monitoring_dashboard_v13_1.png Binary files differdeleted file mode 100644 index 56a0a508a1d..00000000000 --- a/doc/operations/metrics/img/prometheus_monitoring_dashboard_v13_1.png +++ /dev/null diff --git a/doc/operations/metrics/img/prometheus_monitoring_dashboard_v13_3.png b/doc/operations/metrics/img/prometheus_monitoring_dashboard_v13_3.png Binary files differnew file mode 100644 index 00000000000..1178b4a9be7 --- /dev/null +++ b/doc/operations/metrics/img/prometheus_monitoring_dashboard_v13_3.png diff --git a/doc/operations/metrics/index.md b/doc/operations/metrics/index.md index de817fe0c52..99b3a485002 100644 --- a/doc/operations/metrics/index.md +++ b/doc/operations/metrics/index.md @@ -25,7 +25,7 @@ To view the metrics dashboard for an environment that has GitLab displays the default metrics dashboard for the environment, like the following example: - + The top of the dashboard contains a navigation bar. From left to right, the navigation bar contains: @@ -37,15 +37,19 @@ navigation bar contains: - **Range** - The time period of data to display. - **Refresh dashboard** **{retry}** - Reload the dashboard with current data. - **Set refresh rate** - Set a time frame for refreshing the data displayed. -- **Star dashboard** **{star-o}** - Click to mark a dashboard as a favorite. +- **More actions** **{ellipsis_v}** - More dashboard actions + - **Add metric** - Adds a [custom metric](#adding-custom-metrics). Only available on GitLab-defined dashboards. + ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/34779) in GitLab 12.5.) + - **Edit dashboard YAML** - Edit the source YAML file of a custom dashboard. Only available on + [custom dashboards](dashboards/index.md). + ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/34779) in GitLab 12.5.) + - **Duplicate current dashboard** - Save a [complete copy of a dashboard](dashboards/index.md#duplicate-a-gitlab-defined-dashboard). Only available on GitLab-defined dashboards. + - **Star dashboard** **{star-o}** - Click to mark a dashboard as a favorite. Starred dashboards display a solid star **{star}** button, and display first in the **Dashboard** dropdown list. ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/214582) in GitLab 13.0.) -- **Edit dashboard** - Edit the source YAML file of a custom dashboard. Only available on - [custom dashboards](dashboards/index.md). - ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/34779) in GitLab 12.5.) -- **Create dashboard** **{file-addition-solid}** - Create a - [new custom dashboard for your project](dashboards/index.md#add-a-new-dashboard-to-your-project). + - **Create new dashboard** - Create a [new custom dashboard for your project](dashboards/index.md#add-a-new-dashboard-to-your-project). + ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/228856) in GitLab 13.3.) - **Metrics settings** - Configure the [settings for this dashboard](dashboards/index.md#manage-the-metrics-dashboard-settings). @@ -70,7 +74,7 @@ helps quickly create a deployment: 1. When the pipeline has run successfully, graphs are available on the **Operations > Metrics** page. - + ## Customize your metrics dashboard diff --git a/lib/api/import_github.rb b/lib/api/import_github.rb index 1e839816006..0bab891eada 100644 --- a/lib/api/import_github.rb +++ b/lib/api/import_github.rb @@ -10,7 +10,11 @@ module API helpers do def client - @client ||= Gitlab::LegacyGithubImport::Client.new(params[:personal_access_token], client_options) + @client ||= if Feature.enabled?(:remove_legacy_github_client) + Gitlab::GithubImport::Client.new(params[:personal_access_token]) + else + Gitlab::LegacyGithubImport::Client.new(params[:personal_access_token], client_options) + end end def access_params diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 73c4ba5002d..09468fbdde6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8804,9 +8804,6 @@ msgstr "" msgid "Edit comment" msgstr "" -msgid "Edit dashboard" -msgstr "" - msgid "Edit description" msgstr "" @@ -15236,9 +15233,6 @@ msgstr "" msgid "Metrics|Create custom dashboard %{fileName}" msgstr "" -msgid "Metrics|Create dashboard" -msgstr "" - msgid "Metrics|Create metric" msgstr "" @@ -15272,9 +15266,15 @@ msgstr "" msgid "Metrics|Duplicate dashboard" msgstr "" +msgid "Metrics|Duplicate this dashboard to edit dashboard YAML" +msgstr "" + msgid "Metrics|Duplicating..." msgstr "" +msgid "Metrics|Edit dashboard YAML" +msgstr "" + msgid "Metrics|Edit metric" msgid_plural "Metrics|Edit metrics" msgstr[0] "" @@ -15316,6 +15316,9 @@ msgstr "" msgid "Metrics|Min" msgstr "" +msgid "Metrics|More actions" +msgstr "" + msgid "Metrics|Must be a valid PromQL query." msgstr "" @@ -15639,6 +15642,9 @@ msgstr "" msgid "Mirroring will only be available if the feature is included in the plan of the selected group or user." msgstr "" +msgid "Missing OAuth configuration for GitHub." +msgstr "" + msgid "Missing commit signatures endpoint!" msgstr "" @@ -17122,6 +17128,12 @@ msgstr "" msgid "PackageRegistry|Copy npm setup command" msgstr "" +msgid "PackageRegistry|Copy registry include" +msgstr "" + +msgid "PackageRegistry|Copy require package include" +msgstr "" + msgid "PackageRegistry|Copy yarn command" msgstr "" @@ -17137,6 +17149,9 @@ msgstr "" msgid "PackageRegistry|Filter by name" msgstr "" +msgid "PackageRegistry|For more information on Composer packages in GitLab, %{linkStart}see the documentation.%{linkEnd}" +msgstr "" + msgid "PackageRegistry|For more information on the Conan registry, %{linkStart}see the documentation%{linkEnd}." msgstr "" @@ -17257,6 +17272,12 @@ msgstr "" msgid "PackageRegistry|You may also need to setup authentication using an auth token. %{linkStart}See the documentation%{linkEnd} to find out more." msgstr "" +msgid "PackageRegistry|composer.json registry include" +msgstr "" + +msgid "PackageRegistry|composer.json require package include" +msgstr "" + msgid "PackageRegistry|npm" msgstr "" diff --git a/qa/qa/flow/saml.rb b/qa/qa/flow/saml.rb index 676be2beb01..e8007978071 100644 --- a/qa/qa/flow/saml.rb +++ b/qa/qa/flow/saml.rb @@ -18,7 +18,7 @@ module QA end end - def enable_saml_sso(group, saml_idp_service) + def enable_saml_sso(group, saml_idp_service, default_membership_role = 'Guest') page.visit Runtime::Scenario.gitlab_address Page::Main::Login.perform(&:sign_in_using_credentials) unless Page::Main::Menu.perform(&:signed_in?) @@ -29,6 +29,7 @@ module QA EE::Page::Group::Settings::SamlSSO.perform do |saml_sso| saml_sso.set_id_provider_sso_url(saml_idp_service.idp_sso_url) saml_sso.set_cert_fingerprint(saml_idp_service.idp_certificate_fingerprint) + saml_sso.set_default_membership_role(default_membership_role) saml_sso.click_save_changes saml_sso.user_login_url_link_text diff --git a/qa/qa/page/project/operations/metrics/show.rb b/qa/qa/page/project/operations/metrics/show.rb index ee5f42147f6..7576e11bf59 100644 --- a/qa/qa/page/project/operations/metrics/show.rb +++ b/qa/qa/page/project/operations/metrics/show.rb @@ -18,9 +18,12 @@ module QA view 'app/assets/javascripts/monitoring/components/dashboard_header.vue' do element :dashboards_filter_dropdown element :environments_dropdown - element :edit_dashboard_button element :range_picker_dropdown + end + + view 'app/assets/javascripts/monitoring/components/dashboard_actions_menu.vue' do element :actions_menu_dropdown + element :edit_dashboard_button_enabled end view 'app/assets/javascripts/monitoring/components/duplicate_dashboard_form.vue' do @@ -56,7 +59,7 @@ module QA def has_edit_dashboard_enabled? within_element :prometheus_graphs do - has_element? :edit_dashboard_button + has_element? :edit_dashboard_button_enabled end end diff --git a/spec/controllers/import/gitea_controller_spec.rb b/spec/controllers/import/gitea_controller_spec.rb index 9001faef408..3e4b159271a 100644 --- a/spec/controllers/import/gitea_controller_spec.rb +++ b/spec/controllers/import/gitea_controller_spec.rb @@ -34,6 +34,14 @@ RSpec.describe Import::GiteaController do assign_host_url end + it "requests provider repos list" do + expect(stub_client(repos: [], orgs: [])).to receive(:repos) + + get :status + + expect(response).to have_gitlab_http_status(:ok) + end + context 'when host url is local or not http' do %w[https://localhost:3000 http://192.168.0.1 ftp://testing].each do |url| let(:host_url) { url } diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index a5a3dc463d3..e19b6caca5b 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -15,10 +15,7 @@ RSpec.describe Import::GithubController do it "redirects to GitHub for an access token if logged in with GitHub" do allow(controller).to receive(:logged_in_with_provider?).and_return(true) expect(controller).to receive(:go_to_provider_for_permissions).and_call_original - allow_any_instance_of(Gitlab::LegacyGithubImport::Client) - .to receive(:authorize_url) - .with(users_import_github_callback_url) - .and_call_original + allow(controller).to receive(:authorize_url).and_call_original get :new @@ -46,13 +43,15 @@ RSpec.describe Import::GithubController do end describe "GET callback" do + before do + allow(controller).to receive(:get_token).and_return(token) + allow(controller).to receive(:oauth_options).and_return({}) + + stub_omniauth_provider('github') + end + it "updates access token" do token = "asdasd12345" - allow_any_instance_of(Gitlab::LegacyGithubImport::Client) - .to receive(:get_token).and_return(token) - allow_any_instance_of(Gitlab::LegacyGithubImport::Client) - .to receive(:github_options).and_return({}) - stub_omniauth_provider('github') get :callback @@ -66,7 +65,86 @@ RSpec.describe Import::GithubController do end describe "GET status" do - it_behaves_like 'a GitHub-ish import controller: GET status' + context 'when using OAuth' do + before do + allow(controller).to receive(:logged_in_with_provider?).and_return(true) + end + + context 'when OAuth config is missing' do + let(:new_import_url) { public_send("new_import_#{provider}_url") } + + before do + allow(controller).to receive(:oauth_config).and_return(nil) + end + + it 'returns missing config error' do + expect(controller).to receive(:go_to_provider_for_permissions).and_call_original + + get :status + + expect(session[:"#{provider}_access_token"]).to be_nil + expect(controller).to redirect_to(new_import_url) + expect(flash[:alert]).to eq('Missing OAuth configuration for GitHub.') + end + end + end + + context 'when feature remove_legacy_github_client is disabled' do + before do + stub_feature_flags(remove_legacy_github_client: false) + session[:"#{provider}_access_token"] = 'asdasd12345' + end + + it_behaves_like 'a GitHub-ish import controller: GET status' + + it 'uses Gitlab::LegacyGitHubImport::Client' do + expect(controller.send(:client)).to be_instance_of(Gitlab::LegacyGithubImport::Client) + end + + it 'fetches repos using legacy client' do + expect_next_instance_of(Gitlab::LegacyGithubImport::Client) do |client| + expect(client).to receive(:repos) + end + + get :status + end + end + + context 'when feature remove_legacy_github_client is enabled' do + before do + stub_feature_flags(remove_legacy_github_client: true) + session[:"#{provider}_access_token"] = 'asdasd12345' + end + + it_behaves_like 'a GitHub-ish import controller: GET status' + + it 'uses Gitlab::GithubImport::Client' do + expect(controller.send(:client)).to be_instance_of(Gitlab::GithubImport::Client) + end + + it 'fetches repos using latest github client' do + expect_next_instance_of(Gitlab::GithubImport::Client) do |client| + expect(client).to receive(:each_page).with(:repos).and_return([].to_enum) + end + + get :status + end + + it 'concatenates list of repos from multiple pages' do + repo_1 = OpenStruct.new(login: 'emacs', full_name: 'asd/emacs', name: 'emacs', owner: { login: 'owner' }) + repo_2 = OpenStruct.new(login: 'vim', full_name: 'asd/vim', name: 'vim', owner: { login: 'owner' }) + repos = [OpenStruct.new(objects: [repo_1]), OpenStruct.new(objects: [repo_2])].to_enum + + allow(stub_client).to receive(:each_page).and_return(repos) + + get :status, format: :json + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.dig('provider_repos').count).to eq(2) + expect(json_response.dig('provider_repos', 0, 'id')).to eq(repo_1.id) + expect(json_response.dig('provider_repos', 1, 'id')).to eq(repo_2.id) + end + end end describe "POST create" do diff --git a/spec/features/issuables/issuable_list_spec.rb b/spec/features/issuables/issuable_list_spec.rb index abebff00f76..7790d8f1c4c 100644 --- a/spec/features/issuables/issuable_list_spec.rb +++ b/spec/features/issuables/issuable_list_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'issuable list', :js do end issuable_types.each do |issuable_type| - it "avoids N+1 database queries for #{issuable_type.to_s.humanize.pluralize}" do + it "avoids N+1 database queries for #{issuable_type.to_s.humanize.pluralize}", quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/231426' } do control_count = ActiveRecord::QueryRecorder.new { visit_issuable_list(issuable_type) }.count create_issuables(issuable_type) diff --git a/spec/frontend/flash_spec.js b/spec/frontend/flash_spec.js index fa7c1904339..68e506702c7 100644 --- a/spec/frontend/flash_spec.js +++ b/spec/frontend/flash_spec.js @@ -1,4 +1,10 @@ -import flash, { createFlashEl, createAction, hideFlash, removeFlashClickListener } from '~/flash'; +import flash, { + newCreateFlash, + createFlashEl, + createAction, + hideFlash, + removeFlashClickListener, +} from '~/flash'; describe('Flash', () => { describe('createFlashEl', () => { @@ -205,6 +211,109 @@ describe('Flash', () => { }); }); + describe('newCreateFlash', () => { + const message = 'test'; + const type = 'alert'; + const parent = document; + const fadeTransition = false; + const addBodyClass = true; + const defaultParams = { + message, + type, + parent, + actionConfig: null, + fadeTransition, + addBodyClass, + }; + + describe('no flash-container', () => { + it('does not add to the DOM', () => { + const flashEl = newCreateFlash({ message }); + + expect(flashEl).toBeNull(); + + expect(document.querySelector('.flash-alert')).toBeNull(); + }); + }); + + describe('with flash-container', () => { + beforeEach(() => { + setFixtures( + '<div class="content-wrapper js-content-wrapper"><div class="flash-container"></div></div>', + ); + }); + + afterEach(() => { + document.querySelector('.js-content-wrapper').remove(); + }); + + it('adds flash element into container', () => { + newCreateFlash({ ...defaultParams }); + + expect(document.querySelector('.flash-alert')).not.toBeNull(); + + expect(document.body.className).toContain('flash-shown'); + }); + + it('adds flash into specified parent', () => { + newCreateFlash({ ...defaultParams, parent: document.querySelector('.content-wrapper') }); + + expect(document.querySelector('.content-wrapper .flash-alert')).not.toBeNull(); + expect(document.querySelector('.content-wrapper').innerText.trim()).toEqual(message); + }); + + it('adds container classes when inside content-wrapper', () => { + newCreateFlash(defaultParams); + + expect(document.querySelector('.flash-text').className).toBe('flash-text'); + expect(document.querySelector('.content-wrapper').innerText.trim()).toEqual(message); + }); + + it('does not add container when outside of content-wrapper', () => { + document.querySelector('.content-wrapper').className = 'js-content-wrapper'; + newCreateFlash(defaultParams); + + expect(document.querySelector('.flash-text').className.trim()).toContain('flash-text'); + }); + + it('removes element after clicking', () => { + newCreateFlash({ ...defaultParams }); + + document.querySelector('.flash-alert .js-close-icon').click(); + + expect(document.querySelector('.flash-alert')).toBeNull(); + + expect(document.body.className).not.toContain('flash-shown'); + }); + + describe('with actionConfig', () => { + it('adds action link', () => { + newCreateFlash({ + ...defaultParams, + actionConfig: { + title: 'test', + }, + }); + + expect(document.querySelector('.flash-action')).not.toBeNull(); + }); + + it('calls actionConfig clickHandler on click', () => { + const actionConfig = { + title: 'test', + clickHandler: jest.fn(), + }; + + newCreateFlash({ ...defaultParams, actionConfig }); + + document.querySelector('.flash-action').click(); + + expect(actionConfig.clickHandler).toHaveBeenCalled(); + }); + }); + }); + }); + describe('removeFlashClickListener', () => { beforeEach(() => { document.body.innerHTML += ` diff --git a/spec/frontend/monitoring/components/__snapshots__/dashboard_template_spec.js.snap b/spec/frontend/monitoring/components/__snapshots__/dashboard_template_spec.js.snap index 5662d1c3700..f50b14570af 100644 --- a/spec/frontend/monitoring/components/__snapshots__/dashboard_template_spec.js.snap +++ b/spec/frontend/monitoring/components/__snapshots__/dashboard_template_spec.js.snap @@ -99,34 +99,19 @@ exports[`Dashboard template matches the default snapshot 1`] = ` <div class="d-sm-flex" > - <div - class="mb-2 mr-2 d-flex" - > - <div - class="flex-grow-1" - title="Star dashboard" - > - <gl-button-stub - category="tertiary" - class="w-100" - icon="star-o" - size="medium" - variant="default" - /> - </div> - </div> - - <!----> - - <!----> - - <!----> - <!----> <!----> - <!----> + <div + class="gl-mb-3 gl-mr-3 d-flex d-sm-block" + > + <actions-menu-stub + custommetricspath="/monitoring/monitor-project/prometheus/metrics" + defaultbranch="master" + validatequerypath="/monitoring/monitor-project/prometheus/metrics/validate_query" + /> + </div> <!----> </div> diff --git a/spec/frontend/monitoring/components/dashboard_actions_menu_spec.js b/spec/frontend/monitoring/components/dashboard_actions_menu_spec.js new file mode 100644 index 00000000000..1f6178b895b --- /dev/null +++ b/spec/frontend/monitoring/components/dashboard_actions_menu_spec.js @@ -0,0 +1,388 @@ +import { shallowMount } from '@vue/test-utils'; +import { createStore } from '~/monitoring/stores'; +import { setupAllDashboards, setupStoreWithData } from '../store_utils'; +import { redirectTo } from '~/lib/utils/url_utility'; +import Tracking from '~/tracking'; +import ActionsMenu from '~/monitoring/components/dashboard_actions_menu.vue'; +import CustomMetricsFormFields from '~/custom_metrics/components/custom_metrics_form_fields.vue'; +import { dashboardActionsMenuProps, dashboardGitResponse } from '../mock_data'; +import * as types from '~/monitoring/stores/mutation_types'; + +jest.mock('~/lib/utils/url_utility', () => ({ + redirectTo: jest.fn(), + queryToObject: jest.fn(), +})); + +describe('Actions menu', () => { + const ootbDashboards = [dashboardGitResponse[0], dashboardGitResponse[2]]; + const customDashboard = dashboardGitResponse[1]; + + let store; + let wrapper; + + const findAddMetricItem = () => wrapper.find('[data-testid="add-metric-item"]'); + const findAddMetricModal = () => wrapper.find('[data-testid="add-metric-modal"]'); + const findAddMetricModalSubmitButton = () => + wrapper.find('[data-testid="add-metric-modal-submit-button"]'); + const findStarDashboardItem = () => wrapper.find('[data-testid="star-dashboard-item"]'); + const findEditDashboardItemEnabled = () => + wrapper.find('[data-testid="edit-dashboard-item-enabled"]'); + const findEditDashboardItemDisabled = () => + wrapper.find('[data-testid="edit-dashboard-item-disabled"]'); + const findDuplicateDashboardItem = () => wrapper.find('[data-testid="duplicate-dashboard-item"]'); + const findDuplicateDashboardModal = () => + wrapper.find('[data-testid="duplicate-dashboard-modal"]'); + const findCreateDashboardItem = () => wrapper.find('[data-testid="create-dashboard-item"]'); + const findCreateDashboardModal = () => wrapper.find('[data-testid="create-dashboard-modal"]'); + + const createShallowWrapper = (props = {}, options = {}) => { + wrapper = shallowMount(ActionsMenu, { + propsData: { ...dashboardActionsMenuProps, ...props }, + store, + ...options, + }); + }; + + beforeEach(() => { + store = createStore(); + }); + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + describe('add metric item', () => { + it('is rendered when custom metrics are available', () => { + createShallowWrapper(); + + return wrapper.vm.$nextTick(() => { + expect(findAddMetricItem().exists()).toBe(true); + }); + }); + + it('is not rendered when custom metrics are not available', () => { + createShallowWrapper({ + addingMetricsAvailable: false, + }); + + return wrapper.vm.$nextTick(() => { + expect(findAddMetricItem().exists()).toBe(false); + }); + }); + + describe('when available', () => { + beforeEach(() => { + createShallowWrapper(); + }); + + it('modal for custom metrics form is rendered', () => { + expect(findAddMetricModal().exists()).toBe(true); + expect(findAddMetricModal().attributes().modalid).toBe('addMetric'); + }); + + it('add metric modal submit button exists', () => { + expect(findAddMetricModalSubmitButton().exists()).toBe(true); + }); + + it('renders custom metrics form fields', () => { + expect(wrapper.find(CustomMetricsFormFields).exists()).toBe(true); + }); + }); + + describe('when not available', () => { + beforeEach(() => { + createShallowWrapper({ addingMetricsAvailable: false }); + }); + + it('modal for custom metrics form is not rendered', () => { + expect(findAddMetricModal().exists()).toBe(false); + }); + }); + + describe('adding new metric from modal', () => { + let origPage; + + beforeEach(done => { + jest.spyOn(Tracking, 'event').mockReturnValue(); + createShallowWrapper(); + + setupStoreWithData(store); + + origPage = document.body.dataset.page; + document.body.dataset.page = 'projects:environments:metrics'; + + wrapper.vm.$nextTick(done); + }); + + afterEach(() => { + document.body.dataset.page = origPage; + }); + + it('is tracked', done => { + const submitButton = findAddMetricModalSubmitButton().vm; + + wrapper.vm.$nextTick(() => { + submitButton.$el.click(); + wrapper.vm.$nextTick(() => { + expect(Tracking.event).toHaveBeenCalledWith( + document.body.dataset.page, + 'click_button', + { + label: 'add_new_metric', + property: 'modal', + value: undefined, + }, + ); + done(); + }); + }); + }); + }); + }); + + describe('edit dashboard yml item', () => { + beforeEach(() => { + createShallowWrapper(); + }); + + describe('when current dashboard is custom', () => { + beforeEach(() => { + setupAllDashboards(store, customDashboard.path); + }); + + it('enabled item is rendered and has falsy disabled attribute', () => { + expect(findEditDashboardItemEnabled().exists()).toBe(true); + expect(findEditDashboardItemEnabled().attributes('disabled')).toBe(undefined); + }); + + it('enabled item links to their edit path', () => { + expect(findEditDashboardItemEnabled().attributes('href')).toBe( + customDashboard.project_blob_path, + ); + }); + + it('disabled item is not rendered', () => { + expect(findEditDashboardItemDisabled().exists()).toBe(false); + }); + }); + + describe.each(ootbDashboards)('when current dashboard is OOTB', dashboard => { + beforeEach(() => { + setupAllDashboards(store, dashboard.path); + }); + + it('disabled item is rendered and has disabled attribute set on it', () => { + expect(findEditDashboardItemDisabled().exists()).toBe(true); + expect(findEditDashboardItemDisabled().attributes('disabled')).toBe(''); + }); + + it('enabled item is not rendered', () => { + expect(findEditDashboardItemEnabled().exists()).toBe(false); + }); + }); + }); + + describe('duplicate dashboard item', () => { + beforeEach(() => { + createShallowWrapper(); + }); + + describe.each(ootbDashboards)('when current dashboard is OOTB', dashboard => { + beforeEach(() => { + setupAllDashboards(store, dashboard.path); + }); + + it('is rendered', () => { + expect(findDuplicateDashboardItem().exists()).toBe(true); + }); + + it('duplicate dashboard modal is rendered', () => { + expect(findDuplicateDashboardModal().exists()).toBe(true); + }); + + it('clicking on item opens up the duplicate dashboard modal', () => { + const modalId = 'duplicateDashboard'; + const modalTrigger = findDuplicateDashboardItem(); + const rootEmit = jest.spyOn(wrapper.vm.$root, '$emit'); + + modalTrigger.trigger('click'); + + return wrapper.vm.$nextTick().then(() => { + expect(rootEmit.mock.calls[0]).toContainEqual(modalId); + }); + }); + }); + + describe('when current dashboard is custom', () => { + beforeEach(() => { + setupAllDashboards(store, customDashboard.path); + }); + + it('is not rendered', () => { + expect(findDuplicateDashboardItem().exists()).toBe(false); + }); + + it('duplicate dashboard modal is not rendered', () => { + expect(findDuplicateDashboardModal().exists()).toBe(false); + }); + }); + + describe('when no dashboard is set', () => { + it('is not rendered', () => { + expect(findDuplicateDashboardItem().exists()).toBe(false); + }); + + it('duplicate dashboard modal is not rendered', () => { + expect(findDuplicateDashboardModal().exists()).toBe(false); + }); + }); + + describe('when a dashboard has been duplicated in the duplicate dashboard modal', () => { + beforeEach(() => { + store.state.monitoringDashboard.projectPath = 'root/sandbox'; + + setupAllDashboards(store, dashboardGitResponse[0].path); + }); + + it('redirects to the newly created dashboard', () => { + delete window.location; + window.location = new URL('https://localhost'); + + const newDashboard = dashboardGitResponse[1]; + + const newDashboardUrl = 'root/sandbox/-/metrics/dashboard.yml'; + findDuplicateDashboardModal().vm.$emit('dashboardDuplicated', newDashboard); + + return wrapper.vm.$nextTick().then(() => { + expect(redirectTo).toHaveBeenCalled(); + expect(redirectTo).toHaveBeenCalledWith(newDashboardUrl); + }); + }); + }); + }); + + describe('star dashboard item', () => { + beforeEach(() => { + createShallowWrapper(); + setupAllDashboards(store); + + jest.spyOn(store, 'dispatch').mockResolvedValue(); + }); + + it('is shown', () => { + expect(findStarDashboardItem().exists()).toBe(true); + }); + + it('is not disabled', () => { + expect(findStarDashboardItem().attributes('disabled')).toBeFalsy(); + }); + + it('is disabled when starring is taking place', () => { + store.commit(`monitoringDashboard/${types.REQUEST_DASHBOARD_STARRING}`); + + return wrapper.vm.$nextTick(() => { + expect(findStarDashboardItem().exists()).toBe(true); + expect(findStarDashboardItem().attributes('disabled')).toBe('true'); + }); + }); + + it('on click it dispatches a toggle star action', () => { + findStarDashboardItem().vm.$emit('click'); + + return wrapper.vm.$nextTick().then(() => { + expect(store.dispatch).toHaveBeenCalledWith( + 'monitoringDashboard/toggleStarredValue', + undefined, + ); + }); + }); + + describe('when dashboard is not starred', () => { + beforeEach(() => { + store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, { + currentDashboard: dashboardGitResponse[0].path, + }); + return wrapper.vm.$nextTick(); + }); + + it('item text shows "Star dashboard"', () => { + expect(findStarDashboardItem().html()).toMatch(/Star dashboard/); + }); + }); + + describe('when dashboard is starred', () => { + beforeEach(() => { + store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, { + currentDashboard: dashboardGitResponse[1].path, + }); + return wrapper.vm.$nextTick(); + }); + + it('item text shows "Unstar dashboard"', () => { + expect(findStarDashboardItem().html()).toMatch(/Unstar dashboard/); + }); + }); + }); + + describe('create dashboard item', () => { + beforeEach(() => { + createShallowWrapper(); + }); + + it('is rendered by default but it is disabled', () => { + expect(findCreateDashboardItem().attributes('disabled')).toBe('true'); + }); + + describe('when project path is set', () => { + const mockProjectPath = 'root/sandbox'; + const mockAddDashboardDocPath = '/doc/add-dashboard'; + + beforeEach(() => { + store.state.monitoringDashboard.projectPath = mockProjectPath; + store.state.monitoringDashboard.addDashboardDocumentationPath = mockAddDashboardDocPath; + }); + + it('is not disabled', () => { + expect(findCreateDashboardItem().attributes('disabled')).toBe(undefined); + }); + + it('renders a modal for creating a dashboard', () => { + expect(findCreateDashboardModal().exists()).toBe(true); + }); + + it('clicking opens up the modal', () => { + const modalId = 'createDashboard'; + const modalTrigger = findCreateDashboardItem(); + const rootEmit = jest.spyOn(wrapper.vm.$root, '$emit'); + + modalTrigger.trigger('click'); + + return wrapper.vm.$nextTick().then(() => { + expect(rootEmit.mock.calls[0]).toContainEqual(modalId); + }); + }); + + it('modal gets passed correct props', () => { + expect(findCreateDashboardModal().props('projectPath')).toBe(mockProjectPath); + expect(findCreateDashboardModal().props('addDashboardDocumentationPath')).toBe( + mockAddDashboardDocPath, + ); + }); + }); + + describe('when project path is not set', () => { + beforeEach(() => { + store.state.monitoringDashboard.projectPath = null; + }); + + it('is disabled', () => { + expect(findCreateDashboardItem().attributes('disabled')).toBe('true'); + }); + + it('does not render a modal for creating a dashboard', () => { + expect(findCreateDashboardModal().exists()).toBe(false); + }); + }); + }); +}); diff --git a/spec/frontend/monitoring/components/dashboard_header_spec.js b/spec/frontend/monitoring/components/dashboard_header_spec.js index ad592a3354a..134c5f32bf7 100644 --- a/spec/frontend/monitoring/components/dashboard_header_spec.js +++ b/spec/frontend/monitoring/components/dashboard_header_spec.js @@ -6,8 +6,7 @@ import DateTimePicker from '~/vue_shared/components/date_time_picker/date_time_p import RefreshButton from '~/monitoring/components/refresh_button.vue'; import DashboardHeader from '~/monitoring/components/dashboard_header.vue'; import DashboardsDropdown from '~/monitoring/components/dashboards_dropdown.vue'; -import DuplicateDashboardModal from '~/monitoring/components/duplicate_dashboard_modal.vue'; -import CreateDashboardModal from '~/monitoring/components/create_dashboard_modal.vue'; +import ActionsMenu from '~/monitoring/components/dashboard_actions_menu.vue'; import { setupAllDashboards, setupStoreWithDashboard, setupStoreWithData } from '../store_utils'; import { environmentData, @@ -18,7 +17,6 @@ import { import { redirectTo } from '~/lib/utils/url_utility'; const mockProjectPath = 'https://path/to/project'; -const mockAddDashboardDocPath = '/doc/add-dashboard'; jest.mock('~/lib/utils/url_utility', () => ({ redirectTo: jest.fn(), @@ -41,13 +39,7 @@ describe('Dashboard header', () => { const findDateTimePicker = () => wrapper.find(DateTimePicker); const findRefreshButton = () => wrapper.find(RefreshButton); - const findActionsMenu = () => wrapper.find('[data-testid="actions-menu"]'); - const findCreateDashboardMenuItem = () => - findActionsMenu().find('[data-testid="action-create-dashboard"]'); - const findCreateDashboardDuplicateItem = () => - findActionsMenu().find('[data-testid="action-duplicate-dashboard"]'); - const findDuplicateDashboardModal = () => wrapper.find(DuplicateDashboardModal); - const findCreateDashboardModal = () => wrapper.find('[data-testid="create-dashboard-modal"]'); + const findActionsMenu = () => wrapper.find(ActionsMenu); const setSearchTerm = searchTerm => { store.commit(`monitoringDashboard/${types.SET_ENVIRONMENTS_FILTER}`, searchTerm); @@ -264,31 +256,6 @@ describe('Dashboard header', () => { }); }); - describe('when a dashboard has been duplicated in the duplicate dashboard modal', () => { - beforeEach(() => { - store.state.monitoringDashboard.projectPath = 'root/sandbox'; - - setupAllDashboards(store, dashboardGitResponse[0].path); - }); - - it('redirects to the newly created dashboard', () => { - delete window.location; - window.location = new URL('https://localhost'); - - const newDashboard = dashboardGitResponse[1]; - - createShallowWrapper(); - - const newDashboardUrl = 'root/sandbox/-/metrics/dashboard.yml'; - findDuplicateDashboardModal().vm.$emit('dashboardDuplicated', newDashboard); - - return wrapper.vm.$nextTick().then(() => { - expect(redirectTo).toHaveBeenCalled(); - expect(redirectTo).toHaveBeenCalledWith(newDashboardUrl); - }); - }); - }); - describe('external dashboard link', () => { beforeEach(() => { store.state.monitoringDashboard.externalDashboardUrl = '/mockUrl'; @@ -307,113 +274,97 @@ describe('Dashboard header', () => { }); describe('actions menu', () => { - beforeEach(() => { - store.state.monitoringDashboard.projectPath = ''; - createShallowWrapper(); - }); + const ootbDashboards = [ + dashboardGitResponse[0].path, + selfMonitoringDashboardGitResponse[0].path, + ]; + const customDashboards = [ + dashboardGitResponse[1].path, + selfMonitoringDashboardGitResponse[1].path, + ]; - it('is rendered if projectPath is set in store', () => { - store.state.monitoringDashboard.projectPath = mockProjectPath; + it('is rendered', () => { + createShallowWrapper(); - return wrapper.vm.$nextTick().then(() => { - expect(findActionsMenu().exists()).toBe(true); - }); + expect(findActionsMenu().exists()).toBe(true); }); - it('is not rendered if projectPath is not set in store', () => { - expect(findActionsMenu().exists()).toBe(false); - }); + describe('adding metrics prop', () => { + it.each(ootbDashboards)('gets passed true if current dashboard is OOTB', dashboardPath => { + createShallowWrapper({ customMetricsAvailable: true }); - it('contains the create dashboard modal', () => { - store.state.monitoringDashboard.projectPath = mockProjectPath; + store.state.monitoringDashboard.emptyState = false; + setupAllDashboards(store, dashboardPath); - return wrapper.vm.$nextTick().then(() => { - expect(findActionsMenu().contains(CreateDashboardModal)).toBe(true); + return wrapper.vm.$nextTick().then(() => { + expect(findActionsMenu().props('addingMetricsAvailable')).toBe(true); + }); }); - }); - const duplicableCases = [ - null, // When no path is specified, it uses the overview dashboard path. - dashboardGitResponse[0].path, - dashboardGitResponse[2].path, - selfMonitoringDashboardGitResponse[0].path, - ]; + it.each(customDashboards)( + 'gets passed false if current dashboard is custom', + dashboardPath => { + createShallowWrapper({ customMetricsAvailable: true }); - describe.each(duplicableCases)( - 'when the selected dashboard can be duplicated', - dashboardPath => { - it('contains menu items for "Create New", "Duplicate Dashboard" and a modal for duplicating dashboards', () => { - store.state.monitoringDashboard.projectPath = mockProjectPath; + store.state.monitoringDashboard.emptyState = false; setupAllDashboards(store, dashboardPath); return wrapper.vm.$nextTick().then(() => { - expect(findCreateDashboardMenuItem().exists()).toBe(true); - expect(findCreateDashboardDuplicateItem().exists()).toBe(true); - expect(findDuplicateDashboardModal().exists()).toBe(true); + expect(findActionsMenu().props('addingMetricsAvailable')).toBe(false); }); - }); - }, - ); + }, + ); - const nonDuplicableCases = [ - dashboardGitResponse[1].path, - selfMonitoringDashboardGitResponse[1].path, - ]; + it('gets passed false if empty state is shown', () => { + createShallowWrapper({ customMetricsAvailable: true }); - describe.each(nonDuplicableCases)( - 'when the selected dashboard cannot be duplicated', - dashboardPath => { - it('contains a "Create New" menu item, but no "Duplicate Dashboard" menu item and modal', () => { - store.state.monitoringDashboard.projectPath = mockProjectPath; - setupAllDashboards(store, dashboardPath); + store.state.monitoringDashboard.emptyState = true; + setupAllDashboards(store, ootbDashboards[0]); - return wrapper.vm.$nextTick().then(() => { - expect(findCreateDashboardMenuItem().exists()).toBe(true); - expect(findCreateDashboardDuplicateItem().exists()).toBe(false); - expect(findDuplicateDashboardModal().exists()).toBe(false); - }); + return wrapper.vm.$nextTick().then(() => { + expect(findActionsMenu().props('addingMetricsAvailable')).toBe(false); }); - }, - ); - }); + }); - describe('actions menu modals', () => { - beforeEach(() => { - store.state.monitoringDashboard.projectPath = mockProjectPath; - store.state.monitoringDashboard.addDashboardDocumentationPath = mockAddDashboardDocPath; - setupAllDashboards(store); + it('gets passed false if custom metrics are not available', () => { + createShallowWrapper({ customMetricsAvailable: false }); - createShallowWrapper(); + store.state.monitoringDashboard.emptyState = false; + setupAllDashboards(store, ootbDashboards[0]); + + return wrapper.vm.$nextTick().then(() => { + expect(findActionsMenu().props('addingMetricsAvailable')).toBe(false); + }); + }); }); - it('Clicking on "Create New" opens up a modal', () => { - const modalId = 'createDashboard'; - const modalTrigger = findCreateDashboardMenuItem(); - const rootEmit = jest.spyOn(wrapper.vm.$root, '$emit'); + it('custom metrics path gets passed', () => { + const path = 'https://path/to/customMetrics'; - modalTrigger.trigger('click'); + createShallowWrapper({ customMetricsPath: path }); return wrapper.vm.$nextTick().then(() => { - expect(rootEmit.mock.calls[0]).toContainEqual(modalId); + expect(findActionsMenu().props('customMetricsPath')).toBe(path); }); }); - it('"Create new dashboard" modal contains correct buttons', () => { - expect(findCreateDashboardModal().props('projectPath')).toBe(mockProjectPath); - expect(findCreateDashboardModal().props('addDashboardDocumentationPath')).toBe( - mockAddDashboardDocPath, - ); + it('validate query path gets passed', () => { + const path = 'https://path/to/validateQuery'; + + createShallowWrapper({ validateQueryPath: path }); + + return wrapper.vm.$nextTick().then(() => { + expect(findActionsMenu().props('validateQueryPath')).toBe(path); + }); }); - it('"Duplicate Dashboard" opens up a modal', () => { - const modalId = 'duplicateDashboard'; - const modalTrigger = findCreateDashboardDuplicateItem(); - const rootEmit = jest.spyOn(wrapper.vm.$root, '$emit'); + it('default branch gets passed', () => { + const branch = 'branchName'; - modalTrigger.trigger('click'); + createShallowWrapper({ defaultBranch: branch }); return wrapper.vm.$nextTick().then(() => { - expect(rootEmit.mock.calls[0]).toContainEqual(modalId); + expect(findActionsMenu().props('defaultBranch')).toBe(branch); }); }); }); @@ -465,72 +416,4 @@ describe('Dashboard header', () => { }); }); }); - - describe('Add metric button', () => { - const findAddMetricButton = () => wrapper.find('[data-qa-selector="add_metric_button"]'); - - it('is not rendered when custom metrics are not available', () => { - store.state.monitoringDashboard.emptyState = false; - - createShallowWrapper({ - customMetricsAvailable: false, - }); - - setupAllDashboards(store, dashboardGitResponse[0].path); - - return wrapper.vm.$nextTick(() => { - expect(findAddMetricButton().exists()).toBe(false); - }); - }); - - it('is not rendered when displaying empty state', () => { - store.state.monitoringDashboard.emptyState = true; - - createShallowWrapper({ - customMetricsAvailable: true, - }); - - setupAllDashboards(store, dashboardGitResponse[0].path); - - return wrapper.vm.$nextTick(() => { - expect(findAddMetricButton().exists()).toBe(false); - }); - }); - - describe('system dashboards', () => { - const systemDashboards = [ - dashboardGitResponse[0].path, - selfMonitoringDashboardGitResponse[0].path, - ]; - const nonSystemDashboards = [ - dashboardGitResponse[1].path, - dashboardGitResponse[2].path, - selfMonitoringDashboardGitResponse[1].path, - ]; - - beforeEach(() => { - store.state.monitoringDashboard.emptyState = false; - - createShallowWrapper({ - customMetricsAvailable: true, - }); - }); - - it.each(systemDashboards)('is rendered for system dashboards', dashboardPath => { - setupAllDashboards(store, dashboardPath); - - return wrapper.vm.$nextTick(() => { - expect(findAddMetricButton().exists()).toBe(true); - }); - }); - - it.each(nonSystemDashboards)('is not rendered for non-system dashboards', dashboardPath => { - setupAllDashboards(store, dashboardPath); - - return wrapper.vm.$nextTick(() => { - expect(findAddMetricButton().exists()).toBe(false); - }); - }); - }); - }); }); diff --git a/spec/frontend/monitoring/components/dashboard_spec.js b/spec/frontend/monitoring/components/dashboard_spec.js index 1f9f6a738f2..4ab3ae3588d 100644 --- a/spec/frontend/monitoring/components/dashboard_spec.js +++ b/spec/frontend/monitoring/components/dashboard_spec.js @@ -1,7 +1,5 @@ import { shallowMount, mount } from '@vue/test-utils'; -import Tracking from '~/tracking'; import { ESC_KEY, ESC_KEY_IE11 } from '~/lib/utils/keys'; -import { GlModal } from '@gitlab/ui'; import { objectToQuery } from '~/lib/utils/url_utility'; import VueDraggable from 'vuedraggable'; import MockAdapter from 'axios-mock-adapter'; @@ -10,7 +8,6 @@ import { dashboardEmptyStates, metricStates } from '~/monitoring/constants'; import Dashboard from '~/monitoring/components/dashboard.vue'; import DashboardHeader from '~/monitoring/components/dashboard_header.vue'; -import CustomMetricsFormFields from '~/custom_metrics/components/custom_metrics_form_fields.vue'; import EmptyState from '~/monitoring/components/empty_state.vue'; import GroupEmptyState from '~/monitoring/components/group_empty_state.vue'; import DashboardPanel from '~/monitoring/components/dashboard_panel.vue'; @@ -42,8 +39,6 @@ describe('Dashboard', () => { let wrapper; let mock; - const findDashboardHeader = () => wrapper.find(DashboardHeader); - const createShallowWrapper = (props = {}, options = {}) => { wrapper = shallowMount(Dashboard, { propsData: { ...dashboardProps, ...props }, @@ -446,84 +441,6 @@ describe('Dashboard', () => { }); }); - describe('star dashboards', () => { - const findToggleStar = () => findDashboardHeader().find({ ref: 'toggleStarBtn' }); - - beforeEach(() => { - createShallowWrapper(); - setupAllDashboards(store); - }); - - it('toggle star button is shown', () => { - expect(findToggleStar().exists()).toBe(true); - expect(findToggleStar().props('disabled')).toBe(false); - }); - - it('toggle star button is disabled when starring is taking place', () => { - store.commit(`monitoringDashboard/${types.REQUEST_DASHBOARD_STARRING}`); - - return wrapper.vm.$nextTick(() => { - expect(findToggleStar().exists()).toBe(true); - expect(findToggleStar().props('disabled')).toBe(true); - }); - }); - - describe('when the dashboard list is loaded', () => { - // Tooltip element should wrap directly - const getToggleTooltip = () => findToggleStar().element.parentElement.getAttribute('title'); - - beforeEach(() => { - setupAllDashboards(store); - jest.spyOn(store, 'dispatch'); - }); - - it('dispatches a toggle star action', () => { - findToggleStar().vm.$emit('click'); - - return wrapper.vm.$nextTick().then(() => { - expect(store.dispatch).toHaveBeenCalledWith( - 'monitoringDashboard/toggleStarredValue', - undefined, - ); - }); - }); - - describe('when dashboard is not starred', () => { - beforeEach(() => { - store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, { - currentDashboard: dashboardGitResponse[0].path, - }); - return wrapper.vm.$nextTick(); - }); - - it('toggle star button shows "Star dashboard"', () => { - expect(getToggleTooltip()).toBe('Star dashboard'); - }); - - it('toggle star button shows an unstarred state', () => { - expect(findToggleStar().attributes('icon')).toBe('star-o'); - }); - }); - - describe('when dashboard is starred', () => { - beforeEach(() => { - store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, { - currentDashboard: dashboardGitResponse[1].path, - }); - return wrapper.vm.$nextTick(); - }); - - it('toggle star button shows "Star dashboard"', () => { - expect(getToggleTooltip()).toBe('Unstar dashboard'); - }); - - it('toggle star button shows a starred state', () => { - expect(findToggleStar().attributes('icon')).toBe('star'); - }); - }); - }); - }); - describe('variables section', () => { beforeEach(() => { createShallowWrapper({ hasMetrics: true }); @@ -800,33 +717,6 @@ describe('Dashboard', () => { }); }); - describe('dashboard edit link', () => { - const findEditLink = () => wrapper.find('.js-edit-link'); - - beforeEach(() => { - createShallowWrapper({ hasMetrics: true }); - - setupAllDashboards(store); - return wrapper.vm.$nextTick(); - }); - - it('is not present for the overview dashboard', () => { - expect(findEditLink().exists()).toBe(false); - }); - - it('is present for a custom dashboard, and links to its edit_path', () => { - const dashboard = dashboardGitResponse[1]; - store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, { - currentDashboard: dashboard.path, - }); - - return wrapper.vm.$nextTick().then(() => { - expect(findEditLink().exists()).toBe(true); - expect(findEditLink().attributes('href')).toBe(dashboard.project_blob_path); - }); - }); - }); - describe('document title', () => { const originalTitle = 'Original Title'; const overviewDashboardName = dashboardGitResponse[0].display_name; @@ -940,74 +830,4 @@ describe('Dashboard', () => { expect(dashboardPanel.exists()).toBe(true); }); }); - - describe('add custom metrics', () => { - const findAddMetricButton = () => findDashboardHeader().find({ ref: 'addMetricBtn' }); - - describe('when not available', () => { - beforeEach(() => { - createShallowWrapper({ - hasMetrics: true, - customMetricsPath: '/endpoint', - }); - }); - it('does not render add button on the dashboard', () => { - expect(findAddMetricButton().exists()).toBe(false); - }); - }); - - describe('when available', () => { - let origPage; - beforeEach(done => { - jest.spyOn(Tracking, 'event').mockReturnValue(); - createShallowWrapper({ - hasMetrics: true, - customMetricsPath: '/endpoint', - customMetricsAvailable: true, - }); - setupStoreWithData(store); - - origPage = document.body.dataset.page; - document.body.dataset.page = 'projects:environments:metrics'; - - wrapper.vm.$nextTick(done); - }); - afterEach(() => { - document.body.dataset.page = origPage; - }); - - it('renders add button on the dashboard', () => { - expect(findAddMetricButton()).toBeDefined(); - }); - - it('uses modal for custom metrics form', () => { - expect(wrapper.find(GlModal).exists()).toBe(true); - expect(wrapper.find(GlModal).attributes().modalid).toBe('addMetric'); - }); - it('adding new metric is tracked', done => { - const submitButton = wrapper - .find(DashboardHeader) - .find({ ref: 'submitCustomMetricsFormBtn' }).vm; - wrapper.vm.$nextTick(() => { - submitButton.$el.click(); - wrapper.vm.$nextTick(() => { - expect(Tracking.event).toHaveBeenCalledWith( - document.body.dataset.page, - 'click_button', - { - label: 'add_new_metric', - property: 'modal', - value: undefined, - }, - ); - done(); - }); - }); - }); - - it('renders custom metrics form fields', () => { - expect(wrapper.find(CustomMetricsFormFields).exists()).toBe(true); - }); - }); - }); }); diff --git a/spec/frontend/monitoring/mock_data.js b/spec/frontend/monitoring/mock_data.js index a22f35b4315..17b1df30269 100644 --- a/spec/frontend/monitoring/mock_data.js +++ b/spec/frontend/monitoring/mock_data.js @@ -622,3 +622,10 @@ export const dashboardHeaderProps = { end: '2020-01-01T01:00:00.000Z', }, }; + +export const dashboardActionsMenuProps = { + defaultBranch: 'master', + addingMetricsAvailable: true, + customMetricsPath: 'https://path/to/customMetrics', + validateQueryPath: 'https://path/to/validateQuery', +}; diff --git a/spec/frontend/packages/details/components/composer_installation_spec.js b/spec/frontend/packages/details/components/composer_installation_spec.js new file mode 100644 index 00000000000..767945d416d --- /dev/null +++ b/spec/frontend/packages/details/components/composer_installation_spec.js @@ -0,0 +1,95 @@ +import Vuex from 'vuex'; +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import { GlSprintf, GlLink } from '@gitlab/ui'; +import ComposerInstallation from '~/packages/details/components/composer_installation.vue'; +import CodeInstructions from '~/packages/details/components/code_instruction.vue'; +import { TrackingActions } from '~/packages/details/constants'; +import { registryUrl as composerHelpPath } from 'jest/packages/details/mock_data'; +import { composerPackage as packageEntity } from 'jest/packages/mock_data'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('ComposerInstallation', () => { + let wrapper; + + const composerRegistryIncludeStr = 'foo/registry'; + const composerPackageIncludeStr = 'foo/package'; + + const store = new Vuex.Store({ + state: { + packageEntity, + composerHelpPath, + }, + getters: { + composerRegistryInclude: () => composerRegistryIncludeStr, + composerPackageInclude: () => composerPackageIncludeStr, + }, + }); + + const findCodeInstructions = () => wrapper.findAll(CodeInstructions); + const findRegistryIncludeTitle = () => wrapper.find('[data-testid="registry-include-title"]'); + const findPackageIncludeTitle = () => wrapper.find('[data-testid="package-include-title"]'); + const findHelpText = () => wrapper.find('[data-testid="help-text"]'); + const findHelpLink = () => wrapper.find(GlLink); + + function createComponent() { + wrapper = shallowMount(ComposerInstallation, { + localVue, + store, + stubs: { + GlSprintf, + }, + }); + } + + beforeEach(() => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + describe('registry include command', () => { + it('uses code_instructions', () => { + const registryIncludeCommand = findCodeInstructions().at(0); + expect(registryIncludeCommand.exists()).toBe(true); + expect(registryIncludeCommand.props()).toMatchObject({ + instruction: composerRegistryIncludeStr, + copyText: 'Copy registry include', + trackingAction: TrackingActions.COPY_COMPOSER_REGISTRY_INCLUDE_COMMAND, + }); + }); + + it('has the correct title', () => { + expect(findRegistryIncludeTitle().text()).toBe('composer.json registry include'); + }); + }); + + describe('package include command', () => { + it('uses code_instructions', () => { + const registryIncludeCommand = findCodeInstructions().at(1); + expect(registryIncludeCommand.exists()).toBe(true); + expect(registryIncludeCommand.props()).toMatchObject({ + instruction: composerPackageIncludeStr, + copyText: 'Copy require package include', + trackingAction: TrackingActions.COPY_COMPOSER_PACKAGE_INCLUDE_COMMAND, + }); + }); + + it('has the correct title', () => { + expect(findPackageIncludeTitle().text()).toBe('composer.json require package include'); + }); + + it('has the correct help text', () => { + expect(findHelpText().text()).toBe( + 'For more information on Composer packages in GitLab, see the documentation.', + ); + expect(findHelpLink().attributes()).toMatchObject({ + href: composerHelpPath, + target: '_blank', + }); + }); + }); +}); diff --git a/spec/frontend/packages/details/components/installations_commands_spec.js b/spec/frontend/packages/details/components/installations_commands_spec.js index 65904749e2c..60da34ebcd9 100644 --- a/spec/frontend/packages/details/components/installations_commands_spec.js +++ b/spec/frontend/packages/details/components/installations_commands_spec.js @@ -6,8 +6,16 @@ import MavenInstallation from '~/packages/details/components/maven_installation. import ConanInstallation from '~/packages/details/components/conan_installation.vue'; import NugetInstallation from '~/packages/details/components/nuget_installation.vue'; import PypiInstallation from '~/packages/details/components/pypi_installation.vue'; +import ComposerInstallation from '~/packages/details/components/composer_installation.vue'; -import { conanPackage, mavenPackage, npmPackage, nugetPackage, pypiPackage } from '../../mock_data'; +import { + conanPackage, + mavenPackage, + npmPackage, + nugetPackage, + pypiPackage, + composerPackage, +} from '../../mock_data'; describe('InstallationCommands', () => { let wrapper; @@ -23,6 +31,7 @@ describe('InstallationCommands', () => { const conanInstallation = () => wrapper.find(ConanInstallation); const nugetInstallation = () => wrapper.find(NugetInstallation); const pypiInstallation = () => wrapper.find(PypiInstallation); + const composerInstallation = () => wrapper.find(ComposerInstallation); afterEach(() => { wrapper.destroy(); @@ -30,12 +39,13 @@ describe('InstallationCommands', () => { describe('installation instructions', () => { describe.each` - packageEntity | selector - ${conanPackage} | ${conanInstallation} - ${mavenPackage} | ${mavenInstallation} - ${npmPackage} | ${npmInstallation} - ${nugetPackage} | ${nugetInstallation} - ${pypiPackage} | ${pypiInstallation} + packageEntity | selector + ${conanPackage} | ${conanInstallation} + ${mavenPackage} | ${mavenInstallation} + ${npmPackage} | ${npmInstallation} + ${nugetPackage} | ${nugetInstallation} + ${pypiPackage} | ${pypiInstallation} + ${composerPackage} | ${composerInstallation} `('renders', ({ packageEntity, selector }) => { it(`${packageEntity.package_type} instructions exist`, () => { createComponent({ packageEntity }); diff --git a/spec/helpers/packages_helper_spec.rb b/spec/helpers/packages_helper_spec.rb index 6dc6f8b2299..1917c851547 100644 --- a/spec/helpers/packages_helper_spec.rb +++ b/spec/helpers/packages_helper_spec.rb @@ -44,6 +44,14 @@ RSpec.describe PackagesHelper do end end + describe 'composer_registry_url' do + it 'return the composer registry url' do + url = helper.composer_registry_url(1) + + expect(url).to eq("#{base_url}group/1/-/packages/composer/packages.json") + end + end + describe 'packages_coming_soon_enabled?' do it 'returns false when the feature flag is disabled' do stub_feature_flags(packages_coming_soon: false) diff --git a/spec/policies/personal_access_token_policy_spec.rb b/spec/policies/personal_access_token_policy_spec.rb index 706150597b2..71795202e13 100644 --- a/spec/policies/personal_access_token_policy_spec.rb +++ b/spec/policies/personal_access_token_policy_spec.rb @@ -5,38 +5,59 @@ require 'spec_helper' RSpec.describe PersonalAccessTokenPolicy do include AdminModeHelper - using RSpec::Parameterized::TableSyntax + subject { described_class.new(current_user, token) } - where(:user_type, :owned_by_same_user, :expected_permitted?) do - :user | true | true - :user | false | false - :admin | false | true + context 'current_user is an administrator', :enable_admin_mode do + let_it_be(:current_user) { build(:admin) } + + context 'not the owner of the token' do + let_it_be(:token) { build(:personal_access_token) } + + it { is_expected.to be_allowed(:read_token) } + it { is_expected.to be_allowed(:revoke_token) } + end + + context 'owner of the token' do + let_it_be(:token) { build(:personal_access_token, user: current_user) } + + it { is_expected.to be_allowed(:read_token) } + it { is_expected.to be_allowed(:revoke_token) } + end end - with_them do - context 'determine if a token is readable or revocable by a user' do - let(:user) { build_stubbed(user_type) } - let(:token_owner) { owned_by_same_user ? user : build(:user) } - let(:token) { build(:personal_access_token, user: token_owner) } + context 'current_user is not an administrator' do + let_it_be(:current_user) { build(:user) } - subject { described_class.new(user, token) } + context 'not the owner of the token' do + let_it_be(:token) { build(:personal_access_token) } - before do - enable_admin_mode!(user) if user.admin? - end + it { is_expected.to be_disallowed(:read_token) } + it { is_expected.to be_disallowed(:revoke_token) } + end + + context 'owner of the token' do + let_it_be(:token) { build(:personal_access_token, user: current_user) } - it { is_expected.to(expected_permitted? ? be_allowed(:read_token) : be_disallowed(:read_token)) } - it { is_expected.to(expected_permitted? ? be_allowed(:revoke_token) : be_disallowed(:revoke_token)) } + it { is_expected.to be_allowed(:read_token) } + it { is_expected.to be_allowed(:revoke_token) } end end context 'current_user is a blocked administrator', :enable_admin_mode do - subject { described_class.new(current_user, token) } + let_it_be(:current_user) { build(:admin, :blocked) } + + context 'owner of the token' do + let_it_be(:token) { build(:personal_access_token, user: current_user) } - let(:current_user) { create(:user, :admin, :blocked) } - let(:token) { create(:personal_access_token) } + it { is_expected.to be_disallowed(:read_token) } + it { is_expected.to be_disallowed(:revoke_token) } + end + + context 'not the owner of the token' do + let_it_be(:token) { build(:personal_access_token) } - it { is_expected.to be_disallowed(:revoke_token) } - it { is_expected.to be_disallowed(:read_token) } + it { is_expected.to be_disallowed(:read_token) } + it { is_expected.to be_disallowed(:revoke_token) } + end end end diff --git a/spec/requests/api/import_github_spec.rb b/spec/requests/api/import_github_spec.rb index f026314f7a8..bbfb17fe753 100644 --- a/spec/requests/api/import_github_spec.rb +++ b/spec/requests/api/import_github_spec.rb @@ -22,7 +22,7 @@ RSpec.describe API::ImportGithub do before do Grape::Endpoint.before_each do |endpoint| - allow(endpoint).to receive(:client).and_return(double('client', user: provider_user, repo: provider_repo).as_null_object) + allow(endpoint).to receive(:client).and_return(double('client', user: provider_user, repository: provider_repo).as_null_object) end end diff --git a/spec/services/import/github_service_spec.rb b/spec/services/import/github_service_spec.rb index 266ff309662..408d7767254 100644 --- a/spec/services/import/github_service_spec.rb +++ b/spec/services/import/github_service_spec.rb @@ -6,7 +6,6 @@ RSpec.describe Import::GithubService do let_it_be(:user) { create(:user) } let_it_be(:token) { 'complex-token' } let_it_be(:access_params) { { github_access_token: 'github-complex-token' } } - let_it_be(:client) { Gitlab::LegacyGithubImport::Client.new(token) } let_it_be(:params) { { repo_id: 123, new_name: 'new_repo', target_namespace: 'root' } } let(:subject) { described_class.new(client, user, params) } @@ -15,41 +14,61 @@ RSpec.describe Import::GithubService do allow(subject).to receive(:authorized?).and_return(true) end - context 'do not raise an exception on input error' do - let(:exception) { Octokit::ClientError.new(status: 404, body: 'Not Found') } + shared_examples 'handles errors' do |klass| + let(:client) { klass.new(token) } - before do - expect(client).to receive(:repo).and_raise(exception) - end + context 'do not raise an exception on input error' do + let(:exception) { Octokit::ClientError.new(status: 404, body: 'Not Found') } + + before do + expect(client).to receive(:repository).and_raise(exception) + end - it 'logs the original error' do - expect(Gitlab::Import::Logger).to receive(:error).with({ - message: 'Import failed due to a GitHub error', - status: 404, - error: 'Not Found' - }).and_call_original + it 'logs the original error' do + expect(Gitlab::Import::Logger).to receive(:error).with({ + message: 'Import failed due to a GitHub error', + status: 404, + error: 'Not Found' + }).and_call_original - subject.execute(access_params, :github) + subject.execute(access_params, :github) + end + + it 'returns an error' do + result = subject.execute(access_params, :github) + + expect(result).to include( + message: 'Import failed due to a GitHub error: Not Found', + status: :error, + http_status: :unprocessable_entity + ) + end end - it 'returns an error' do - result = subject.execute(access_params, :github) + it 'raises an exception for unknown error causes' do + exception = StandardError.new('Not Implemented') + + expect(client).to receive(:repository).and_raise(exception) - expect(result).to include( - message: 'Import failed due to a GitHub error: Not Found', - status: :error, - http_status: :unprocessable_entity - ) + expect(Gitlab::Import::Logger).not_to receive(:error) + + expect { subject.execute(access_params, :github) }.to raise_error(exception) end end - it 'raises an exception for unknown error causes' do - exception = StandardError.new('Not Implemented') + context 'when remove_legacy_github_client feature flag is enabled' do + before do + stub_feature_flags(remove_legacy_github_client: true) + end - expect(client).to receive(:repo).and_raise(exception) + include_examples 'handles errors', Gitlab::GithubImport::Client + end - expect(Gitlab::Import::Logger).not_to receive(:error) + context 'when remove_legacy_github_client feature flag is enabled' do + before do + stub_feature_flags(remove_legacy_github_client: false) + end - expect { subject.execute(access_params, :github) }.to raise_error(exception) + include_examples 'handles errors', Gitlab::LegacyGithubImport::Client end end diff --git a/spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb b/spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb index a01fa49d701..8bc91f72b8c 100644 --- a/spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb +++ b/spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb @@ -72,7 +72,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: GET status' do project = create(:project, import_type: provider, namespace: user.namespace, import_status: :finished, import_source: 'example/repo') group = create(:group) group.add_owner(user) - stub_client(repos: [repo, org_repo], orgs: [org], org_repos: [org_repo]) + stub_client(repos: [repo, org_repo], orgs: [org], org_repos: [org_repo], each_page: [OpenStruct.new(objects: [repo, org_repo])].to_enum) get :status, format: :json @@ -85,7 +85,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: GET status' do it "does not show already added project" do project = create(:project, import_type: provider, namespace: user.namespace, import_status: :finished, import_source: 'asd/vim') - stub_client(repos: [repo], orgs: []) + stub_client(repos: [repo], orgs: [], each_page: [OpenStruct.new(objects: [repo])].to_enum) get :status, format: :json @@ -94,7 +94,8 @@ RSpec.shared_examples 'a GitHub-ish import controller: GET status' do end it "touches the etag cache store" do - expect(stub_client(repos: [], orgs: [])).to receive(:repos) + stub_client(repos: [], orgs: [], each_page: []) + expect_next_instance_of(Gitlab::EtagCaching::Store) do |store| expect(store).to receive(:touch) { "realtime_changes_import_#{provider}_path" } end @@ -102,17 +103,11 @@ RSpec.shared_examples 'a GitHub-ish import controller: GET status' do get :status, format: :json end - it "requests provider repos list" do - expect(stub_client(repos: [], orgs: [])).to receive(:repos) - - get :status - - expect(response).to have_gitlab_http_status(:ok) - end - it "handles an invalid access token" do - allow_any_instance_of(Gitlab::LegacyGithubImport::Client) - .to receive(:repos).and_raise(Octokit::Unauthorized) + client = stub_client(repos: [], orgs: [], each_page: []) + + allow(client).to receive(:repos).and_raise(Octokit::Unauthorized) + allow(client).to receive(:each_page).and_raise(Octokit::Unauthorized) get :status @@ -122,7 +117,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: GET status' do end it "does not produce N+1 database queries" do - stub_client(repos: [repo], orgs: []) + stub_client(repos: [repo], orgs: [], each_page: [].to_enum) group_a = create(:group) group_a.add_owner(user) create(:project, :import_started, import_type: provider, namespace: user.namespace) @@ -144,10 +139,12 @@ RSpec.shared_examples 'a GitHub-ish import controller: GET status' do let(:repo_2) { OpenStruct.new(login: 'emacs', full_name: 'asd/emacs', name: 'emacs', owner: { login: 'owner' }) } let(:project) { create(:project, import_type: provider, namespace: user.namespace, import_status: :finished, import_source: 'example/repo') } let(:group) { create(:group) } + let(:repos) { [repo, repo_2, org_repo] } before do group.add_owner(user) - stub_client(repos: [repo, repo_2, org_repo], orgs: [org], org_repos: [org_repo]) + client = stub_client(repos: repos, orgs: [org], org_repos: [org_repo]) + allow(client).to receive(:each_page).and_return([OpenStruct.new(objects: repos)].to_enum) end it 'filters list of repositories by name' do @@ -187,14 +184,14 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do end before do - stub_client(user: provider_user, repo: provider_repo) + stub_client(user: provider_user, repo: provider_repo, repository: provider_repo) assign_session_token(provider) end it 'returns 200 response when the project is imported successfully' do allow(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, format: :json @@ -208,7 +205,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do allow(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, format: :json @@ -219,7 +216,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it "touches the etag cache store" do allow(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) expect_next_instance_of(Gitlab::EtagCaching::Store) do |store| expect(store).to receive(:touch) { "realtime_changes_import_#{provider}_path" } end @@ -232,7 +229,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it "takes the current user's namespace" do expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, format: :json end @@ -244,7 +241,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it "takes the current user's namespace" do expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, format: :json end @@ -271,7 +268,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it "takes the existing namespace" do expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, existing_namespace, user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, format: :json end @@ -283,7 +280,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, format: :json end @@ -302,7 +299,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it "takes the new namespace" do expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, an_instance_of(Group), user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, params: { target_namespace: provider_repo.name }, format: :json end @@ -323,7 +320,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it "takes the current user's namespace" do expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, format: :json end @@ -341,7 +338,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it 'takes the selected namespace and name' do expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, test_namespace, user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, params: { target_namespace: test_namespace.name, new_name: test_name }, format: :json end @@ -349,7 +346,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it 'takes the selected name and default namespace' do expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, user.namespace, user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, params: { new_name: test_name }, format: :json end @@ -368,7 +365,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it 'takes the selected namespace and name' do expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, nested_namespace, user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, params: { target_namespace: nested_namespace.full_path, new_name: test_name }, format: :json end @@ -380,7 +377,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it 'takes the selected namespace and name' do expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, params: { target_namespace: 'foo/bar', new_name: test_name }, format: :json end @@ -388,7 +385,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it 'creates the namespaces' do allow(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) expect { post :create, params: { target_namespace: 'foo/bar', new_name: test_name }, format: :json } .to change { Namespace.count }.by(2) @@ -397,7 +394,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it 'new namespace has the right parent' do allow(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, params: { target_namespace: 'foo/bar', new_name: test_name }, format: :json @@ -416,7 +413,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it 'takes the selected namespace and name' do expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post :create, params: { target_namespace: 'foo/foobar/bar', new_name: test_name }, format: :json end @@ -424,7 +421,7 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it 'creates the namespaces' do allow(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) - .and_return(double(execute: project)) + .and_return(double(execute: project)) expect { post :create, params: { target_namespace: 'foo/foobar/bar', new_name: test_name }, format: :json } .to change { Namespace.count }.by(2) @@ -432,11 +429,11 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it 'does not create a new namespace under the user namespace' do expect(Gitlab::LegacyGithubImport::ProjectCreator) - .to receive(:new).with(provider_repo, test_name, user.namespace, user, access_params, type: provider) - .and_return(double(execute: project)) + .to receive(:new).with(provider_repo, test_name, user.namespace, user, access_params, type: provider) + .and_return(double(execute: project)) expect { post :create, params: { target_namespace: "#{user.namespace_path}/test_group", new_name: test_name }, format: :js } - .not_to change { Namespace.count } + .not_to change { Namespace.count } end end @@ -446,19 +443,19 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do it 'does not take the selected namespace and name' do expect(Gitlab::LegacyGithubImport::ProjectCreator) - .to receive(:new).with(provider_repo, test_name, user.namespace, user, access_params, type: provider) - .and_return(double(execute: project)) + .to receive(:new).with(provider_repo, test_name, user.namespace, user, access_params, type: provider) + .and_return(double(execute: project)) post :create, params: { target_namespace: 'foo/foobar/bar', new_name: test_name }, format: :js end it 'does not create the namespaces' do allow(Gitlab::LegacyGithubImport::ProjectCreator) - .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) - .and_return(double(execute: project)) + .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) + .and_return(double(execute: project)) expect { post :create, params: { target_namespace: 'foo/foobar/bar', new_name: test_name }, format: :js } - .not_to change { Namespace.count } + .not_to change { Namespace.count } end end @@ -471,8 +468,8 @@ RSpec.shared_examples 'a GitHub-ish import controller: POST create' do user.update!(can_create_group: false) expect(Gitlab::LegacyGithubImport::ProjectCreator) - .to receive(:new).with(provider_repo, test_name, group, user, access_params, type: provider) - .and_return(double(execute: project)) + .to receive(:new).with(provider_repo, test_name, group, user, access_params, type: provider) + .and_return(double(execute: project)) post :create, params: { target_namespace: 'foo', new_name: test_name }, format: :js end |
