diff options
36 files changed, 1742 insertions, 86 deletions
diff --git a/app/assets/javascripts/error_tracking_settings/components/app.vue b/app/assets/javascripts/error_tracking_settings/components/app.vue new file mode 100644 index 00000000000..50eb3e63b7c --- /dev/null +++ b/app/assets/javascripts/error_tracking_settings/components/app.vue @@ -0,0 +1,129 @@ +<script> +import { mapActions, mapGetters, mapState } from 'vuex'; +import { GlButton } from '@gitlab/ui'; +import ProjectDropdown from './project_dropdown.vue'; +import ErrorTrackingForm from './error_tracking_form.vue'; + +export default { + components: { ProjectDropdown, ErrorTrackingForm, GlButton }, + props: { + initialApiHost: { + type: String, + required: false, + default: '', + }, + initialEnabled: { + type: String, + required: true, + }, + initialProject: { + type: String, + required: false, + default: null, + }, + initialToken: { + type: String, + required: false, + default: '', + }, + listProjectsEndpoint: { + type: String, + required: true, + }, + operationsSettingsEndpoint: { + type: String, + required: true, + }, + }, + computed: { + ...mapGetters([ + 'dropdownLabel', + 'hasProjects', + 'invalidProjectLabel', + 'isProjectInvalid', + 'projectSelectionLabel', + ]), + ...mapState([ + 'apiHost', + 'connectError', + 'connectSuccessful', + 'enabled', + 'projects', + 'selectedProject', + 'settingsLoading', + 'token', + ]), + }, + created() { + this.setInitialState({ + apiHost: this.initialApiHost, + enabled: this.initialEnabled, + project: this.initialProject, + token: this.initialToken, + listProjectsEndpoint: this.listProjectsEndpoint, + operationsSettingsEndpoint: this.operationsSettingsEndpoint, + }); + }, + methods: { + ...mapActions([ + 'fetchProjects', + 'setInitialState', + 'updateApiHost', + 'updateEnabled', + 'updateSelectedProject', + 'updateSettings', + 'updateToken', + ]), + handleSubmit() { + this.updateSettings(); + }, + }, +}; +</script> + +<template> + <div> + <div class="form-check form-group"> + <input + id="error-tracking-enabled" + :checked="enabled" + class="form-check-input" + type="checkbox" + @change="updateEnabled($event.target.checked)" + /> + <label class="form-check-label" for="error-tracking-enabled">{{ + s__('ErrorTracking|Active') + }}</label> + </div> + <error-tracking-form + :api-host="apiHost" + :connect-error="connectError" + :connect-successful="connectSuccessful" + :token="token" + @handle-connect="fetchProjects" + @update-api-host="updateApiHost" + @update-token="updateToken" + /> + <div class="form-group"> + <project-dropdown + :has-projects="hasProjects" + :invalid-project-label="invalidProjectLabel" + :is-project-invalid="isProjectInvalid" + :dropdown-label="dropdownLabel" + :project-selection-label="projectSelectionLabel" + :projects="projects" + :selected-project="selectedProject" + :token="token" + @select-project="updateSelectedProject" + /> + </div> + <gl-button + :disabled="settingsLoading" + class="js-error-tracking-button" + variant="success" + @click="handleSubmit" + > + {{ __('Save changes') }} + </gl-button> + </div> +</template> diff --git a/app/assets/javascripts/error_tracking_settings/components/error_tracking_form.vue b/app/assets/javascripts/error_tracking_settings/components/error_tracking_form.vue new file mode 100644 index 00000000000..060d8e25227 --- /dev/null +++ b/app/assets/javascripts/error_tracking_settings/components/error_tracking_form.vue @@ -0,0 +1,91 @@ +<script> +import { GlButton, GlFormInput } from '@gitlab/ui'; +import Icon from '~/vue_shared/components/icon.vue'; + +export default { + components: { GlButton, GlFormInput, Icon }, + props: { + apiHost: { + type: String, + required: true, + }, + connectError: { + type: Boolean, + required: true, + }, + connectSuccessful: { + type: Boolean, + required: true, + }, + token: { + type: String, + required: true, + }, + }, + computed: { + tokenInputState() { + return this.connectError ? false : null; + }, + }, +}; +</script> + +<template> + <div> + <div class="form-group"> + <label class="label-bold" for="error-tracking-api-host">{{ __('Sentry API URL') }}</label> + <div class="row"> + <div class="col-8 col-md-9 gl-pr-0"> + <gl-form-input + id="error-tracking-api-host" + :value="apiHost" + placeholder="https://mysentryserver.com" + @input="$emit('update-api-host', $event)" + /> + </div> + </div> + <p class="form-text text-muted"> + {{ s__('ErrorTracking|Find your hostname in your Sentry account settings page') }} + </p> + </div> + <div class="form-group" :class="{ 'gl-show-field-errors': connectError }"> + <label class="label-bold" for="error-tracking-token">{{ + s__('ErrorTracking|Auth Token') + }}</label> + <div class="row"> + <div class="col-8 col-md-9 gl-pr-0"> + <gl-form-input + id="error-tracking-token" + :value="token" + :state="tokenInputState" + @input="$emit('update-token', $event)" + /> + </div> + <div class="col-4 col-md-3 gl-pl-0"> + <gl-button + class="js-error-tracking-connect prepend-left-5" + @click="$emit('handle-connect')" + > + {{ __('Connect') }} + </gl-button> + <icon + v-show="connectSuccessful" + class="js-error-tracking-connect-success prepend-left-5 text-success align-middle" + :aria-label="__('Projects Successfully Retrieved')" + name="check-circle" + /> + </div> + </div> + <p v-if="connectError" class="gl-field-error"> + {{ s__('ErrorTracking|Connection has failed. Re-check Auth Token and try again.') }} + </p> + <p v-else class="form-text text-muted"> + {{ + s__( + "ErrorTracking|After adding your Auth Token, use the 'Connect' button to load projects", + ) + }} + </p> + </div> + </div> +</template> diff --git a/app/assets/javascripts/error_tracking_settings/components/project_dropdown.vue b/app/assets/javascripts/error_tracking_settings/components/project_dropdown.vue new file mode 100644 index 00000000000..82df02afafd --- /dev/null +++ b/app/assets/javascripts/error_tracking_settings/components/project_dropdown.vue @@ -0,0 +1,82 @@ +<script> +import { GlDropdown, GlDropdownHeader, GlDropdownItem } from '@gitlab/ui'; +import Icon from '~/vue_shared/components/icon.vue'; +import { getDisplayName } from '../utils'; + +export default { + components: { + GlDropdown, + GlDropdownHeader, + GlDropdownItem, + Icon, + }, + props: { + dropdownLabel: { + type: String, + required: true, + }, + hasProjects: { + type: Boolean, + required: true, + }, + invalidProjectLabel: { + type: String, + required: true, + }, + isProjectInvalid: { + type: Boolean, + required: true, + }, + projects: { + type: Array, + required: true, + }, + selectedProject: { + type: Object, + required: false, + default: null, + }, + projectSelectionLabel: { + type: String, + required: true, + }, + token: { + type: String, + required: true, + }, + }, + methods: { + getDisplayName, + }, +}; +</script> + +<template> + <div :class="{ 'gl-show-field-errors': isProjectInvalid }"> + <label class="label-bold" for="project-dropdown">{{ __('Project') }}</label> + <div class="row"> + <gl-dropdown + id="project-dropdown" + class="col-8 col-md-9 gl-pr-0" + :disabled="!hasProjects" + menu-class="w-100 mw-100" + toggle-class="dropdown-menu-toggle w-100 gl-field-error-outline" + :text="dropdownLabel" + > + <gl-dropdown-item + v-for="project in projects" + :key="`${project.organizationSlug}.${project.slug}`" + class="w-100" + @click="$emit('select-project', project)" + >{{ getDisplayName(project) }}</gl-dropdown-item + > + </gl-dropdown> + </div> + <p v-if="isProjectInvalid" class="js-project-dropdown-error gl-field-error"> + {{ invalidProjectLabel }} + </p> + <p v-else-if="!hasProjects" class="js-project-dropdown-label form-text text-muted"> + {{ projectSelectionLabel }} + </p> + </div> +</template> diff --git a/app/assets/javascripts/error_tracking_settings/index.js b/app/assets/javascripts/error_tracking_settings/index.js new file mode 100644 index 00000000000..ce315963723 --- /dev/null +++ b/app/assets/javascripts/error_tracking_settings/index.js @@ -0,0 +1,27 @@ +import Vue from 'vue'; +import ErrorTrackingSettings from './components/app.vue'; +import createStore from './store'; + +export default () => { + const formContainerEl = document.querySelector('.js-error-tracking-form'); + const { + dataset: { apiHost, enabled, project, token, listProjectsEndpoint, operationsSettingsEndpoint }, + } = formContainerEl; + + return new Vue({ + el: formContainerEl, + store: createStore(), + render(createElement) { + return createElement(ErrorTrackingSettings, { + props: { + initialApiHost: apiHost, + initialEnabled: enabled, + initialProject: project, + initialToken: token, + listProjectsEndpoint, + operationsSettingsEndpoint, + }, + }); + }, + }); +}; diff --git a/app/assets/javascripts/error_tracking_settings/store/actions.js b/app/assets/javascripts/error_tracking_settings/store/actions.js new file mode 100644 index 00000000000..95105797807 --- /dev/null +++ b/app/assets/javascripts/error_tracking_settings/store/actions.js @@ -0,0 +1,91 @@ +import { __ } from '~/locale'; +import axios from '~/lib/utils/axios_utils'; +import { refreshCurrentPage } from '~/lib/utils/url_utility'; +import createFlash from '~/flash'; +import { transformFrontendSettings } from '../utils'; +import * as types from './mutation_types'; + +export const requestProjects = ({ commit }) => { + commit(types.RESET_CONNECT); +}; + +export const receiveProjectsSuccess = ({ commit }, projects) => { + commit(types.UPDATE_CONNECT_SUCCESS); + commit(types.RECEIVE_PROJECTS, projects); +}; + +export const receiveProjectsError = ({ commit }) => { + commit(types.UPDATE_CONNECT_ERROR); + commit(types.CLEAR_PROJECTS); +}; + +export const fetchProjects = ({ dispatch, state }) => { + dispatch('requestProjects'); + return axios + .post(state.listProjectsEndpoint, { + error_tracking_setting: { + api_host: state.apiHost, + token: state.token, + }, + }) + .then(({ data: { projects } }) => { + dispatch('receiveProjectsSuccess', projects); + }) + .catch(() => { + dispatch('receiveProjectsError'); + }); +}; + +export const requestSettings = ({ commit }) => { + commit(types.UPDATE_SETTINGS_LOADING, true); +}; + +export const receiveSettingsError = ({ commit }, { response = {} }) => { + const message = response.data && response.data.message ? response.data.message : ''; + + createFlash(`${__('There was an error saving your changes.')} ${message}`, 'alert'); + commit(types.UPDATE_SETTINGS_LOADING, false); +}; + +export const updateSettings = ({ dispatch, state }) => { + dispatch('requestSettings'); + return axios + .patch(state.operationsSettingsEndpoint, { + project: { + error_tracking_setting_attributes: { + ...transformFrontendSettings(state), + }, + }, + }) + .then(() => { + refreshCurrentPage(); + }) + .catch(err => { + dispatch('receiveSettingsError', err); + }); +}; + +export const updateApiHost = ({ commit }, apiHost) => { + commit(types.UPDATE_API_HOST, apiHost); + commit(types.RESET_CONNECT); +}; + +export const updateEnabled = ({ commit }, enabled) => { + commit(types.UPDATE_ENABLED, enabled); +}; + +export const updateToken = ({ commit }, token) => { + commit(types.UPDATE_TOKEN, token); + commit(types.RESET_CONNECT); +}; + +export const updateSelectedProject = ({ commit }, selectedProject) => { + commit(types.UPDATE_SELECTED_PROJECT, selectedProject); +}; + +export const setInitialState = ({ commit }, data) => { + commit(types.SET_INITIAL_STATE, data); +}; + +// prevent babel-plugin-rewire from generating an invalid default during karma tests +export default () => {}; diff --git a/app/assets/javascripts/error_tracking_settings/store/getters.js b/app/assets/javascripts/error_tracking_settings/store/getters.js new file mode 100644 index 00000000000..a008b181907 --- /dev/null +++ b/app/assets/javascripts/error_tracking_settings/store/getters.js @@ -0,0 +1,44 @@ +import _ from 'underscore'; +import { __, s__, sprintf } from '~/locale'; +import { getDisplayName } from '../utils'; + +export const hasProjects = state => !!state.projects && state.projects.length > 0; + +export const isProjectInvalid = (state, getters) => + !!state.selectedProject && + getters.hasProjects && + !state.projects.some(project => _.isMatch(state.selectedProject, project)); + +export const dropdownLabel = (state, getters) => { + if (state.selectedProject !== null) { + return getDisplayName(state.selectedProject); + } + if (!getters.hasProjects) { + return s__('ErrorTracking|No projects available'); + } + return s__('ErrorTracking|Select project'); +}; + +export const invalidProjectLabel = state => { + if (state.selectedProject) { + return sprintf( + __('Project "%{name}" is no longer available. Select another project to continue.'), + { + name: state.selectedProject.name, + }, + ); + } + return ''; +}; + +export const projectSelectionLabel = state => { + if (state.token) { + return s__( + "ErrorTracking|Click 'Connect' to re-establish the connection to Sentry and activate the dropdown.", + ); + } + return s__('ErrorTracking|To enable project selection, enter a valid Auth Token'); +}; + +// prevent babel-plugin-rewire from generating an invalid default during karma tests +export default () => {}; diff --git a/app/assets/javascripts/error_tracking_settings/store/index.js b/app/assets/javascripts/error_tracking_settings/store/index.js new file mode 100644 index 00000000000..560f265a2ea --- /dev/null +++ b/app/assets/javascripts/error_tracking_settings/store/index.js @@ -0,0 +1,16 @@ +import Vue from 'vue'; +import Vuex from 'vuex'; +import createState from './state'; +import * as actions from './actions'; +import * as getters from './getters'; +import mutations from './mutations'; + +Vue.use(Vuex); + +export default () => + new Vuex.Store({ + state: createState(), + actions, + getters, + mutations, + }); diff --git a/app/assets/javascripts/error_tracking_settings/store/mutation_types.js b/app/assets/javascripts/error_tracking_settings/store/mutation_types.js new file mode 100644 index 00000000000..b4f8a237947 --- /dev/null +++ b/app/assets/javascripts/error_tracking_settings/store/mutation_types.js @@ -0,0 +1,11 @@ +export const CLEAR_PROJECTS = 'CLEAR_PROJECTS'; +export const SET_INITIAL_STATE = 'SET_INITIAL_STATE'; +export const RECEIVE_PROJECTS = 'RECEIVE_PROJECTS'; +export const RESET_CONNECT = 'RESET_CONNECT'; +export const UPDATE_API_HOST = 'UPDATE_API_HOST'; +export const UPDATE_CONNECT_ERROR = 'UPDATE_CONNECT_ERROR'; +export const UPDATE_CONNECT_SUCCESS = 'UPDATE_CONNECT_SUCCESS'; +export const UPDATE_ENABLED = 'UPDATE_ENABLED'; +export const UPDATE_SELECTED_PROJECT = 'UPDATE_SELECTED_PROJECT'; +export const UPDATE_SETTINGS_LOADING = 'UPDATE_SETTINGS_LOADING'; +export const UPDATE_TOKEN = 'UPDATE_TOKEN'; diff --git a/app/assets/javascripts/error_tracking_settings/store/mutations.js b/app/assets/javascripts/error_tracking_settings/store/mutations.js new file mode 100644 index 00000000000..4089d1ee94e --- /dev/null +++ b/app/assets/javascripts/error_tracking_settings/store/mutations.js @@ -0,0 +1,61 @@ +import _ from 'underscore'; +import { convertObjectPropsToCamelCase, parseBoolean } from '~/lib/utils/common_utils'; +import * as types from './mutation_types'; +import { projectKeys } from '../utils'; + +export default { + [types.CLEAR_PROJECTS](state) { + state.projects = []; + }, + [types.RECEIVE_PROJECTS](state, projects) { + state.projects = projects + .map(convertObjectPropsToCamelCase) + // The `pick` strips out extra properties returned from Sentry. + // Such properties could be problematic later, e.g. when checking whether `projects` contains `selectedProject` + .map(project => _.pick(project, projectKeys)); + }, + [types.RESET_CONNECT](state) { + state.connectSuccessful = false; + state.connectError = false; + }, + [types.SET_INITIAL_STATE]( + state, + { apiHost, enabled, project, token, listProjectsEndpoint, operationsSettingsEndpoint }, + ) { + state.enabled = parseBoolean(enabled); + state.apiHost = apiHost; + state.token = token; + state.listProjectsEndpoint = listProjectsEndpoint; + state.operationsSettingsEndpoint = operationsSettingsEndpoint; + + if (project) { + state.selectedProject = _.pick( + convertObjectPropsToCamelCase(JSON.parse(project)), + projectKeys, + ); + } + }, + [types.UPDATE_API_HOST](state, apiHost) { + state.apiHost = apiHost; + }, + [types.UPDATE_ENABLED](state, enabled) { + state.enabled = enabled; + }, + [types.UPDATE_TOKEN](state, token) { + state.token = token; + }, + [types.UPDATE_SELECTED_PROJECT](state, selectedProject) { + state.selectedProject = selectedProject; + }, + [types.UPDATE_SETTINGS_LOADING](state, settingsLoading) { + state.settingsLoading = settingsLoading; + }, + [types.UPDATE_CONNECT_SUCCESS](state) { + state.connectSuccessful = true; + state.connectError = false; + }, + [types.UPDATE_CONNECT_ERROR](state) { + state.connectSuccessful = false; + state.connectError = true; + }, +}; diff --git a/app/assets/javascripts/error_tracking_settings/store/state.js b/app/assets/javascripts/error_tracking_settings/store/state.js new file mode 100644 index 00000000000..98219d33f4d --- /dev/null +++ b/app/assets/javascripts/error_tracking_settings/store/state.js @@ -0,0 +1,12 @@ +export default () => ({ + apiHost: '', + enabled: false, + token: '', + projects: [], + selectedProject: null, + settingsLoading: false, + connectSuccessful: false, + connectError: false, + listProjectsEndpoint: '', + operationsSettingsEndpoint: '', +}); diff --git a/app/assets/javascripts/error_tracking_settings/utils.js b/app/assets/javascripts/error_tracking_settings/utils.js new file mode 100644 index 00000000000..6613e04ee0e --- /dev/null +++ b/app/assets/javascripts/error_tracking_settings/utils.js @@ -0,0 +1,18 @@ +export const projectKeys = ['name', 'organizationName', 'organizationSlug', 'slug']; + +export const transformFrontendSettings = ({ apiHost, enabled, token, selectedProject }) => { + const project = selectedProject + ? { + slug: selectedProject.slug, + name: selectedProject.name, + organization_name: selectedProject.organizationName, + organization_slug: selectedProject.organizationSlug, + } + : null; + + return { api_host: apiHost || null, enabled, token: token || null, project }; +}; + +export const getDisplayName = project => `${project.organizationName} | ${project.name}`; + +export default () => {}; diff --git a/app/assets/javascripts/pages/projects/settings/operations/show/index.js b/app/assets/javascripts/pages/projects/settings/operations/show/index.js new file mode 100644 index 00000000000..73c745179be --- /dev/null +++ b/app/assets/javascripts/pages/projects/settings/operations/show/index.js @@ -0,0 +1,5 @@ +import mountErrorTrackingForm from '~/error_tracking_settings'; + +document.addEventListener('DOMContentLoaded', () => { + mountErrorTrackingForm(); +}); diff --git a/app/controllers/projects/settings/operations_controller.rb b/app/controllers/projects/settings/operations_controller.rb index 521ec2acebb..7276964b6e1 100644 --- a/app/controllers/projects/settings/operations_controller.rb +++ b/app/controllers/projects/settings/operations_controller.rb @@ -14,16 +14,37 @@ module Projects def update result = ::Projects::Operations::UpdateService.new(project, current_user, update_params).execute + render_update_response(result) + end + + private + + # overridden in EE + def render_update_response(result) + respond_to do |format| + format.json do + render_update_json_response(result) + end + end + end + + def render_update_json_response(result) if result[:status] == :success flash[:notice] = _('Your changes have been saved') - redirect_to project_settings_operations_path(@project) + render json: { + status: result[:status] + } else - render 'show' + render( + status: result[:http_status] || :bad_request, + json: { + status: result[:status], + message: result[:message] + } + ) end end - private - def error_tracking_setting @error_tracking_setting ||= project.error_tracking_setting || project.build_error_tracking_setting @@ -35,7 +56,14 @@ module Projects # overridden in EE def permitted_project_params - { error_tracking_setting_attributes: [:enabled, :api_url, :token] } + { + error_tracking_setting_attributes: [ + :enabled, + :api_host, + :token, + project: [:slug, :name, :organization_slug, :organization_name] + ] + } end def check_license diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index c400302cda3..2ac8ddc5244 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -284,6 +284,20 @@ module ProjectsHelper can?(current_user, :read_environment, @project) end + def error_tracking_setting_project_json + setting = @project.error_tracking_setting + + return if setting.blank? || setting.project_slug.blank? || + setting.organization_slug.blank? + + { + name: setting.project_name, + organization_name: setting.organization_name, + organization_slug: setting.organization_slug, + slug: setting.project_slug + }.to_json + end + private def get_project_nav_tabs(project, current_user) diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb index 57283a78ea9..1e2bd3bda7f 100644 --- a/app/models/error_tracking/project_error_tracking_setting.rb +++ b/app/models/error_tracking/project_error_tracking_setting.rb @@ -2,19 +2,30 @@ module ErrorTracking class ProjectErrorTrackingSetting < ActiveRecord::Base + include Gitlab::Utils::StrongMemoize include ReactiveCaching + API_URL_PATH_REGEXP = %r{ + \A + (?<prefix>/api/0/projects/+) + (?: + (?<organization>[^/]+)/+ + (?<project>[^/]+)/* + )? + \z + }x + self.reactive_cache_key = ->(setting) { [setting.class.model_name.singular, setting.project_id] } belongs_to :project validates :api_url, length: { maximum: 255 }, public_url: true, url: { enforce_sanitization: true, ascii_only: true }, allow_nil: true - validates :api_url, presence: true, if: :enabled + validates :api_url, presence: { message: 'is a required field' }, if: :enabled validate :validate_api_url_path, if: :enabled - validates :token, presence: true, if: :enabled + validates :token, presence: { message: 'is a required field' }, if: :enabled attr_encrypted :token, mode: :per_attribute_iv, @@ -23,6 +34,11 @@ module ErrorTracking after_save :clear_reactive_cache! + def api_url=(value) + super + clear_memoization(:api_url_slugs) + end + def project_name super || project_name_from_slug end @@ -40,6 +56,8 @@ module ErrorTracking end def self.build_api_url_from(api_host:, project_slug:, organization_slug:) + return if api_host.blank? + uri = Addressable::URI.parse("#{api_host}/api/0/projects/#{organization_slug}/#{project_slug}/") uri.path = uri.path.squeeze('/') @@ -100,34 +118,39 @@ module ErrorTracking end def project_slug_from_api_url - extract_slug(:project) + api_url_slug(:project) end def organization_slug_from_api_url - extract_slug(:organization) + api_url_slug(:organization) + end + + def api_url_slug(capture) + slugs = strong_memoize(:api_url_slugs) { extract_api_url_slugs || {} } + slugs[capture] end - def extract_slug(capture) + def extract_api_url_slugs return if api_url.blank? begin url = Addressable::URI.parse(api_url) rescue Addressable::URI::InvalidURIError - return nil + return end - @slug_match ||= url.path.match(%r{^/api/0/projects/+(?<organization>[^/]+)/+(?<project>[^/|$]+)}) || {} - @slug_match[capture] + url.path.match(API_URL_PATH_REGEXP) end def validate_api_url_path return if api_url.blank? - begin - unless Addressable::URI.parse(api_url).path.starts_with?('/api/0/projects') - errors.add(:api_url, 'path needs to start with /api/0/projects') - end - rescue Addressable::URI::InvalidURIError + unless api_url_slug(:prefix) + return errors.add(:api_url, 'is invalid') + end + + unless api_url_slug(:organization) + errors.add(:project, 'is a required field') end end end diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb index c6e8be0f2be..4e92353a13c 100644 --- a/app/services/error_tracking/list_projects_service.rb +++ b/app/services/error_tracking/list_projects_service.rb @@ -28,8 +28,8 @@ module ErrorTracking (project.error_tracking_setting || project.build_error_tracking_setting).tap do |setting| setting.api_url = ErrorTracking::ProjectErrorTrackingSetting.build_api_url_from( api_host: params[:api_host], - organization_slug: nil, - project_slug: nil + organization_slug: 'org', + project_slug: 'proj' ) setting.token = params[:token] diff --git a/app/services/projects/operations/update_service.rb b/app/services/projects/operations/update_service.rb index abd6d8de750..aedf79c86d7 100644 --- a/app/services/projects/operations/update_service.rb +++ b/app/services/projects/operations/update_service.rb @@ -12,7 +12,28 @@ module Projects private def project_update_params - params.slice(:error_tracking_setting_attributes) + error_tracking_params + end + + def error_tracking_params + settings = params[:error_tracking_setting_attributes] + return {} if settings.blank? + + api_url = ErrorTracking::ProjectErrorTrackingSetting.build_api_url_from( + api_host: settings[:api_host], + project_slug: settings.dig(:project, :slug), + organization_slug: settings.dig(:project, :organization_slug) + ) + + { + error_tracking_setting_attributes: { + api_url: api_url, + token: settings[:token], + enabled: settings[:enabled], + project_name: settings.dig(:project, :name), + organization_name: settings.dig(:project, :organization_name) + } + } end end end diff --git a/app/views/projects/settings/operations/_error_tracking.html.haml b/app/views/projects/settings/operations/_error_tracking.html.haml index 4911e8d3770..6b15331db01 100644 --- a/app/views/projects/settings/operations/_error_tracking.html.haml +++ b/app/views/projects/settings/operations/_error_tracking.html.haml @@ -8,23 +8,11 @@ = _('Error Tracking') %p = _('To link Sentry to GitLab, enter your Sentry URL and Auth Token.') + = link_to _('More information'), help_page_path('user/project/operations/error_tracking'), target: '_blank', rel: 'noopener noreferrer' .settings-content - = form_for @project, url: project_settings_operations_path(@project), method: :patch do |f| - = form_errors(@project) - .form-group - = f.fields_for :error_tracking_setting_attributes, setting do |form| - .form-check.form-group - = form.check_box :enabled, class: 'form-check-input' - = form.label :enabled, _('Active'), class: 'form-check-label' - .form-group - = form.label :api_url, _('Sentry API URL'), class: 'label-bold' - = form.url_field :api_url, class: 'form-control', placeholder: _('http://<sentry-host>/api/0/projects/{organization_slug}/{project_slug}/') - %p.form-text.text-muted - = _('Enter your Sentry API URL') - .form-group - = form.label :token, _('Auth Token'), class: 'label-bold' - = form.text_field :token, class: 'form-control' - %p.form-text.text-muted - = _('Find and manage Auth Tokens in your Sentry account settings page.') - - = f.submit _('Save changes'), class: 'btn btn-success' + .js-error-tracking-form{ data: { list_projects_endpoint: list_projects_project_error_tracking_index_path(@project, format: :json), + operations_settings_endpoint: project_settings_operations_path(@project), + project: error_tracking_setting_project_json, + api_host: setting.api_host, + enabled: setting.enabled.to_json, + token: setting.token } } diff --git a/changelogs/unreleased/tr-error-tracking-project-selection.yml b/changelogs/unreleased/tr-error-tracking-project-selection.yml new file mode 100644 index 00000000000..36cfe4556bb --- /dev/null +++ b/changelogs/unreleased/tr-error-tracking-project-selection.yml @@ -0,0 +1,5 @@ +--- +title: Error tracking configuration - add a Sentry project selection dropdown +merge_request: 24701 +author: +type: changed diff --git a/config/locales/en.yml b/config/locales/en.yml index e8dbc033a7c..eb3b7771968 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -10,6 +10,10 @@ en: target: Target issue group: path: Group URL + project/error_tracking_setting: + token: "Auth Token" + project: "Project" + api_url: "Sentry API URL" errors: messages: label_already_exists_at_group_level: "already exists at group level for %{group}. Please choose another one." diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 04833adce09..cb599e4744c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -888,9 +888,6 @@ msgstr "" msgid "August" msgstr "" -msgid "Auth Token" -msgstr "" - msgid "Authentication Log" msgstr "" @@ -3028,9 +3025,6 @@ msgstr "" msgid "Enter the merge request title" msgstr "" -msgid "Enter your Sentry API URL" -msgstr "" - msgid "Environment variables" msgstr "" @@ -3205,6 +3199,33 @@ msgstr "" msgid "Error:" msgstr "" +msgid "ErrorTracking|Active" +msgstr "" + +msgid "ErrorTracking|After adding your Auth Token, use the 'Connect' button to load projects" +msgstr "" + +msgid "ErrorTracking|Auth Token" +msgstr "" + +msgid "ErrorTracking|Click 'Connect' to re-establish the connection to Sentry and activate the dropdown." +msgstr "" + +msgid "ErrorTracking|Connection has failed. Re-check Auth Token and try again." +msgstr "" + +msgid "ErrorTracking|Find your hostname in your Sentry account settings page" +msgstr "" + +msgid "ErrorTracking|No projects available" +msgstr "" + +msgid "ErrorTracking|Select project" +msgstr "" + +msgid "ErrorTracking|To enable project selection, enter a valid Auth Token" +msgstr "" + msgid "Errors" msgstr "" @@ -3429,9 +3450,6 @@ msgstr "" msgid "Filter..." msgstr "" -msgid "Find and manage Auth Tokens in your Sentry account settings page." -msgstr "" - msgid "Find by path" msgstr "" @@ -5872,6 +5890,9 @@ msgstr "" msgid "Project" msgstr "" +msgid "Project \"%{name}\" is no longer available. Select another project to continue." +msgstr "" + msgid "Project '%{project_name}' is in the process of being deleted." msgstr "" @@ -5977,6 +5998,9 @@ msgstr "" msgid "Projects" msgstr "" +msgid "Projects Successfully Retrieved" +msgstr "" + msgid "Projects shared with %{group_name}" msgstr "" @@ -7456,6 +7480,9 @@ msgstr "" msgid "There was an error loading users activity calendar." msgstr "" +msgid "There was an error saving your changes." +msgstr "" + msgid "There was an error saving your notification settings." msgstr "" @@ -8844,9 +8871,6 @@ msgstr "" msgid "here" msgstr "" -msgid "http://<sentry-host>/api/0/projects/{organization_slug}/{project_slug}/" -msgstr "" - msgid "https://your-bitbucket-server" msgstr "" diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index d989ec22481..02a392f23c2 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/spec/controllers/projects/settings/operations_controller_spec.rb @@ -74,38 +74,55 @@ describe Projects::Settings::OperationsController do { error_tracking_setting_attributes: { enabled: '1', - api_url: 'http://url', - token: 'token' + api_host: 'http://url', + token: 'token', + project: { + slug: 'sentry-project', + name: 'Sentry Project', + organization_slug: 'sentry-org', + organization_name: 'Sentry Org' + } } } end + let(:error_tracking_permitted) do ActionController::Parameters.new(error_tracking_params).permit! end - context 'when update succeeds' do - before do - stub_operations_update_service_returning(status: :success) - end - - it 'shows a notice' do - patch :update, params: project_params(project, error_tracking_params) - - expect(response).to redirect_to(operations_url) - expect(flash[:notice]).to eq _('Your changes have been saved') - end - end - - context 'when update fails' do - before do - stub_operations_update_service_returning(status: :error) + context 'format json' do + context 'when update succeeds' do + before do + stub_operations_update_service_returning(status: :success) + end + + it 'returns success status' do + patch :update, + params: project_params(project, error_tracking_params), + format: :json + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq('status' => 'success') + expect(flash[:notice]).to eq('Your changes have been saved') + end end - it 'renders show page' do - patch :update, params: project_params(project, error_tracking_params) - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:show) + context 'when update fails' do + before do + stub_operations_update_service_returning( + status: :error, + message: 'error message' + ) + end + + it 'returns error' do + patch :update, + params: project_params(project, error_tracking_params), + format: :json + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).not_to be_nil + end end end diff --git a/spec/features/projects/settings/operations_settings_spec.rb b/spec/features/projects/settings/operations_settings_spec.rb index 06290c67c70..af56cb0d4ee 100644 --- a/spec/features/projects/settings/operations_settings_spec.rb +++ b/spec/features/projects/settings/operations_settings_spec.rb @@ -20,4 +20,81 @@ describe 'Projects > Settings > For a forked project', :js do expect(page).to have_selector('a[title="Operations"]', visible: false) end end + + describe 'Settings > Operations' do + context 'error tracking settings form' do + let(:sentry_list_projects_url) { 'http://sentry.example.com/api/0/projects/' } + + context 'success path' do + let(:projects_sample_response) do + Gitlab::Utils.deep_indifferent_access( + JSON.parse(fixture_file('sentry/list_projects_sample_response.json')) + ) + end + + before do + WebMock.stub_request(:get, sentry_list_projects_url) + .to_return( + status: 200, + headers: { 'Content-Type' => 'application/json' }, + body: projects_sample_response.to_json + ) + end + + it 'successfully fills and submits the form' do + visit project_settings_operations_path(project) + + wait_for_requests + + expect(page).to have_content('Sentry API URL') + expect(page.body).to include('Error Tracking') + expect(page).to have_button('Connect') + + check('Active') + fill_in('error-tracking-api-host', with: 'http://sentry.example.com') + fill_in('error-tracking-token', with: 'token') + + click_button('Connect') + + within('div#project-dropdown') do + click_button('Select project') + click_button('Sentry | Internal') + end + + click_button('Save changes') + + wait_for_requests + + assert_text('Your changes have been saved') + end + end + + context 'project dropdown fails to load' do + before do + WebMock.stub_request(:get, sentry_list_projects_url) + .to_return( + status: 400, + headers: { 'Content-Type' => 'application/json' }, + body: { + message: 'Sentry response code: 401' + }.to_json + ) + end + + it 'displays error message' do + visit project_settings_operations_path(project) + + wait_for_requests + + check('Active') + fill_in('error-tracking-api-host', with: 'http://sentry.example.com') + fill_in('error-tracking-token', with: 'token') + + click_button('Connect') + + assert_text('Connection has failed. Re-check Auth Token and try again.') + end + end + end + end end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 49895b0680b..291eafece94 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -3,6 +3,56 @@ require 'spec_helper' describe ProjectsHelper do include ProjectForksHelper + describe '#error_tracking_setting_project_json' do + let(:project) { create(:project) } + + context 'error tracking setting does not exist' do + before do + helper.instance_variable_set(:@project, project) + end + + it 'returns nil' do + expect(helper.error_tracking_setting_project_json).to be_nil + end + end + + context 'error tracking setting exists' do + let!(:error_tracking_setting) { create(:project_error_tracking_setting, project: project) } + + context 'api_url present' do + let(:json) do + { + name: error_tracking_setting.project_name, + organization_name: error_tracking_setting.organization_name, + organization_slug: error_tracking_setting.organization_slug, + slug: error_tracking_setting.project_slug + }.to_json + end + + before do + helper.instance_variable_set(:@project, project) + end + + it 'returns error tracking json' do + expect(helper.error_tracking_setting_project_json).to eq(json) + end + end + + context 'api_url not present' do + before do + project.error_tracking_setting.api_url = nil + project.error_tracking_setting.enabled = false + + helper.instance_variable_set(:@project, project) + end + + it 'returns nil' do + expect(helper.error_tracking_setting_project_json).to be_nil + end + end + end + end + describe "#project_status_css_class" do it "returns appropriate class" do expect(project_status_css_class("started")).to eq("table-active") diff --git a/spec/javascripts/error_tracking_settings/components/app_spec.js b/spec/javascripts/error_tracking_settings/components/app_spec.js new file mode 100644 index 00000000000..2e52a45fd34 --- /dev/null +++ b/spec/javascripts/error_tracking_settings/components/app_spec.js @@ -0,0 +1,63 @@ +import Vuex from 'vuex'; +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import ErrorTrackingSettings from '~/error_tracking_settings/components/app.vue'; +import ErrorTrackingForm from '~/error_tracking_settings/components/error_tracking_form.vue'; +import ProjectDropdown from '~/error_tracking_settings/components/project_dropdown.vue'; +import createStore from '~/error_tracking_settings/store'; +import { TEST_HOST } from 'spec/test_constants'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('error tracking settings app', () => { + let store; + let wrapper; + + function mountComponent() { + wrapper = shallowMount(ErrorTrackingSettings, { + localVue, + store, // Override the imported store + propsData: { + initialEnabled: 'true', + initialApiHost: TEST_HOST, + initialToken: 'someToken', + initialProject: null, + listProjectsEndpoint: TEST_HOST, + operationsSettingsEndpoint: TEST_HOST, + }, + }); + } + + beforeEach(() => { + store = createStore(); + + mountComponent(); + }); + + afterEach(() => { + if (wrapper) { + wrapper.destroy(); + } + }); + + describe('section', () => { + it('renders the form and dropdown', () => { + expect(wrapper.find(ErrorTrackingForm).exists()).toBeTruthy(); + expect(wrapper.find(ProjectDropdown).exists()).toBeTruthy(); + }); + + it('renders the Save Changes button', () => { + expect(wrapper.find('.js-error-tracking-button').exists()).toBeTruthy(); + }); + + it('enables the button by default', () => { + expect(wrapper.find('.js-error-tracking-button').attributes('disabled')).toBeFalsy(); + }); + + it('disables the button when saving', () => { + store.state.settingsLoading = true; + + expect(wrapper.find('.js-error-tracking-button').attributes('disabled')).toBeTruthy(); + }); + }); +}); diff --git a/spec/javascripts/error_tracking_settings/components/error_tracking_form_spec.js b/spec/javascripts/error_tracking_settings/components/error_tracking_form_spec.js new file mode 100644 index 00000000000..23e57c4bbf1 --- /dev/null +++ b/spec/javascripts/error_tracking_settings/components/error_tracking_form_spec.js @@ -0,0 +1,91 @@ +import Vuex from 'vuex'; +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import { GlButton, GlFormInput } from '@gitlab/ui'; +import ErrorTrackingForm from '~/error_tracking_settings/components/error_tracking_form.vue'; +import { defaultProps } from '../mock'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('error tracking settings form', () => { + let wrapper; + + function mountComponent() { + wrapper = shallowMount(ErrorTrackingForm, { + localVue, + propsData: defaultProps, + }); + } + + beforeEach(() => { + mountComponent(); + }); + + afterEach(() => { + if (wrapper) { + wrapper.destroy(); + } + }); + + describe('an empty form', () => { + it('is rendered', () => { + expect(wrapper.findAll(GlFormInput).length).toBe(2); + expect(wrapper.find(GlFormInput).attributes('id')).toBe('error-tracking-api-host'); + expect( + wrapper + .findAll(GlFormInput) + .at(1) + .attributes('id'), + ).toBe('error-tracking-token'); + + expect(wrapper.findAll(GlButton).exists()).toBe(true); + }); + + it('is rendered with labels and placeholders', () => { + const pageText = wrapper.text(); + + expect(pageText).toContain('Find your hostname in your Sentry account settings page'); + expect(pageText).toContain( + "After adding your Auth Token, use the 'Connect' button to load projects", + ); + + expect(pageText).not.toContain('Connection has failed. Re-check Auth Token and try again'); + expect( + wrapper + .findAll(GlFormInput) + .at(0) + .attributes('placeholder'), + ).toContain('https://mysentryserver.com'); + }); + }); + + describe('after a successful connection', () => { + beforeEach(() => { + wrapper.setProps({ connectSuccessful: true }); + }); + + it('shows the success checkmark', () => { + expect(wrapper.find('.js-error-tracking-connect-success').isVisible()).toBe(true); + }); + + it('does not show an error', () => { + expect(wrapper.text()).not.toContain( + 'Connection has failed. Re-check Auth Token and try again', + ); + }); + }); + + describe('after an unsuccessful connection', () => { + beforeEach(() => { + wrapper.setProps({ connectError: true }); + }); + + it('does not show the check mark', () => { + expect(wrapper.find('.js-error-tracking-connect-success').isVisible()).toBe(false); + }); + + it('shows an error', () => { + expect(wrapper.text()).toContain('Connection has failed. Re-check Auth Token and try again'); + }); + }); +}); diff --git a/spec/javascripts/error_tracking_settings/components/project_dropdown_spec.js b/spec/javascripts/error_tracking_settings/components/project_dropdown_spec.js new file mode 100644 index 00000000000..8e5dbe28452 --- /dev/null +++ b/spec/javascripts/error_tracking_settings/components/project_dropdown_spec.js @@ -0,0 +1,109 @@ +import _ from 'underscore'; +import Vuex from 'vuex'; +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; +import ProjectDropdown from '~/error_tracking_settings/components/project_dropdown.vue'; +import { defaultProps, projectList, staleProject } from '../mock'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('error tracking settings project dropdown', () => { + let wrapper; + + function mountComponent() { + wrapper = shallowMount(ProjectDropdown, { + localVue, + propsData: { + ..._.pick( + defaultProps, + 'dropdownLabel', + 'invalidProjectLabel', + 'projects', + 'projectSelectionLabel', + 'selectedProject', + 'token', + ), + hasProjects: false, + isProjectInvalid: false, + }, + }); + } + + beforeEach(() => { + mountComponent(); + }); + + afterEach(() => { + if (wrapper) { + wrapper.destroy(); + } + }); + + describe('empty project list', () => { + it('renders the dropdown', () => { + expect(wrapper.find('#project-dropdown').exists()).toBeTruthy(); + expect(wrapper.find(GlDropdown).exists()).toBeTruthy(); + }); + + it('shows helper text', () => { + expect(wrapper.find('.js-project-dropdown-label').exists()).toBeTruthy(); + expect(wrapper.find('.js-project-dropdown-label').text()).toContain( + 'To enable project selection', + ); + }); + + it('does not show an error', () => { + expect(wrapper.find('.js-project-dropdown-error').exists()).toBeFalsy(); + }); + + it('does not contain any dropdown items', () => { + expect(wrapper.find(GlDropdownItem).exists()).toBeFalsy(); + expect(wrapper.find(GlDropdown).props('text')).toBe('No projects available'); + }); + }); + + describe('populated project list', () => { + beforeEach(() => { + wrapper.setProps({ projects: _.clone(projectList), hasProjects: true }); + }); + + it('renders the dropdown', () => { + expect(wrapper.find('#project-dropdown').exists()).toBeTruthy(); + expect(wrapper.find(GlDropdown).exists()).toBeTruthy(); + }); + + it('contains a number of dropdown items', () => { + expect(wrapper.find(GlDropdownItem).exists()).toBeTruthy(); + expect(wrapper.findAll(GlDropdownItem).length).toBe(2); + }); + }); + + describe('selected project', () => { + const selectedProject = _.clone(projectList[0]); + + beforeEach(() => { + wrapper.setProps({ projects: _.clone(projectList), selectedProject, hasProjects: true }); + }); + + it('does not show helper text', () => { + expect(wrapper.find('.js-project-dropdown-label').exists()).toBeFalsy(); + expect(wrapper.find('.js-project-dropdown-error').exists()).toBeFalsy(); + }); + }); + + describe('invalid project selected', () => { + beforeEach(() => { + wrapper.setProps({ + projects: _.clone(projectList), + selectedProject: staleProject, + isProjectInvalid: true, + }); + }); + + it('displays a error', () => { + expect(wrapper.find('.js-project-dropdown-label').exists()).toBeFalsy(); + expect(wrapper.find('.js-project-dropdown-error').exists()).toBeTruthy(); + }); + }); +}); diff --git a/spec/javascripts/error_tracking_settings/mock.js b/spec/javascripts/error_tracking_settings/mock.js new file mode 100644 index 00000000000..32cdba33c14 --- /dev/null +++ b/spec/javascripts/error_tracking_settings/mock.js @@ -0,0 +1,92 @@ +import createStore from '~/error_tracking_settings/store'; +import { TEST_HOST } from 'spec/test_constants'; + +const defaultStore = createStore(); + +export const projectList = [ + { + name: 'name', + slug: 'slug', + organizationName: 'organizationName', + organizationSlug: 'organizationSlug', + }, + { + name: 'name2', + slug: 'slug2', + organizationName: 'organizationName2', + organizationSlug: 'organizationSlug2', + }, +]; + +export const staleProject = { + name: 'staleName', + slug: 'staleSlug', + organizationName: 'staleOrganizationName', + organizationSlug: 'staleOrganizationSlug', +}; + +export const normalizedProject = { + name: 'name', + slug: 'slug', + organizationName: 'organization_name', + organizationSlug: 'organization_slug', +}; + +export const sampleBackendProject = { + name: normalizedProject.name, + slug: normalizedProject.slug, + organization_name: normalizedProject.organizationName, + organization_slug: normalizedProject.organizationSlug, +}; + +export const sampleFrontendSettings = { + apiHost: 'apiHost', + enabled: false, + token: 'token', + selectedProject: { + slug: normalizedProject.slug, + name: normalizedProject.name, + organizationName: normalizedProject.organizationName, + organizationSlug: normalizedProject.organizationSlug, + }, +}; + +export const transformedSettings = { + api_host: 'apiHost', + enabled: false, + token: 'token', + project: { + slug: normalizedProject.slug, + name: normalizedProject.name, + organization_name: normalizedProject.organizationName, + organization_slug: normalizedProject.organizationSlug, + }, +}; + +export const defaultProps = { + ...defaultStore.state, + ...defaultStore.getters, +}; + +export const initialEmptyState = { + apiHost: '', + enabled: false, + project: null, + token: '', + listProjectsEndpoint: TEST_HOST, + operationsSettingsEndpoint: TEST_HOST, +}; + +export const initialPopulatedState = { + apiHost: 'apiHost', + enabled: true, + project: JSON.stringify(projectList[0]), + token: 'token', + listProjectsEndpoint: TEST_HOST, + operationsSettingsEndpoint: TEST_HOST, +}; + +export const projectWithHtmlTemplate = { + ...projectList[0], + name: '<strong>bold</strong>', +}; diff --git a/spec/javascripts/error_tracking_settings/store/actions_spec.js b/spec/javascripts/error_tracking_settings/store/actions_spec.js new file mode 100644 index 00000000000..0255b3a7aa4 --- /dev/null +++ b/spec/javascripts/error_tracking_settings/store/actions_spec.js @@ -0,0 +1,191 @@ +import MockAdapter from 'axios-mock-adapter'; +import testAction from 'spec/helpers/vuex_action_helper'; +import { TEST_HOST } from 'spec/test_constants'; +import axios from '~/lib/utils/axios_utils'; +import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; +import actionsDefaultExport, * as actions from '~/error_tracking_settings/store/actions'; +import * as types from '~/error_tracking_settings/store/mutation_types'; +import defaultState from '~/error_tracking_settings/store/state'; +import { projectList } from '../mock'; + +describe('error tracking settings actions', () => { + let state; + + describe('project list actions', () => { + let mock; + + beforeEach(() => { + mock = new MockAdapter(axios); + state = { ...defaultState(), listProjectsEndpoint: TEST_HOST }; + }); + + afterEach(() => { + mock.restore(); + }); + + it('should request and transform the project list', done => { + mock.onPost(TEST_HOST).reply(() => [200, { projects: projectList }]); + testAction( + actions.fetchProjects, + null, + state, + [], + [ + { type: 'requestProjects' }, + { + type: 'receiveProjectsSuccess', + payload: projectList.map(convertObjectPropsToCamelCase), + }, + ], + () => { + expect(mock.history.post.length).toBe(1); + done(); + }, + ); + }); + + it('should handle a server error', done => { + mock.onPost(`${TEST_HOST}.json`).reply(() => [400]); + testAction( + actions.fetchProjects, + null, + state, + [], + [ + { type: 'requestProjects' }, + { + type: 'receiveProjectsError', + }, + ], + () => { + expect(mock.history.post.length).toBe(1); + done(); + }, + ); + }); + + it('should request projects correctly', done => { + testAction(actions.requestProjects, null, state, [{ type: types.RESET_CONNECT }], [], done); + }); + + it('should receive projects correctly', done => { + const testPayload = []; + testAction( + actions.receiveProjectsSuccess, + testPayload, + state, + [ + { type: types.UPDATE_CONNECT_SUCCESS }, + { type: types.RECEIVE_PROJECTS, payload: testPayload }, + ], + [], + done, + ); + }); + + it('should handle errors when receiving projects', done => { + const testPayload = []; + testAction( + actions.receiveProjectsError, + testPayload, + state, + [{ type: types.UPDATE_CONNECT_ERROR }, { type: types.CLEAR_PROJECTS }], + [], + done, + ); + }); + }); + + describe('save changes actions', () => { + let mock; + + beforeEach(() => { + mock = new MockAdapter(axios); + state = { + operationsSettingsEndpoint: TEST_HOST, + }; + }); + + afterEach(() => { + mock.restore(); + }); + + it('should save the page', done => { + const refreshCurrentPage = spyOnDependency(actionsDefaultExport, 'refreshCurrentPage'); + mock.onPatch(TEST_HOST).reply(200); + testAction(actions.updateSettings, null, state, [], [{ type: 'requestSettings' }], () => { + expect(mock.history.patch.length).toBe(1); + expect(refreshCurrentPage).toHaveBeenCalled(); + done(); + }); + }); + + it('should handle a server error', done => { + mock.onPatch(TEST_HOST).reply(400); + testAction( + actions.updateSettings, + null, + state, + [], + [ + { type: 'requestSettings' }, + { + type: 'receiveSettingsError', + payload: new Error('Request failed with status code 400'), + }, + ], + () => { + expect(mock.history.patch.length).toBe(1); + done(); + }, + ); + }); + + it('should request to save the page', done => { + testAction( + actions.requestSettings, + null, + state, + [{ type: types.UPDATE_SETTINGS_LOADING, payload: true }], + [], + done, + ); + }); + + it('should handle errors when requesting to save the page', done => { + testAction( + actions.receiveSettingsError, + {}, + state, + [{ type: types.UPDATE_SETTINGS_LOADING, payload: false }], + [], + done, + ); + }); + }); + + describe('generic actions to update the store', () => { + const testData = 'test'; + it('should reset the `connect success` flag when updating the api host', done => { + testAction( + actions.updateApiHost, + testData, + state, + [{ type: types.UPDATE_API_HOST, payload: testData }, { type: types.RESET_CONNECT }], + [], + done, + ); + }); + + it('should reset the `connect success` flag when updating the token', done => { + testAction( + actions.updateToken, + testData, + state, + [{ type: types.UPDATE_TOKEN, payload: testData }, { type: types.RESET_CONNECT }], + [], + done, + ); + }); + }); +}); diff --git a/spec/javascripts/error_tracking_settings/store/getters_spec.js b/spec/javascripts/error_tracking_settings/store/getters_spec.js new file mode 100644 index 00000000000..2c5ff084b8a --- /dev/null +++ b/spec/javascripts/error_tracking_settings/store/getters_spec.js @@ -0,0 +1,93 @@ +import * as getters from '~/error_tracking_settings/store/getters'; +import defaultState from '~/error_tracking_settings/store/state'; +import { projectList, projectWithHtmlTemplate, staleProject } from '../mock'; + +describe('Error Tracking Settings - Getters', () => { + let state; + + beforeEach(() => { + state = defaultState(); + }); + + describe('hasProjects', () => { + it('should reflect when no projects exist', () => { + expect(getters.hasProjects(state)).toEqual(false); + }); + + it('should reflect when projects exist', () => { + state.projects = projectList; + + expect(getters.hasProjects(state)).toEqual(true); + }); + }); + + describe('isProjectInvalid', () => { + const mockGetters = { hasProjects: true }; + it('should show when a project is valid', () => { + state.projects = projectList; + [state.selectedProject] = projectList; + + expect(getters.isProjectInvalid(state, mockGetters)).toEqual(false); + }); + + it('should show when a project is invalid', () => { + state.projects = projectList; + state.selectedProject = staleProject; + + expect(getters.isProjectInvalid(state, mockGetters)).toEqual(true); + }); + }); + + describe('dropdownLabel', () => { + const mockGetters = { hasProjects: false }; + it('should display correctly when there are no projects available', () => { + expect(getters.dropdownLabel(state, mockGetters)).toEqual('No projects available'); + }); + + it('should display correctly when a project is selected', () => { + [state.selectedProject] = projectList; + + expect(getters.dropdownLabel(state, mockGetters)).toEqual('organizationName | name'); + }); + + it('should display correctly when no project is selected', () => { + state.projects = projectList; + + expect(getters.dropdownLabel(state, { hasProjects: true })).toEqual('Select project'); + }); + }); + + describe('invalidProjectLabel', () => { + it('should display an error containing the project name', () => { + [state.selectedProject] = projectList; + + expect(getters.invalidProjectLabel(state)).toEqual( + 'Project "name" is no longer available. Select another project to continue.', + ); + }); + + it('should properly escape the label text', () => { + state.selectedProject = projectWithHtmlTemplate; + + expect(getters.invalidProjectLabel(state)).toEqual( + 'Project "<strong>bold</strong>" is no longer available. Select another project to continue.', + ); + }); + }); + + describe('projectSelectionLabel', () => { + it('should show the correct message when the token is empty', () => { + expect(getters.projectSelectionLabel(state)).toEqual( + 'To enable project selection, enter a valid Auth Token', + ); + }); + + it('should show the correct message when token exists', () => { + state.token = 'test-token'; + + expect(getters.projectSelectionLabel(state)).toEqual( + "Click 'Connect' to re-establish the connection to Sentry and activate the dropdown.", + ); + }); + }); +}); diff --git a/spec/javascripts/error_tracking_settings/store/mutation_spec.js b/spec/javascripts/error_tracking_settings/store/mutation_spec.js new file mode 100644 index 00000000000..bb1f1da784e --- /dev/null +++ b/spec/javascripts/error_tracking_settings/store/mutation_spec.js @@ -0,0 +1,82 @@ +import { TEST_HOST } from 'spec/test_constants'; +import mutations from '~/error_tracking_settings/store/mutations'; +import defaultState from '~/error_tracking_settings/store/state'; +import * as types from '~/error_tracking_settings/store/mutation_types'; +import { + initialEmptyState, + initialPopulatedState, + projectList, + sampleBackendProject, + normalizedProject, +} from '../mock'; + +describe('error tracking settings mutations', () => { + describe('mutations', () => { + let state; + + beforeEach(() => { + state = defaultState(); + }); + + it('should create an empty initial state correctly', () => { + mutations[types.SET_INITIAL_STATE](state, { + ...initialEmptyState, + }); + + expect(state.apiHost).toEqual(''); + expect(state.enabled).toEqual(false); + expect(state.selectedProject).toEqual(null); + expect(state.token).toEqual(''); + expect(state.listProjectsEndpoint).toEqual(TEST_HOST); + expect(state.operationsSettingsEndpoint).toEqual(TEST_HOST); + }); + + it('should populate the initial state correctly', () => { + mutations[types.SET_INITIAL_STATE](state, { + ...initialPopulatedState, + }); + + expect(state.apiHost).toEqual('apiHost'); + expect(state.enabled).toEqual(true); + expect(state.selectedProject).toEqual(projectList[0]); + expect(state.token).toEqual('token'); + expect(state.listProjectsEndpoint).toEqual(TEST_HOST); + expect(state.operationsSettingsEndpoint).toEqual(TEST_HOST); + }); + + it('should receive projects successfully', () => { + mutations[types.RECEIVE_PROJECTS](state, [sampleBackendProject]); + + expect(state.projects).toEqual([normalizedProject]); + }); + + it('should strip out unnecessary project properties', () => { + mutations[types.RECEIVE_PROJECTS](state, [ + { ...sampleBackendProject, extra_property: 'extra_property' }, + ]); + + expect(state.projects).toEqual([normalizedProject]); + }); + + it('should update state when connect is successful', () => { + mutations[types.UPDATE_CONNECT_SUCCESS](state); + + expect(state.connectSuccessful).toBe(true); + expect(state.connectError).toBe(false); + }); + + it('should update state when connect fails', () => { + mutations[types.UPDATE_CONNECT_ERROR](state); + + expect(state.connectSuccessful).toBe(false); + expect(state.connectError).toBe(true); + }); + + it('should update state when connect is reset', () => { + mutations[types.RESET_CONNECT](state); + + expect(state.connectSuccessful).toBe(false); + expect(state.connectError).toBe(false); + }); + }); +}); diff --git a/spec/javascripts/error_tracking_settings/utils_spec.js b/spec/javascripts/error_tracking_settings/utils_spec.js new file mode 100644 index 00000000000..4b144f7daf1 --- /dev/null +++ b/spec/javascripts/error_tracking_settings/utils_spec.js @@ -0,0 +1,29 @@ +import { transformFrontendSettings } from '~/error_tracking_settings/utils'; +import { sampleFrontendSettings, transformedSettings } from './mock'; + +describe('error tracking settings utils', () => { + describe('data transform functions', () => { + it('should transform settings successfully for the backend', () => { + expect(transformFrontendSettings(sampleFrontendSettings)).toEqual(transformedSettings); + }); + + it('should transform empty values in the settings object to null', () => { + const emptyFrontendSettingsObject = { + apiHost: '', + enabled: false, + token: '', + selectedProject: null, + }; + const transformedEmptySettingsObject = { + api_host: null, + enabled: false, + token: null, + project: null, + }; + + expect(transformFrontendSettings(emptyFrontendSettingsObject)).toEqual( + transformedEmptySettingsObject, + ); + }); + }); +}); diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index 076ccc96041..cbde13a2c7a 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -62,11 +62,32 @@ describe ErrorTracking::ProjectErrorTrackingSetting do end context 'URL path' do - it 'fails validation with wrong path' do + it 'fails validation without api/0/projects' do subject.api_url = 'http://gitlab.com/project1/something' expect(subject).not_to be_valid - expect(subject.errors.messages[:api_url]).to include('path needs to start with /api/0/projects') + expect(subject.errors.messages[:api_url]).to include('is invalid') + end + + it 'fails validation without org and project slugs' do + subject.api_url = 'http://gitlab.com/api/0/projects/' + + expect(subject).not_to be_valid + expect(subject.errors.messages[:project]).to include('is a required field') + end + + it 'fails validation when api_url has extra parts' do + subject.api_url = 'http://gitlab.com/api/0/projects/org/proj/something' + + expect(subject).not_to be_valid + expect(subject.errors.messages[:api_url]).to include("is invalid") + end + + it 'fails validation when api_url has less parts' do + subject.api_url = 'http://gitlab.com/api/0/projects/org' + + expect(subject).not_to be_valid + expect(subject.errors.messages[:api_url]).to include("is invalid") end it 'passes validation with correct path' do @@ -275,6 +296,16 @@ describe ErrorTracking::ProjectErrorTrackingSetting do expect(api_url).to eq(':::') end + + it 'returns nil when api_host is blank' do + api_url = described_class.build_api_url_from( + api_host: '', + organization_slug: 'org-slug', + project_slug: 'proj-slug' + ) + + expect(api_url).to be_nil + end end describe '#api_host' do diff --git a/spec/services/error_tracking/list_projects_service_spec.rb b/spec/services/error_tracking/list_projects_service_spec.rb index 9f25a633deb..a92d3376f7b 100644 --- a/spec/services/error_tracking/list_projects_service_spec.rb +++ b/spec/services/error_tracking/list_projects_service_spec.rb @@ -32,7 +32,7 @@ describe ErrorTracking::ListProjectsService do end context 'set model attributes to new values' do - let(:new_api_url) { new_api_host + 'api/0/projects/' } + let(:new_api_url) { new_api_host + 'api/0/projects/org/proj/' } before do expect(error_tracking_setting).to receive(:list_sentry_projects) @@ -121,7 +121,7 @@ describe ErrorTracking::ListProjectsService do context 'error_tracking_setting is nil' do let(:error_tracking_setting) { build(:project_error_tracking_setting) } - let(:new_api_url) { new_api_host + 'api/0/projects/' } + let(:new_api_url) { new_api_host + 'api/0/projects/org/proj/' } before do expect(project).to receive(:build_error_tracking_setting).once diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb index 6afae3da80c..86b1ec83f50 100644 --- a/spec/services/projects/operations/update_service_spec.rb +++ b/spec/services/projects/operations/update_service_spec.rb @@ -17,8 +17,14 @@ describe Projects::Operations::UpdateService do { error_tracking_setting_attributes: { enabled: false, - api_url: 'http://gitlab.com/api/0/projects/org/project', - token: 'token' + api_host: 'http://gitlab.com/', + token: 'token', + project: { + slug: 'project', + name: 'Project', + organization_slug: 'org', + organization_name: 'Org' + } } } end @@ -32,8 +38,30 @@ describe Projects::Operations::UpdateService do project.reload expect(project.error_tracking_setting).not_to be_enabled - expect(project.error_tracking_setting.api_url).to eq('http://gitlab.com/api/0/projects/org/project') + expect(project.error_tracking_setting.api_url).to eq( + 'http://gitlab.com/api/0/projects/org/project/' + ) expect(project.error_tracking_setting.token).to eq('token') + expect(project.error_tracking_setting[:project_name]).to eq('Project') + expect(project.error_tracking_setting[:organization_name]).to eq('Org') + end + + context 'disable error tracking' do + before do + params[:error_tracking_setting_attributes][:api_host] = '' + params[:error_tracking_setting_attributes][:enabled] = false + end + + it 'can set api_url to nil' do + expect(result[:status]).to eq(:success) + + project.reload + expect(project.error_tracking_setting).not_to be_enabled + expect(project.error_tracking_setting.api_url).to be_nil + expect(project.error_tracking_setting.token).to eq('token') + expect(project.error_tracking_setting[:project_name]).to eq('Project') + expect(project.error_tracking_setting[:organization_name]).to eq('Org') + end end end @@ -42,8 +70,14 @@ describe Projects::Operations::UpdateService do { error_tracking_setting_attributes: { enabled: true, - api_url: 'http://gitlab.com/api/0/projects/org/project', - token: 'token' + api_host: 'http://gitlab.com/', + token: 'token', + project: { + slug: 'project', + name: 'Project', + organization_slug: 'org', + organization_name: 'Org' + } } } end @@ -52,8 +86,12 @@ describe Projects::Operations::UpdateService do expect(result[:status]).to eq(:success) expect(project.error_tracking_setting).to be_enabled - expect(project.error_tracking_setting.api_url).to eq('http://gitlab.com/api/0/projects/org/project') + expect(project.error_tracking_setting.api_url).to eq( + 'http://gitlab.com/api/0/projects/org/project/' + ) expect(project.error_tracking_setting.token).to eq('token') + expect(project.error_tracking_setting[:project_name]).to eq('Project') + expect(project.error_tracking_setting[:organization_name]).to eq('Org') end end diff --git a/spec/views/projects/settings/operations/show.html.haml_spec.rb b/spec/views/projects/settings/operations/show.html.haml_spec.rb index 8e34521c7c8..1bca8bba940 100644 --- a/spec/views/projects/settings/operations/show.html.haml_spec.rb +++ b/spec/views/projects/settings/operations/show.html.haml_spec.rb @@ -30,7 +30,6 @@ describe 'projects/settings/operations/show' do expect(rendered).to have_content _('Error Tracking') expect(rendered).to have_content _('To link Sentry to GitLab, enter your Sentry URL and Auth Token') - expect(rendered).to have_content _('Active') end end end |