diff options
71 files changed, 1270 insertions, 251 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c85b192b34a..e4d878641b3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -20,11 +20,11 @@ class ApplicationController < ActionController::Base before_action :authenticate_user!, except: [:route_not_found] before_action :enforce_terms!, if: :should_enforce_terms? before_action :validate_user_service_ticket! - before_action :check_password_expiration + before_action :check_password_expiration, if: :html_request? before_action :ldap_security_check before_action :sentry_context before_action :default_headers - before_action :add_gon_variables, unless: [:peek_request?, :json_request?] + before_action :add_gon_variables, if: :html_request? before_action :configure_permitted_parameters, if: :devise_controller? before_action :require_email, unless: :devise_controller? before_action :active_user_check, unless: :devise_controller? @@ -455,8 +455,8 @@ class ApplicationController < ActionController::Base response.headers['Page-Title'] = URI.escape(page_title('GitLab')) end - def peek_request? - request.path.start_with?('/-/peek') + def html_request? + request.format.html? end def json_request? @@ -466,7 +466,7 @@ class ApplicationController < ActionController::Base def should_enforce_terms? return false unless Gitlab::CurrentSettings.current_application_settings.enforce_terms - !(peek_request? || devise_controller?) + html_request? && !devise_controller? end def set_usage_stats_consent_flag diff --git a/app/controllers/concerns/confirm_email_warning.rb b/app/controllers/concerns/confirm_email_warning.rb index 86df0010665..32e1a46e580 100644 --- a/app/controllers/concerns/confirm_email_warning.rb +++ b/app/controllers/concerns/confirm_email_warning.rb @@ -4,15 +4,18 @@ module ConfirmEmailWarning extend ActiveSupport::Concern included do - before_action :set_confirm_warning, if: -> { Feature.enabled?(:soft_email_confirmation) } + before_action :set_confirm_warning, if: :show_confirm_warning? end protected + def show_confirm_warning? + html_request? && request.get? && Feature.enabled?(:soft_email_confirmation) + end + def set_confirm_warning return unless current_user return if current_user.confirmed? - return if peek_request? || json_request? || !request.get? email = current_user.unconfirmed_email || current_user.email diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb index b87779c22d3..023c41821da 100644 --- a/app/controllers/concerns/uploads_actions.rb +++ b/app/controllers/concerns/uploads_actions.rb @@ -1,11 +1,16 @@ # frozen_string_literal: true module UploadsActions + extend ActiveSupport::Concern include Gitlab::Utils::StrongMemoize include SendFileUpload UPLOAD_MOUNTS = %w(avatar attachment file logo header_logo favicon).freeze + included do + prepend_before_action :set_request_format_from_path_extension + end + def create uploader = UploadService.new(model, params[:file], uploader_class).execute @@ -64,6 +69,18 @@ module UploadsActions private + # From ActionDispatch::Http::MimeNegotiation. We have an initializer that + # monkey-patches this method out (so that repository paths don't guess a + # format based on extension), but we do want this behaviour when serving + # uploads. + def set_request_format_from_path_extension + path = request.headers['action_dispatch.original_path'] || request.headers['PATH_INFO'] + + if match = path&.match(/\.(\w+)\z/) + request.format = match.captures.first + end + end + def uploader_class raise NotImplementedError end diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 5fd475f5499..e0da3d9b6c9 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -133,7 +133,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController if environment redirect_to environment_metrics_path(environment) else - render :empty + render :empty_metrics end end diff --git a/app/controllers/projects/error_tracking_controller.rb b/app/controllers/projects/error_tracking_controller.rb index 88d0755f41f..9dea6b663ea 100644 --- a/app/controllers/projects/error_tracking_controller.rb +++ b/app/controllers/projects/error_tracking_controller.rb @@ -15,6 +15,23 @@ class Projects::ErrorTrackingController < Projects::ApplicationController end end + def details + respond_to do |format| + format.html + format.json do + render_issue_detail_json + end + end + end + + def stack_trace + respond_to do |format| + format.json do + render_issue_stack_trace_json + end + end + end + def list_projects respond_to do |format| format.json do @@ -29,10 +46,7 @@ class Projects::ErrorTrackingController < Projects::ApplicationController service = ErrorTracking::ListIssuesService.new(project, current_user) result = service.execute - unless result[:status] == :success - return render json: { message: result[:message] }, - status: result[:http_status] || :bad_request - end + return if handle_errors(result) render json: { errors: serialize_errors(result[:issues]), @@ -40,6 +54,28 @@ class Projects::ErrorTrackingController < Projects::ApplicationController } end + def render_issue_detail_json + service = ErrorTracking::IssueDetailsService.new(project, current_user, issue_details_params) + result = service.execute + + return if handle_errors(result) + + render json: { + error: serialize_detailed_error(result[:issue]) + } + end + + def render_issue_stack_trace_json + service = ErrorTracking::IssueLatestEventService.new(project, current_user, issue_details_params) + result = service.execute + + return if handle_errors(result) + + render json: { + error: serialize_error_event(result[:latest_event]) + } + end + def render_project_list_json service = ErrorTracking::ListProjectsService.new( project, @@ -62,10 +98,21 @@ class Projects::ErrorTrackingController < Projects::ApplicationController end end + def handle_errors(result) + unless result[:status] == :success + render json: { message: result[:message] }, + status: result[:http_status] || :bad_request + end + end + def list_projects_params params.require(:error_tracking_setting).permit([:api_host, :token]) end + def issue_details_params + params.permit(:issue_id) + end + def set_polling_interval Gitlab::PollingInterval.set_header(response, interval: POLLING_INTERVAL) end @@ -76,6 +123,18 @@ class Projects::ErrorTrackingController < Projects::ApplicationController .represent(errors) end + def serialize_detailed_error(error) + ErrorTracking::DetailedErrorSerializer + .new(project: project, user: current_user) + .represent(error) + end + + def serialize_error_event(event) + ErrorTracking::ErrorEventSerializer + .new(project: project, user: current_user) + .represent(event) + end + def serialize_projects(projects) ErrorTracking::ProjectSerializer .new(project: project, user: current_user) diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 635db386792..f39a2b81b54 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -20,7 +20,7 @@ class UploadsController < ApplicationController skip_before_action :authenticate_user! before_action :upload_mount_satisfied? - before_action :find_model + before_action :model before_action :authorize_access!, only: [:show] before_action :authorize_create_access!, only: [:create, :authorize] before_action :verify_workhorse_api!, only: [:authorize] diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index ec2e1648904..120958eb631 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -293,7 +293,7 @@ module ApplicationSettingsHelper :snowplow_collector_hostname, :snowplow_cookie_domain, :snowplow_enabled, - :snowplow_site_id, + :snowplow_app_id, :snowplow_iglu_registry_url, :push_event_hooks_limit, :push_event_activities_limit, diff --git a/app/helpers/projects/error_tracking_helper.rb b/app/helpers/projects/error_tracking_helper.rb index fd1222a1dfb..2f5f612ed4c 100644 --- a/app/helpers/projects/error_tracking_helper.rb +++ b/app/helpers/projects/error_tracking_helper.rb @@ -13,4 +13,13 @@ module Projects::ErrorTrackingHelper 'illustration-path' => image_path('illustrations/cluster_popover.svg') } end + + def error_details_data(project, issue) + opts = [project, issue, { format: :json }] + + { + 'issue-details-path' => details_namespace_project_error_tracking_index_path(*opts), + 'issue-stack-trace-path' => stack_trace_namespace_project_error_tracking_index_path(*opts) + } + end end diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index 9119f8766eb..2b2492a793a 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -132,11 +132,12 @@ module ApplicationSettingImplementation snowplow_collector_hostname: nil, snowplow_cookie_domain: nil, snowplow_enabled: false, - snowplow_site_id: nil, + snowplow_app_id: nil, snowplow_iglu_registry_url: nil, custom_http_clone_url_root: nil, pendo_enabled: false, - pendo_url: nil + pendo_url: nil, + productivity_analytics_start_date: Time.now } end diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb index 0b4fef5eac1..0fa19b1cedc 100644 --- a/app/models/error_tracking/project_error_tracking_setting.rb +++ b/app/models/error_tracking/project_error_tracking_setting.rb @@ -87,10 +87,30 @@ module ErrorTracking { projects: sentry_client.list_projects } end + def issue_details(opts = {}) + with_reactive_cache('issue_details', opts.stringify_keys) do |result| + result + end + end + + def issue_latest_event(opts = {}) + with_reactive_cache('issue_latest_event', opts.stringify_keys) do |result| + result + end + end + def calculate_reactive_cache(request, opts) case request when 'list_issues' { issues: sentry_client.list_issues(**opts.symbolize_keys) } + when 'issue_details' + { + issue: sentry_client.issue_details(**opts.symbolize_keys) + } + when 'issue_latest_event' + { + latest_event: sentry_client.issue_latest_event(**opts.symbolize_keys) + } end rescue Sentry::Client::Error => e { error: e.message, error_type: SENTRY_API_ERROR_TYPE_NON_20X_RESPONSE } diff --git a/app/serializers/error_tracking/detailed_error_entity.rb b/app/serializers/error_tracking/detailed_error_entity.rb new file mode 100644 index 00000000000..8f08f84aa41 --- /dev/null +++ b/app/serializers/error_tracking/detailed_error_entity.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module ErrorTracking + class DetailedErrorEntity < Grape::Entity + expose :count, + :culprit, + :external_base_url, + :external_url, + :first_release_last_commit, + :first_release_short_version, + :first_seen, + :frequency, + :id, + :last_release_last_commit, + :last_release_short_version, + :last_seen, + :message, + :project_id, + :project_name, + :project_slug, + :short_id, + :status, + :title, + :type, + :user_count + end +end diff --git a/app/serializers/error_tracking/detailed_error_serializer.rb b/app/serializers/error_tracking/detailed_error_serializer.rb new file mode 100644 index 00000000000..201da16a1ae --- /dev/null +++ b/app/serializers/error_tracking/detailed_error_serializer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module ErrorTracking + class DetailedErrorSerializer < BaseSerializer + entity DetailedErrorEntity + end +end diff --git a/app/serializers/error_tracking/error_event_entity.rb b/app/serializers/error_tracking/error_event_entity.rb new file mode 100644 index 00000000000..6cf0e6e3ae2 --- /dev/null +++ b/app/serializers/error_tracking/error_event_entity.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module ErrorTracking + class ErrorEventEntity < Grape::Entity + expose :issue_id, :date_received, :stack_trace_entries + end +end diff --git a/app/serializers/error_tracking/error_event_serializer.rb b/app/serializers/error_tracking/error_event_serializer.rb new file mode 100644 index 00000000000..bc4eae16368 --- /dev/null +++ b/app/serializers/error_tracking/error_event_serializer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module ErrorTracking + class ErrorEventSerializer < BaseSerializer + entity ErrorEventEntity + end +end diff --git a/app/services/error_tracking/base_service.rb b/app/services/error_tracking/base_service.rb new file mode 100644 index 00000000000..430d9952332 --- /dev/null +++ b/app/services/error_tracking/base_service.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module ErrorTracking + class BaseService < ::BaseService + def execute + unauthorized = check_permissions + return unauthorized if unauthorized + + begin + response = fetch + rescue Sentry::Client::Error => e + return error(e.message, :bad_request) + rescue Sentry::Client::MissingKeysError => e + return error(e.message, :internal_server_error) + end + + errors = parse_errors(response) + return errors if errors + + success(parse_response(response)) + end + + private + + def fetch + raise NotImplementedError, + "#{self.class} does not implement #{__method__}" + end + + def parse_response(response) + raise NotImplementedError, + "#{self.class} does not implement #{__method__}" + end + + def check_permissions + return error('Error Tracking is not enabled') unless enabled? + return error('Access denied', :unauthorized) unless can_read? + end + + def parse_errors(response) + return error('Not ready. Try again later', :no_content) unless response + return error(response[:error], http_status_for(response[:error_type])) if response[:error].present? + end + + def http_status_for(error_type) + case error_type + when ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_MISSING_KEYS + :internal_server_error + else + :bad_request + end + end + + def project_error_tracking_setting + project.error_tracking_setting + end + + def enabled? + project_error_tracking_setting&.enabled? + end + + def can_read? + can?(current_user, :read_sentry_issue, project) + end + end +end diff --git a/app/services/error_tracking/issue_details_service.rb b/app/services/error_tracking/issue_details_service.rb new file mode 100644 index 00000000000..368cd4517fc --- /dev/null +++ b/app/services/error_tracking/issue_details_service.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module ErrorTracking + class IssueDetailsService < ErrorTracking::BaseService + private + + def fetch + project_error_tracking_setting.issue_details(issue_id: params[:issue_id]) + end + + def parse_response(response) + { issue: response[:issue] } + end + end +end diff --git a/app/services/error_tracking/issue_latest_event_service.rb b/app/services/error_tracking/issue_latest_event_service.rb new file mode 100644 index 00000000000..b6ad8f8028b --- /dev/null +++ b/app/services/error_tracking/issue_latest_event_service.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module ErrorTracking + class IssueLatestEventService < ErrorTracking::BaseService + private + + def fetch + project_error_tracking_setting.issue_latest_event(issue_id: params[:issue_id]) + end + + def parse_response(response) + { latest_event: response[:latest_event] } + end + end +end diff --git a/app/services/error_tracking/list_issues_service.rb b/app/services/error_tracking/list_issues_service.rb index 86ab21fa865..e09f4345308 100644 --- a/app/services/error_tracking/list_issues_service.rb +++ b/app/services/error_tracking/list_issues_service.rb @@ -1,48 +1,24 @@ # frozen_string_literal: true module ErrorTracking - class ListIssuesService < ::BaseService + class ListIssuesService < ErrorTracking::BaseService DEFAULT_ISSUE_STATUS = 'unresolved' DEFAULT_LIMIT = 20 - def execute - return error('Error Tracking is not enabled') unless enabled? - return error('Access denied', :unauthorized) unless can_read? - - result = project_error_tracking_setting - .list_sentry_issues(issue_status: issue_status, limit: limit) - - # our results are not yet ready - unless result - return error('Not ready. Try again later', :no_content) - end + private - if result[:error].present? - return error(result[:error], http_status_from_error_type(result[:error_type])) - end + def fetch + project_error_tracking_setting.list_sentry_issues(issue_status: issue_status, limit: limit) + end - success(issues: result[:issues]) + def parse_response(response) + { issues: response[:issues] } end def external_url project_error_tracking_setting&.sentry_external_url end - private - - def http_status_from_error_type(error_type) - case error_type - when ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_MISSING_KEYS - :internal_server_error - else - :bad_request - end - end - - def project_error_tracking_setting - project.error_tracking_setting - end - def issue_status params[:issue_status] || DEFAULT_ISSUE_STATUS end @@ -50,13 +26,5 @@ module ErrorTracking def limit params[:limit] || DEFAULT_LIMIT end - - def enabled? - project_error_tracking_setting&.enabled? - end - - def can_read? - can?(current_user, :read_sentry_issue, project) - end end end diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb index 92d4ef85ecf..09a0b952e84 100644 --- a/app/services/error_tracking/list_projects_service.rb +++ b/app/services/error_tracking/list_projects_service.rb @@ -1,44 +1,38 @@ # frozen_string_literal: true module ErrorTracking - class ListProjectsService < ::BaseService + class ListProjectsService < ErrorTracking::BaseService def execute - return error('access denied') unless can_read? - - setting = project_error_tracking_setting - - unless setting.valid? - return error(setting.errors.full_messages.join(', '), :bad_request) + unless project_error_tracking_setting.valid? + return error(project_error_tracking_setting.errors.full_messages.join(', '), :bad_request) end - begin - result = setting.list_sentry_projects - rescue Sentry::Client::Error => e - return error(e.message, :bad_request) - rescue Sentry::Client::MissingKeysError => e - return error(e.message, :internal_server_error) - end - - success(projects: result[:projects]) + super end private - def project_error_tracking_setting - (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: 'org', - project_slug: 'proj' - ) - - setting.token = token(setting) - setting.enabled = true - end + def fetch + project_error_tracking_setting.list_sentry_projects + end + + def parse_response(response) + { projects: response[:projects] } end - def can_read? - can?(current_user, :read_sentry_issue, project) + def project_error_tracking_setting + @project_error_tracking_setting ||= begin + (super || project.build_error_tracking_setting).tap do |setting| + setting.api_url = ErrorTracking::ProjectErrorTrackingSetting.build_api_url_from( + api_host: params[:api_host], + organization_slug: 'org', + project_slug: 'proj' + ) + + setting.token = token(setting) + setting.enabled = true + end + end end def token(setting) diff --git a/app/views/admin/application_settings/_snowplow.html.haml b/app/views/admin/application_settings/_snowplow.html.haml index ff96c5d64ac..a2597433270 100644 --- a/app/views/admin/application_settings/_snowplow.html.haml +++ b/app/views/admin/application_settings/_snowplow.html.haml @@ -21,8 +21,8 @@ = f.label :snowplow_collector_hostname, _('Collector hostname'), class: 'label-light' = f.text_field :snowplow_collector_hostname, class: 'form-control', placeholder: 'snowplow.example.com' .form-group - = f.label :snowplow_site_id, _('Site ID'), class: 'label-light' - = f.text_field :snowplow_site_id, class: 'form-control' + = f.label :snowplow_app_id, _('App ID'), class: 'label-light' + = f.text_field :snowplow_app_id, class: 'form-control' .form-group = f.label :snowplow_cookie_domain, _('Cookie domain'), class: 'label-light' = f.text_field :snowplow_cookie_domain, class: 'form-control' diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index 2c0cb973542..9b3ad05d0c0 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -247,6 +247,8 @@ %span = _('Serverless') + = render_if_exists 'layouts/nav/sidebar/pod_logs_link' # EE-specific + - if project_nav_tab? :clusters - show_cluster_hint = show_gke_cluster_integration_callout?(@project) = nav_link(controller: [:clusters, :user, :gcp]) do diff --git a/app/views/projects/environments/empty_logs.html.haml b/app/views/projects/environments/empty_logs.html.haml new file mode 100644 index 00000000000..602dc908b75 --- /dev/null +++ b/app/views/projects/environments/empty_logs.html.haml @@ -0,0 +1,14 @@ +- page_title _('Pod logs') + +.row.empty-state + .col-sm-12 + .svg-content + = image_tag 'illustrations/operations_log_pods_empty.svg' + .col-12 + .text-content + %h4.text-center + = s_('Environments|No deployed environments') + %p.state-description.text-center + = s_('Logs|To see the pod logs, deploy your code to an environment.') + .text-center + = link_to s_('Environments|Learn about environments'), help_page_path('ci/environments'), class: 'btn btn-success' diff --git a/app/views/projects/environments/empty.html.haml b/app/views/projects/environments/empty_metrics.html.haml index 129dbbf4e56..dad93290fbd 100644 --- a/app/views/projects/environments/empty.html.haml +++ b/app/views/projects/environments/empty_metrics.html.haml @@ -7,8 +7,8 @@ .col-12 .text-content %h4.text-center - = s_('Metrics|No deployed environments') + = s_('Environments|No deployed environments') %p.state-description = s_('Metrics|Check out the CI/CD documentation on deploying to an environment') .text-center - = link_to s_("Metrics|Learn about environments"), help_page_path('ci/environments'), class: 'btn btn-success' + = link_to s_("Environments|Learn about environments"), help_page_path('ci/environments'), class: 'btn btn-success' diff --git a/app/views/projects/error_tracking/details.html.haml b/app/views/projects/error_tracking/details.html.haml new file mode 100644 index 00000000000..52e748fa88b --- /dev/null +++ b/app/views/projects/error_tracking/details.html.haml @@ -0,0 +1,3 @@ +- page_title _('Error Details') + +#js-error_tracking{ data: error_details_data(@current_user, @project) } diff --git a/changelogs/unreleased/31222-follow-up-from-adjusts-snowplow-to-use-cookies-for-sessions.yml b/changelogs/unreleased/31222-follow-up-from-adjusts-snowplow-to-use-cookies-for-sessions.yml new file mode 100644 index 00000000000..6d62663a0bc --- /dev/null +++ b/changelogs/unreleased/31222-follow-up-from-adjusts-snowplow-to-use-cookies-for-sessions.yml @@ -0,0 +1,5 @@ +--- +title: Rename snowplow_site_id to snowplow_app_id in application_settings table +merge_request: 19252 +author: +type: other diff --git a/changelogs/unreleased/32052-add-pa-start-date-limit.yml b/changelogs/unreleased/32052-add-pa-start-date-limit.yml new file mode 100644 index 00000000000..fa2cf796edb --- /dev/null +++ b/changelogs/unreleased/32052-add-pa-start-date-limit.yml @@ -0,0 +1,5 @@ +--- +title: Add productivity analytics merge date filtering limit +merge_request: 32052 +author: +type: fixed diff --git a/changelogs/unreleased/32464-backend-sentry-error-details.yml b/changelogs/unreleased/32464-backend-sentry-error-details.yml new file mode 100644 index 00000000000..aa7b44f9d08 --- /dev/null +++ b/changelogs/unreleased/32464-backend-sentry-error-details.yml @@ -0,0 +1,5 @@ +--- +title: API for stack trace & detail view of Sentry error in GitLab +merge_request: 19137 +author: +type: added diff --git a/config/routes/project.rb b/config/routes/project.rb index d49ba20ce84..0812f3f7b62 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -441,6 +441,10 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :metrics, action: :metrics_redirect get :folder, path: 'folders/*id', constraints: { format: /(html|json)/ } get :search + + Gitlab.ee do + get :logs, action: :logs_redirect + end end resources :deployments, only: [:index] do @@ -613,6 +617,12 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do resources :error_tracking, only: [:index], controller: :error_tracking do collection do + get ':issue_id/details', + to: 'error_tracking#details', + as: 'details' + get ':issue_id/stack_trace', + to: 'error_tracking#stack_trace', + as: 'stack_trace' post :list_projects end end diff --git a/db/migrate/20191004080818_add_productivity_analytics_start_date.rb b/db/migrate/20191004080818_add_productivity_analytics_start_date.rb new file mode 100644 index 00000000000..287b0755bc1 --- /dev/null +++ b/db/migrate/20191004080818_add_productivity_analytics_start_date.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddProductivityAnalyticsStartDate < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :application_settings, :productivity_analytics_start_date, :datetime_with_timezone + end +end diff --git a/db/migrate/20191004081520_fill_productivity_analytics_start_date.rb b/db/migrate/20191004081520_fill_productivity_analytics_start_date.rb new file mode 100644 index 00000000000..9432cd68708 --- /dev/null +++ b/db/migrate/20191004081520_fill_productivity_analytics_start_date.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +# Expected migration duration: 1 minute +class FillProductivityAnalyticsStartDate < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :merge_request_metrics, :merged_at, + where: "merged_at > '2019-09-01' AND commits_count IS NOT NULL", + name: 'fill_productivity_analytics_start_date_tmp_index' + + execute( + <<SQL + UPDATE application_settings + SET productivity_analytics_start_date = COALESCE((SELECT MIN(merged_at) FROM merge_request_metrics + WHERE merged_at > '2019-09-01' AND commits_count IS NOT NULL), NOW()) +SQL + ) + + remove_concurrent_index :merge_request_metrics, :merged_at, + name: 'fill_productivity_analytics_start_date_tmp_index' + end + + def down + execute('UPDATE application_settings SET productivity_analytics_start_date = NULL') + end +end diff --git a/db/migrate/20191028184740_rename_snowplow_site_id_to_snowplow_app_id.rb b/db/migrate/20191028184740_rename_snowplow_site_id_to_snowplow_app_id.rb new file mode 100644 index 00000000000..4e3b2da670e --- /dev/null +++ b/db/migrate/20191028184740_rename_snowplow_site_id_to_snowplow_app_id.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class RenameSnowplowSiteIdToSnowplowAppId < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + rename_column_concurrently :application_settings, :snowplow_site_id, :snowplow_app_id + end + + def down + undo_rename_column_concurrently :application_settings, :snowplow_site_id, :snowplow_app_id + end +end diff --git a/db/post_migrate/20191029095537_cleanup_application_settings_snowplow_site_id_rename.rb b/db/post_migrate/20191029095537_cleanup_application_settings_snowplow_site_id_rename.rb new file mode 100644 index 00000000000..83b4a2af2b6 --- /dev/null +++ b/db/post_migrate/20191029095537_cleanup_application_settings_snowplow_site_id_rename.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class CleanupApplicationSettingsSnowplowSiteIdRename < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + cleanup_concurrent_column_rename :application_settings, :snowplow_site_id, :snowplow_app_id + end + + def down + undo_cleanup_concurrent_column_rename :application_settings, :snowplow_site_id, :snowplow_app_id + end +end diff --git a/db/schema.rb b/db/schema.rb index fa12e13f1b4..c0b74841834 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -287,7 +287,6 @@ ActiveRecord::Schema.define(version: 2019_11_05_094625) do t.boolean "hide_third_party_offers", default: false, null: false t.boolean "snowplow_enabled", default: false, null: false t.string "snowplow_collector_hostname" - t.string "snowplow_site_id" t.string "snowplow_cookie_domain" t.boolean "instance_statistics_visibility_private", default: false, null: false t.boolean "web_ide_clientside_preview_enabled", default: false, null: false @@ -350,6 +349,8 @@ ActiveRecord::Schema.define(version: 2019_11_05_094625) do t.string "eks_access_key_id", limit: 128 t.string "encrypted_eks_secret_access_key_iv", limit: 255 t.text "encrypted_eks_secret_access_key" + t.string "snowplow_app_id" + t.datetime_with_timezone "productivity_analytics_start_date" t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id" t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id" t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id" diff --git a/doc/api/settings.md b/doc/api/settings.md index a061309d8d1..16624d56a90 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -319,7 +319,7 @@ are listed in the descriptions of the relevant settings. | `snowplow_collector_hostname` | string | required by: `snowplow_enabled` | The Snowplow collector hostname. (e.g. `snowplow.trx.gitlab.net`) | | `snowplow_cookie_domain` | string | no | The Snowplow cookie domain. (e.g. `.gitlab.com`) | | `snowplow_enabled` | boolean | no | Enable snowplow tracking. | -| `snowplow_site_id` | string | no | The Snowplow site name / application id. (e.g. `gitlab`) | +| `snowplow_app_id` | string | no | The Snowplow site name / application id. (e.g. `gitlab`) | | `snowplow_iglu_registry_url` | string | no | The Snowplow base Iglu Schema Registry URL to use for custom context and self describing events'| | `pendo_url` | string | required by: `pendo_enabled` | The Pendo endpoint url with js snippet. (e.g. `https://cdn.pendo.io/agent/static/your-api-key/pendo.js`) | | `pendo_enabled` | boolean | no | Enable pendo tracking. | diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 9ebc0569127..3134e3120df 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -135,6 +135,7 @@ The following job parameters can be defined inside a `default:` block: - [`before_script`](#before_script-and-after_script) - [`after_script`](#before_script-and-after_script) - [`cache`](#cache) +- [`interruptible`](#interruptible) In the following example, the `ruby:2.5` image is set as the default for all jobs except the `rspec 2.6` job, which uses the `ruby:2.6` image: diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index a18380586a7..47ee81c7080 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -296,9 +296,12 @@ module API end get ':id/merge_requests/:merge_request_iid/commits' do merge_request = find_merge_request_with_access(params[:merge_request_iid]) - commits = ::Kaminari.paginate_array(merge_request.commits) - present paginate(commits), with: Entities::Commit + commits = + paginate(merge_request.merge_request_diff.merge_request_diff_commits) + .map { |commit| Commit.from_hash(commit.to_hash, merge_request.project) } + + present commits, with: Entities::Commit end desc 'Show the merge request changes' do diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 11eece8f862..7bf09fd85e2 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -145,7 +145,7 @@ module API given snowplow_enabled: ->(val) { val } do requires :snowplow_collector_hostname, type: String, desc: 'The Snowplow collector hostname' optional :snowplow_cookie_domain, type: String, desc: 'The Snowplow cookie domain' - optional :snowplow_site_id, type: String, desc: 'The Snowplow site name / application ic' + optional :snowplow_app_id, type: String, desc: 'The Snowplow site name / application id' end optional :pendo_enabled, type: Grape::API::Boolean, desc: 'Enable Pendo tracking' given pendo_enabled: ->(val) { val } do diff --git a/lib/gitlab/ci/config/entry/boolean.rb b/lib/gitlab/ci/config/entry/boolean.rb new file mode 100644 index 00000000000..10619ef9f8d --- /dev/null +++ b/lib/gitlab/ci/config/entry/boolean.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module Entry + ## + # Entry that represents the interrutible value. + # + class Boolean < ::Gitlab::Config::Entry::Node + include ::Gitlab::Config::Entry::Validatable + + validations do + validates :config, boolean: true + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/entry/default.rb b/lib/gitlab/ci/config/entry/default.rb index 78297ff5846..83127bde6e4 100644 --- a/lib/gitlab/ci/config/entry/default.rb +++ b/lib/gitlab/ci/config/entry/default.rb @@ -14,7 +14,7 @@ module Gitlab include ::Gitlab::Config::Entry::Inheritable ALLOWED_KEYS = %i[before_script image services - after_script cache].freeze + after_script cache interruptible].freeze validations do validates :config, allowed_keys: ALLOWED_KEYS @@ -40,7 +40,11 @@ module Gitlab description: 'Configure caching between build jobs.', inherit: true - helpers :before_script, :image, :services, :after_script, :cache + entry :interruptible, Entry::Boolean, + description: 'Set jobs interruptible default value.', + inherit: false + + helpers :before_script, :image, :services, :after_script, :cache, :interruptible private diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 2d5981a4255..ace42ebd872 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -38,7 +38,6 @@ module Gitlab with_options allow_nil: true do validates :tags, array_of_strings: true validates :allow_failure, boolean: true - validates :interruptible, boolean: true validates :parallel, numericality: { only_integer: true, greater_than_or_equal_to: 2, less_than_or_equal_to: 50 } @@ -100,6 +99,10 @@ module Gitlab description: 'Services that will be used to execute this job.', inherit: true + entry :interruptible, Entry::Boolean, + description: 'Set jobs interruptible value.', + inherit: true + entry :only, Entry::Policy, description: 'Refs policy this job will be executed for.', default: Entry::Policy::DEFAULT_ONLY, diff --git a/lib/gitlab/error_tracking/detailed_error.rb b/lib/gitlab/error_tracking/detailed_error.rb new file mode 100644 index 00000000000..225280a42f4 --- /dev/null +++ b/lib/gitlab/error_tracking/detailed_error.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Gitlab + module ErrorTracking + class DetailedError + include ActiveModel::Model + + attr_accessor :count, + :culprit, + :external_base_url, + :external_url, + :first_release_last_commit, + :first_release_short_version, + :first_seen, + :frequency, + :id, + :last_release_last_commit, + :last_release_short_version, + :last_seen, + :message, + :project_id, + :project_name, + :project_slug, + :short_id, + :status, + :title, + :type, + :user_count + end + end +end diff --git a/lib/gitlab/error_tracking/error_event.rb b/lib/gitlab/error_tracking/error_event.rb new file mode 100644 index 00000000000..c6e0d82f868 --- /dev/null +++ b/lib/gitlab/error_tracking/error_event.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Gitlab + module ErrorTracking + class ErrorEvent + include ActiveModel::Model + + attr_accessor :issue_id, :date_received, :stack_trace_entries + end + end +end diff --git a/lib/gitlab/tracking.rb b/lib/gitlab/tracking.rb index af6b13b0d36..91e2ff0b10d 100644 --- a/lib/gitlab/tracking.rb +++ b/lib/gitlab/tracking.rb @@ -45,7 +45,7 @@ module Gitlab namespace: SNOWPLOW_NAMESPACE, hostname: Gitlab::CurrentSettings.snowplow_collector_hostname, cookie_domain: Gitlab::CurrentSettings.snowplow_cookie_domain, - app_id: Gitlab::CurrentSettings.snowplow_site_id, + app_id: Gitlab::CurrentSettings.snowplow_app_id, form_tracking: additional_features, link_click_tracking: additional_features, iglu_registry_url: Gitlab::CurrentSettings.snowplow_iglu_registry_url @@ -59,7 +59,7 @@ module Gitlab SnowplowTracker::AsyncEmitter.new(Gitlab::CurrentSettings.snowplow_collector_hostname, protocol: 'https'), SnowplowTracker::Subject.new, SNOWPLOW_NAMESPACE, - Gitlab::CurrentSettings.snowplow_site_id + Gitlab::CurrentSettings.snowplow_app_id ) end end diff --git a/lib/sentry/client.rb b/lib/sentry/client.rb index 07cca1c8d1e..f16df0dde4f 100644 --- a/lib/sentry/client.rb +++ b/lib/sentry/client.rb @@ -12,6 +12,18 @@ module Sentry @token = token end + def issue_details(issue_id:) + issue = get_issue(issue_id: issue_id) + + map_to_detailed_error(issue) + end + + def issue_latest_event(issue_id:) + latest_event = get_issue_latest_event(issue_id: issue_id) + + map_to_event(latest_event) + end + def list_issues(issue_status:, limit:) issues = get_issues(issue_status: issue_status, limit: limit) @@ -61,6 +73,14 @@ module Sentry }) end + def get_issue(issue_id:) + http_get(issue_api_url(issue_id)) + end + + def get_issue_latest_event(issue_id:) + http_get(issue_latest_event_api_url(issue_id)) + end + def get_projects http_get(projects_api_url) end @@ -102,6 +122,20 @@ module Sentry projects_url end + def issue_api_url(issue_id) + issue_url = URI(@url) + issue_url.path = "/api/0/issues/#{issue_id}/" + + issue_url + end + + def issue_latest_event_api_url(issue_id) + latest_event_url = URI(@url) + latest_event_url.path = "/api/0/issues/#{issue_id}/events/latest/" + + latest_event_url + end + def issues_api_url issues_url = URI(@url + '/issues/') issues_url.path.squeeze!('/') @@ -119,38 +153,87 @@ module Sentry def issue_url(id) issues_url = @url + "/issues/#{id}" - issues_url = ErrorTracking::ProjectErrorTrackingSetting.extract_sentry_external_url(issues_url) - uri = URI(issues_url) + parse_sentry_url(issues_url) + end + + def project_url + parse_sentry_url(@url) + end + + def parse_sentry_url(api_url) + url = ErrorTracking::ProjectErrorTrackingSetting.extract_sentry_external_url(api_url) + + uri = URI(url) uri.path.squeeze!('/') + # Remove trailing spaces + uri = uri.to_s.gsub(/\/\z/, '') - uri.to_s + uri end - def map_to_error(issue) - id = issue.fetch('id') + def map_to_event(event) + stack_trace = parse_stack_trace(event) + + Gitlab::ErrorTracking::ErrorEvent.new( + issue_id: event.dig('groupID'), + date_received: event.dig('dateReceived'), + stack_trace_entries: stack_trace + ) + end - count = issue.fetch('count', nil) + def parse_stack_trace(event) + exception_entry = event.dig('entries')&.detect { |h| h['type'] == 'exception' } + return unless exception_entry - frequency = issue.dig('stats', '24h') - message = issue.dig('metadata', 'value') + exception_values = exception_entry.dig('data', 'values') + stack_trace_entry = exception_values&.detect { |h| h['stacktrace'].present? } + return unless stack_trace_entry - external_url = issue_url(id) + stack_trace_entry.dig('stacktrace', 'frames') + end + + def map_to_detailed_error(issue) + Gitlab::ErrorTracking::DetailedError.new( + id: issue.fetch('id'), + first_seen: issue.fetch('firstSeen', nil), + last_seen: issue.fetch('lastSeen', nil), + title: issue.fetch('title', nil), + type: issue.fetch('type', nil), + user_count: issue.fetch('userCount', nil), + count: issue.fetch('count', nil), + message: issue.dig('metadata', 'value'), + culprit: issue.fetch('culprit', nil), + external_url: issue_url(issue.fetch('id')), + external_base_url: project_url, + short_id: issue.fetch('shortId', nil), + status: issue.fetch('status', nil), + frequency: issue.dig('stats', '24h'), + project_id: issue.dig('project', 'id'), + project_name: issue.dig('project', 'name'), + project_slug: issue.dig('project', 'slug'), + first_release_last_commit: issue.dig('firstRelease', 'lastCommit'), + last_release_last_commit: issue.dig('lastRelease', 'lastCommit'), + first_release_short_version: issue.dig('firstRelease', 'shortVersion'), + last_release_short_version: issue.dig('lastRelease', 'shortVersion') + ) + end + def map_to_error(issue) Gitlab::ErrorTracking::Error.new( - id: id, + id: issue.fetch('id'), first_seen: issue.fetch('firstSeen', nil), last_seen: issue.fetch('lastSeen', nil), title: issue.fetch('title', nil), type: issue.fetch('type', nil), user_count: issue.fetch('userCount', nil), - count: count, - message: message, + count: issue.fetch('count', nil), + message: issue.dig('metadata', 'value'), culprit: issue.fetch('culprit', nil), - external_url: external_url, + external_url: issue_url(issue.fetch('id')), short_id: issue.fetch('shortId', nil), status: issue.fetch('status', nil), - frequency: frequency, + frequency: issue.dig('stats', '24h'), project_id: issue.dig('project', 'id'), project_name: issue.dig('project', 'name'), project_slug: issue.dig('project', 'slug') diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3f9e0e7805b..a45e513d9b2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1798,6 +1798,9 @@ msgstr "" msgid "Any user" msgstr "" +msgid "App ID" +msgstr "" + msgid "Appearance" msgstr "" @@ -6406,12 +6409,18 @@ msgstr "" msgid "Environments|Job" msgstr "" +msgid "Environments|Learn about environments" +msgstr "" + msgid "Environments|Learn more about stopping environments" msgstr "" msgid "Environments|New environment" msgstr "" +msgid "Environments|No deployed environments" +msgstr "" + msgid "Environments|No deployments yet" msgstr "" @@ -6580,6 +6589,9 @@ msgstr "" msgid "Error" msgstr "" +msgid "Error Details" +msgstr "" + msgid "Error Tracking" msgstr "" @@ -10177,6 +10189,9 @@ msgstr "" msgid "Logs" msgstr "" +msgid "Logs|To see the pod logs, deploy your code to an environment." +msgstr "" + msgid "MD5" msgstr "" @@ -10678,9 +10693,6 @@ msgstr "" msgid "Metrics|Label of the y-axis (usually the unit). The x-axis always represents time." msgstr "" -msgid "Metrics|Learn about environments" -msgstr "" - msgid "Metrics|Legend label (optional)" msgstr "" @@ -10696,9 +10708,6 @@ msgstr "" msgid "Metrics|New metric" msgstr "" -msgid "Metrics|No deployed environments" -msgstr "" - msgid "Metrics|PromQL query is valid" msgstr "" @@ -12355,6 +12364,9 @@ msgstr "" msgid "Please wait while we import the repository for you. Refresh at will." msgstr "" +msgid "Pod logs" +msgstr "" + msgid "Pod not found" msgstr "" @@ -15599,9 +15611,6 @@ msgstr "" msgid "Single or combined queries" msgstr "" -msgid "Site ID" -msgstr "" - msgid "Size" msgstr "" diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 53896c7f5c7..ca39f5dd9f2 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -90,14 +90,6 @@ describe ApplicationController do let(:format) { :html } it_behaves_like 'setting gon variables' - - context 'for peek requests' do - before do - request.path = '/-/peek' - end - - it_behaves_like 'not setting gon variables' - end end context 'with json format' do @@ -105,6 +97,12 @@ describe ApplicationController do it_behaves_like 'not setting gon variables' end + + context 'with atom format' do + let(:format) { :atom } + + it_behaves_like 'not setting gon variables' + end end describe 'session expiration' do diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 3fe5ff5feee..7bb956201fd 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -330,11 +330,11 @@ describe Projects::EnvironmentsController do expect(response).to redirect_to(environment_metrics_path(environment)) end - it 'redirects to empty page if no environment exists' do + it 'redirects to empty metrics page if no environment exists' do get :metrics_redirect, params: { namespace_id: project.namespace, project_id: project } expect(response).to be_ok - expect(response).to render_template 'empty' + expect(response).to render_template 'empty_metrics' end end diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb index 31868f5f717..8155d6ddafe 100644 --- a/spec/controllers/projects/error_tracking_controller_spec.rb +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -46,17 +46,6 @@ describe Projects::ErrorTrackingController do end describe 'format json' do - shared_examples 'no data' do - it 'returns no data' do - get :index, params: project_params(format: :json) - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('error_tracking/index') - expect(json_response['external_url']).to be_nil - expect(json_response['errors']).to eq([]) - end - end - let(:list_issues_service) { spy(:list_issues_service) } let(:external_url) { 'http://example.com' } @@ -66,6 +55,19 @@ describe Projects::ErrorTrackingController do .and_return(list_issues_service) end + context 'no data' do + before do + expect(list_issues_service).to receive(:execute) + .and_return(status: :error, http_status: :no_content) + end + + it 'returns no data' do + get :index, params: project_params(format: :json) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + context 'service result is successful' do before do expect(list_issues_service).to receive(:execute) @@ -232,8 +234,186 @@ describe Projects::ErrorTrackingController do end end + describe 'GET #issue_details' do + let_it_be(:issue_id) { 1234 } + + let(:issue_details_service) { spy(:issue_details_service) } + + let(:permitted_params) do + ActionController::Parameters.new( + { issue_id: issue_id.to_s } + ).permit! + end + + before do + expect(ErrorTracking::IssueDetailsService) + .to receive(:new).with(project, user, permitted_params) + .and_return(issue_details_service) + end + + describe 'format json' do + context 'no data' do + before do + expect(issue_details_service).to receive(:execute) + .and_return(status: :error, http_status: :no_content) + end + + it 'returns no data' do + get :details, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'service result is successful' do + before do + expect(issue_details_service).to receive(:execute) + .and_return(status: :success, issue: error) + end + + let(:error) { build(:detailed_error_tracking_error) } + + it 'returns an error' do + get :details, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('error_tracking/issue_detailed') + expect(json_response['error']).to eq(error.as_json) + end + end + + context 'service result is erroneous' do + let(:error_message) { 'error message' } + + context 'without http_status' do + before do + expect(issue_details_service).to receive(:execute) + .and_return(status: :error, message: error_message) + end + + it 'returns 400 with message' do + get :details, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(error_message) + end + end + + context 'with explicit http_status' do + let(:http_status) { :no_content } + + before do + expect(issue_details_service).to receive(:execute).and_return( + status: :error, + message: error_message, + http_status: http_status + ) + end + + it 'returns http_status with message' do + get :details, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(http_status) + expect(json_response['message']).to eq(error_message) + end + end + end + end + end + + describe 'GET #stack_trace' do + let_it_be(:issue_id) { 1234 } + + let(:issue_stack_trace_service) { spy(:issue_stack_trace_service) } + + let(:permitted_params) do + ActionController::Parameters.new( + { issue_id: issue_id.to_s } + ).permit! + end + + before do + expect(ErrorTracking::IssueLatestEventService) + .to receive(:new).with(project, user, permitted_params) + .and_return(issue_stack_trace_service) + end + + describe 'format json' do + context 'awaiting data' do + before do + expect(issue_stack_trace_service).to receive(:execute) + .and_return(status: :error, http_status: :no_content) + end + + it 'returns no data' do + get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'service result is successful' do + before do + expect(issue_stack_trace_service).to receive(:execute) + .and_return(status: :success, latest_event: error_event) + end + + let(:error_event) { build(:error_tracking_error_event) } + + it 'returns an error' do + get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('error_tracking/issue_stack_trace') + expect(json_response['error']).to eq(error_event.as_json) + end + end + + context 'service result is erroneous' do + let(:error_message) { 'error message' } + + context 'without http_status' do + before do + expect(issue_stack_trace_service).to receive(:execute) + .and_return(status: :error, message: error_message) + end + + it 'returns 400 with message' do + get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(error_message) + end + end + + context 'with explicit http_status' do + let(:http_status) { :no_content } + + before do + expect(issue_stack_trace_service).to receive(:execute).and_return( + status: :error, + message: error_message, + http_status: http_status + ) + end + + it 'returns http_status with message' do + get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(http_status) + expect(json_response['message']).to eq(error_message) + end + end + end + end + end + private + def issue_params(opts = {}) + project_params.reverse_merge(opts) + end + def project_params(opts = {}) opts.reverse_merge(namespace_id: project.namespace, project_id: project) end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index 1bcf3bb106b..f35babc1b56 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -228,10 +228,10 @@ describe UploadsController do user.block end - it "redirects to the sign in page" do + it "responds with status 401" do get :show, params: { model: "user", mounted_as: "avatar", id: user.id, filename: "dk.png" } - expect(response).to redirect_to(new_user_session_path) + expect(response).to have_gitlab_http_status(401) end end @@ -320,10 +320,10 @@ describe UploadsController do end context "when not signed in" do - it "redirects to the sign in page" do + it "responds with status 401" do get :show, params: { model: "project", mounted_as: "avatar", id: project.id, filename: "dk.png" } - expect(response).to redirect_to(new_user_session_path) + expect(response).to have_gitlab_http_status(401) end end @@ -343,10 +343,10 @@ describe UploadsController do project.add_maintainer(user) end - it "redirects to the sign in page" do + it "responds with status 401" do get :show, params: { model: "project", mounted_as: "avatar", id: project.id, filename: "dk.png" } - expect(response).to redirect_to(new_user_session_path) + expect(response).to have_gitlab_http_status(401) end end @@ -439,10 +439,10 @@ describe UploadsController do user.block end - it "redirects to the sign in page" do + it "responds with status 401" do get :show, params: { model: "group", mounted_as: "avatar", id: group.id, filename: "dk.png" } - expect(response).to redirect_to(new_user_session_path) + expect(response).to have_gitlab_http_status(401) end end @@ -526,10 +526,10 @@ describe UploadsController do end context "when not signed in" do - it "redirects to the sign in page" do + it "responds with status 401" do get :show, params: { model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png" } - expect(response).to redirect_to(new_user_session_path) + expect(response).to have_gitlab_http_status(401) end end @@ -549,10 +549,10 @@ describe UploadsController do project.add_maintainer(user) end - it "redirects to the sign in page" do + it "responds with status 401" do get :show, params: { model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png" } - expect(response).to redirect_to(new_user_session_path) + expect(response).to have_gitlab_http_status(401) end end diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index d340cec8b70..e8b30868801 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -13,7 +13,7 @@ describe 'Database schema' do # EE: edit the ee/spec/db/schema_support.rb IGNORED_FK_COLUMNS = { abuse_reports: %w[reporter_id user_id], - application_settings: %w[performance_bar_allowed_group_id slack_app_id snowplow_site_id eks_account_id eks_access_key_id], + application_settings: %w[performance_bar_allowed_group_id slack_app_id snowplow_app_id eks_account_id eks_access_key_id], approvers: %w[target_id user_id], approvals: %w[user_id], approver_groups: %w[target_id], diff --git a/spec/factories/error_tracking/detailed_error.rb b/spec/factories/error_tracking/detailed_error.rb new file mode 100644 index 00000000000..cf7de2ece96 --- /dev/null +++ b/spec/factories/error_tracking/detailed_error.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :detailed_error_tracking_error, class: Gitlab::ErrorTracking::DetailedError do + id { 'id' } + title { 'title' } + type { 'error' } + user_count { 1 } + count { 2 } + first_seen { Time.now } + last_seen { Time.now } + message { 'message' } + culprit { 'culprit' } + external_url { 'http://example.com/id' } + external_base_url { 'http://example.com' } + project_id { 'project1' } + project_name { 'project name' } + project_slug { 'project_name' } + short_id { 'ID' } + status { 'unresolved' } + frequency { [] } + first_release_last_commit { '68c914da9' } + last_release_last_commit { '9ad419c86' } + first_release_short_version { 'abc123' } + last_release_short_version { 'abc123' } + + skip_create + end +end diff --git a/spec/factories/error_tracking/error_event.rb b/spec/factories/error_tracking/error_event.rb new file mode 100644 index 00000000000..44c127e7bf5 --- /dev/null +++ b/spec/factories/error_tracking/error_event.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :error_tracking_error_event, class: Gitlab::ErrorTracking::ErrorEvent do + issue_id { 'id' } + date_received { Time.now.iso8601 } + stack_trace_entries do + { + 'stacktrace' => + { + 'frames' => [{ 'file' => 'test.rb' }] + } + } + end + + skip_create + end +end diff --git a/spec/features/projects/members/member_leaves_project_spec.rb b/spec/features/projects/members/member_leaves_project_spec.rb index fb1165838c7..cb7a405e821 100644 --- a/spec/features/projects/members/member_leaves_project_spec.rb +++ b/spec/features/projects/members/member_leaves_project_spec.rb @@ -20,7 +20,7 @@ describe 'Projects > Members > Member leaves project' do expect(project.users.exists?(user.id)).to be_falsey end - it 'user leaves project by url param', :js do + it 'user leaves project by url param', :js, :quarantine do visit project_path(project, leave: 1) page.accept_confirm diff --git a/spec/fixtures/api/schemas/error_tracking/error.json b/spec/fixtures/api/schemas/error_tracking/error.json index df2c02d7d5d..3f65105681e 100644 --- a/spec/fixtures/api/schemas/error_tracking/error.json +++ b/spec/fixtures/api/schemas/error_tracking/error.json @@ -4,7 +4,14 @@ "external_url", "last_seen", "message", - "type" + "type", + "title", + "project_id", + "project_name", + "project_slug", + "short_id", + "status", + "frequency" ], "properties" : { "id": { "type": "string"}, @@ -15,7 +22,14 @@ "culprit": { "type": "string" }, "count": { "type": "integer"}, "external_url": { "type": "string" }, - "user_count": { "type": "integer"} + "user_count": { "type": "integer"}, + "title": { "type": "string"}, + "project_id": { "type": "string"}, + "project_name": { "type": "string"}, + "project_slug": { "type": "string"}, + "short_id": { "type": "string"}, + "status": { "type": "string"}, + "frequency": { "type": "array"} }, - "additionalProperties": true + "additionalProperties": false } diff --git a/spec/fixtures/api/schemas/error_tracking/error_detailed.json b/spec/fixtures/api/schemas/error_tracking/error_detailed.json new file mode 100644 index 00000000000..40d6773f0e6 --- /dev/null +++ b/spec/fixtures/api/schemas/error_tracking/error_detailed.json @@ -0,0 +1,45 @@ +{ + "type": "object", + "required" : [ + "external_url", + "external_base_url", + "last_seen", + "message", + "type", + "title", + "project_id", + "project_name", + "project_slug", + "short_id", + "status", + "frequency", + "first_release_last_commit", + "last_release_last_commit", + "first_release_short_version", + "last_release_short_version" + ], + "properties" : { + "id": { "type": "string"}, + "first_seen": { "type": "string", "format": "date-time" }, + "last_seen": { "type": "string", "format": "date-time" }, + "type": { "type": "string" }, + "message": { "type": "string" }, + "culprit": { "type": "string" }, + "count": { "type": "integer"}, + "external_url": { "type": "string" }, + "external_base_url": { "type": "string" }, + "user_count": { "type": "integer"}, + "title": { "type": "string"}, + "project_id": { "type": "string"}, + "project_name": { "type": "string"}, + "project_slug": { "type": "string"}, + "short_id": { "type": "string"}, + "status": { "type": "string"}, + "frequency": { "type": "array"}, + "first_release_last_commit": { "type": ["string", "null"] }, + "last_release_last_commit": { "type": ["string", "null"] }, + "first_release_short_version": { "type": ["string", "null"] }, + "last_release_short_version": { "type": ["string", "null"] } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/error_tracking/error_stack_trace.json b/spec/fixtures/api/schemas/error_tracking/error_stack_trace.json new file mode 100644 index 00000000000..a684dd0496a --- /dev/null +++ b/spec/fixtures/api/schemas/error_tracking/error_stack_trace.json @@ -0,0 +1,14 @@ +{ + "type": "object", + "required": [ + "issue_id", + "stack_trace_entries", + "date_received" + ], + "properties": { + "issue_id": { "type": ["string", "integer"] }, + "stack_trace_entries": { "type": "object" }, + "date_received": { "type": "string" } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/error_tracking/issue_detailed.json b/spec/fixtures/api/schemas/error_tracking/issue_detailed.json new file mode 100644 index 00000000000..b5adea6fc62 --- /dev/null +++ b/spec/fixtures/api/schemas/error_tracking/issue_detailed.json @@ -0,0 +1,11 @@ +{ + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { "$ref": "error_detailed.json" } + }, + "additionalProperties": false +} + diff --git a/spec/fixtures/api/schemas/error_tracking/issue_stack_trace.json b/spec/fixtures/api/schemas/error_tracking/issue_stack_trace.json new file mode 100644 index 00000000000..7ec1ae63609 --- /dev/null +++ b/spec/fixtures/api/schemas/error_tracking/issue_stack_trace.json @@ -0,0 +1,11 @@ +{ + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { "$ref": "error_stack_trace.json" } + }, + "additionalProperties": false +} + diff --git a/spec/helpers/application_settings_helper_spec.rb b/spec/helpers/application_settings_helper_spec.rb index e466150fa46..a5e8370a715 100644 --- a/spec/helpers/application_settings_helper_spec.rb +++ b/spec/helpers/application_settings_helper_spec.rb @@ -38,7 +38,7 @@ describe ApplicationSettingsHelper do it_behaves_like 'when HTTP protocol is in use', 'http' context 'with tracking parameters' do - it { expect(visible_attributes).to include(*%i(snowplow_collector_hostname snowplow_cookie_domain snowplow_enabled snowplow_site_id)) } + it { expect(visible_attributes).to include(*%i(snowplow_collector_hostname snowplow_cookie_domain snowplow_enabled snowplow_app_id)) } it { expect(visible_attributes).to include(*%i(pendo_enabled pendo_url)) } end diff --git a/spec/lib/gitlab/ci/config/entry/default_spec.rb b/spec/lib/gitlab/ci/config/entry/default_spec.rb index f09df698f68..a0856037340 100644 --- a/spec/lib/gitlab/ci/config/entry/default_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/default_spec.rb @@ -26,7 +26,7 @@ describe Gitlab::Ci::Config::Entry::Default do it 'contains the expected node names' do expect(described_class.nodes.keys) .to match_array(%i[before_script image services - after_script cache]) + after_script cache interruptible]) end end end diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 9fe18caf689..fe83171c57a 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -24,7 +24,7 @@ describe Gitlab::Ci::Config::Entry::Job do let(:result) do %i[before_script script stage type after_script cache image services only except rules needs variables artifacts - environment coverage retry] + environment coverage retry interruptible] end it { is_expected.to match_array result } diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index dc7bbc519ee..5a173470dfe 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -108,6 +108,25 @@ module Gitlab it { expect(subject[:interruptible]).to be_falsy } end + + it "returns interruptible when overridden for job" do + config = YAML.dump({ default: { interruptible: true }, + rspec: { script: "rspec" } }) + + config_processor = Gitlab::Ci::YamlProcessor.new(config) + + expect(config_processor.stage_builds_attributes("test").size).to eq(1) + expect(config_processor.stage_builds_attributes("test").first).to eq({ + stage: "test", + stage_idx: 2, + name: "rspec", + options: { script: ["rspec"] }, + interruptible: true, + allow_failure: false, + when: "on_success", + yaml_variables: [] + }) + end end describe 'retry entry' do diff --git a/spec/lib/gitlab/tracking_spec.rb b/spec/lib/gitlab/tracking_spec.rb index 0bb9d6c331f..dc877f20cae 100644 --- a/spec/lib/gitlab/tracking_spec.rb +++ b/spec/lib/gitlab/tracking_spec.rb @@ -8,7 +8,7 @@ describe Gitlab::Tracking do stub_application_setting(snowplow_enabled: true) stub_application_setting(snowplow_collector_hostname: 'gitfoo.com') stub_application_setting(snowplow_cookie_domain: '.gitfoo.com') - stub_application_setting(snowplow_site_id: '_abc123_') + stub_application_setting(snowplow_app_id: '_abc123_') stub_application_setting(snowplow_iglu_registry_url: 'https://example.org') end diff --git a/spec/migrations/fill_productivity_analytics_start_date_spec.rb b/spec/migrations/fill_productivity_analytics_start_date_spec.rb new file mode 100644 index 00000000000..7cbba9ef20e --- /dev/null +++ b/spec/migrations/fill_productivity_analytics_start_date_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20191004081520_fill_productivity_analytics_start_date.rb') + +describe FillProductivityAnalyticsStartDate, :migration do + let(:settings_table) { table('application_settings') } + let(:metrics_table) { table('merge_request_metrics') } + + before do + settings_table.create! + end + + context 'with NO productivity analytics data available' do + it 'sets start_date to NOW' do + expect { migrate! }.to change { + settings_table.first&.productivity_analytics_start_date + }.to(be_like_time(Time.now)) + end + end + + context 'with productivity analytics data available' do + before do + ActiveRecord::Base.transaction do + ActiveRecord::Base.connection.execute('ALTER TABLE merge_request_metrics DISABLE TRIGGER ALL') + metrics_table.create!(merged_at: Time.parse('2019-09-09'), commits_count: nil, merge_request_id: 3) + metrics_table.create!(merged_at: Time.parse('2019-10-10'), commits_count: 5, merge_request_id: 1) + metrics_table.create!(merged_at: Time.parse('2019-11-11'), commits_count: 10, merge_request_id: 2) + ActiveRecord::Base.connection.execute('ALTER TABLE merge_request_metrics ENABLE TRIGGER ALL') + end + end + + it 'set start_date to earliest merged_at value with PA data available' do + expect { migrate! }.to change { + settings_table.first&.productivity_analytics_start_date + }.to(be_like_time(Time.parse('2019-10-10'))) + end + end +end diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 5aba798f2c2..1b58fb1dab1 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -178,7 +178,7 @@ describe API::Settings, 'Settings' do snowplow_collector_hostname: "snowplow.example.com", snowplow_cookie_domain: ".example.com", snowplow_enabled: true, - snowplow_site_id: "site_id", + snowplow_app_id: "app_id", snowplow_iglu_registry_url: 'https://example.com' } end diff --git a/spec/requests/user_avatar_spec.rb b/spec/requests/user_avatar_spec.rb new file mode 100644 index 00000000000..9451674161c --- /dev/null +++ b/spec/requests/user_avatar_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Loading a user avatar' do + let(:user) { create(:user, :with_avatar) } + + context 'when logged in' do + # The exact query count will vary depending on the 2FA settings of the + # instance, group, and user. Removing those extra 2FA queries in this case + # may not be a good idea, so we just set up the ideal case. + before do + stub_application_setting(require_two_factor_authentication: true) + + login_as(create(:user, :two_factor)) + end + + # One each for: current user, avatar user, and upload record + it 'only performs three SQL queries' do + get user.avatar_url # Skip queries on first application load + + expect(response).to have_gitlab_http_status(200) + expect { get user.avatar_url }.not_to exceed_query_limit(3) + end + end + + context 'when logged out' do + # One each for avatar user and upload record + it 'only performs two SQL queries' do + get user.avatar_url # Skip queries on first application load + + expect(response).to have_gitlab_http_status(200) + expect { get user.avatar_url }.not_to exceed_query_limit(2) + end + end +end diff --git a/spec/services/error_tracking/issue_details_service_spec.rb b/spec/services/error_tracking/issue_details_service_spec.rb new file mode 100644 index 00000000000..4d5505bb5a9 --- /dev/null +++ b/spec/services/error_tracking/issue_details_service_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ErrorTracking::IssueDetailsService do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + let(:token) { 'test-token' } + let(:result) { subject.execute } + + let(:error_tracking_setting) do + create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project) + end + + subject { described_class.new(project, user) } + + before do + expect(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting) + + project.add_reporter(user) + end + + describe '#execute' do + context 'with authorized user' do + context 'when issue_details returns a detailed error' do + let(:detailed_error) { build(:detailed_error_tracking_error) } + + before do + expect(error_tracking_setting) + .to receive(:issue_details).and_return(issue: detailed_error) + end + + it 'returns the detailed error' do + expect(result).to eq(status: :success, issue: detailed_error) + end + end + + include_examples 'error tracking service data not ready', :issue_details + include_examples 'error tracking service sentry error handling', :issue_details + include_examples 'error tracking service http status handling', :issue_details + end + + include_examples 'error tracking service unauthorized user' + include_examples 'error tracking service disabled' + end +end diff --git a/spec/services/error_tracking/issue_latest_event_service_spec.rb b/spec/services/error_tracking/issue_latest_event_service_spec.rb new file mode 100644 index 00000000000..cda15042814 --- /dev/null +++ b/spec/services/error_tracking/issue_latest_event_service_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ErrorTracking::IssueLatestEventService do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + let(:token) { 'test-token' } + let(:result) { subject.execute } + + let(:error_tracking_setting) do + create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project) + end + + subject { described_class.new(project, user) } + + before do + expect(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting) + + project.add_reporter(user) + end + + describe '#execute' do + context 'with authorized user' do + context 'when issue_latest_event returns an error event' do + let(:error_event) { build(:error_tracking_error_event) } + + before do + expect(error_tracking_setting) + .to receive(:issue_latest_event).and_return(latest_event: error_event) + end + + it 'returns the error event' do + expect(result).to eq(status: :success, latest_event: error_event) + end + end + + include_examples 'error tracking service data not ready', :issue_latest_event + include_examples 'error tracking service sentry error handling', :issue_latest_event + include_examples 'error tracking service http status handling', :issue_latest_event + end + + include_examples 'error tracking service unauthorized user' + include_examples 'error tracking service disabled' + end +end diff --git a/spec/services/error_tracking/list_issues_service_spec.rb b/spec/services/error_tracking/list_issues_service_spec.rb index 3a8f3069911..fce790f708d 100644 --- a/spec/services/error_tracking/list_issues_service_spec.rb +++ b/spec/services/error_tracking/list_issues_service_spec.rb @@ -37,93 +37,12 @@ describe ErrorTracking::ListIssuesService do end end - context 'when list_sentry_issues returns nil' do - before do - expect(error_tracking_setting) - .to receive(:list_sentry_issues).and_return(nil) - end - - it 'result is not ready' do - expect(result).to eq( - status: :error, http_status: :no_content, message: 'Not ready. Try again later') - end - end - - context 'when list_sentry_issues returns error' do - before do - allow(error_tracking_setting) - .to receive(:list_sentry_issues) - .and_return( - error: 'Sentry response status code: 401', - error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_NON_20X_RESPONSE - ) - end - - it 'returns the error' do - expect(result).to eq( - status: :error, - http_status: :bad_request, - message: 'Sentry response status code: 401' - ) - end - end - - context 'when list_sentry_issues returns error with http_status' do - before do - allow(error_tracking_setting) - .to receive(:list_sentry_issues) - .and_return( - error: 'Sentry API response is missing keys. key not found: "id"', - error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_MISSING_KEYS - ) - end - - it 'returns the error with correct http_status' do - expect(result).to eq( - status: :error, - http_status: :internal_server_error, - message: 'Sentry API response is missing keys. key not found: "id"' - ) - end - end + include_examples 'error tracking service data not ready', :list_sentry_issues + include_examples 'error tracking service sentry error handling', :list_sentry_issues + include_examples 'error tracking service http status handling', :list_sentry_issues end - context 'with unauthorized user' do - let(:unauthorized_user) { create(:user) } - - subject { described_class.new(project, unauthorized_user) } - - it 'returns error' do - result = subject.execute - - expect(result).to include( - status: :error, - message: 'Access denied', - http_status: :unauthorized - ) - end - end - - context 'with error tracking disabled' do - before do - error_tracking_setting.enabled = false - end - - it 'raises error' do - result = subject.execute - - expect(result).to include(status: :error, message: 'Error Tracking is not enabled') - end - end - end - - describe '#sentry_external_url' do - let(:external_url) { 'https://sentrytest.gitlab.com/sentry-org/sentry-project' } - - it 'calls ErrorTracking::ProjectErrorTrackingSetting' do - expect(error_tracking_setting).to receive(:sentry_external_url).and_call_original - - subject.external_url - end + include_examples 'error tracking service unauthorized user' + include_examples 'error tracking service disabled' end end diff --git a/spec/services/error_tracking/list_projects_service_spec.rb b/spec/services/error_tracking/list_projects_service_spec.rb index a272a604184..cd4b835e097 100644 --- a/spec/services/error_tracking/list_projects_service_spec.rb +++ b/spec/services/error_tracking/list_projects_service_spec.rb @@ -127,7 +127,7 @@ describe ErrorTracking::ListProjectsService do end it 'returns error' do - expect(result).to include(status: :error, message: 'access denied') + expect(result).to include(status: :error, message: 'Access denied', http_status: :unauthorized) end end diff --git a/spec/support/shared_examples/services/error_tracking_service_shared_examples.rb b/spec/support/shared_examples/services/error_tracking_service_shared_examples.rb new file mode 100644 index 00000000000..83c6d89e560 --- /dev/null +++ b/spec/support/shared_examples/services/error_tracking_service_shared_examples.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +shared_examples 'error tracking service data not ready' do |service_call| + context "when #{service_call} returns nil" do + before do + expect(error_tracking_setting) + .to receive(service_call).and_return(nil) + end + + it 'result is not ready' do + expect(result).to eq( + status: :error, http_status: :no_content, message: 'Not ready. Try again later') + end + end +end + +shared_examples 'error tracking service sentry error handling' do |service_call| + context "when #{service_call} returns error" do + before do + allow(error_tracking_setting) + .to receive(service_call) + .and_return( + error: 'Sentry response status code: 401', + error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_NON_20X_RESPONSE + ) + end + + it 'returns the error' do + expect(result).to eq( + status: :error, + http_status: :bad_request, + message: 'Sentry response status code: 401' + ) + end + end +end + +shared_examples 'error tracking service http status handling' do |service_call| + context "when #{service_call} returns error with http_status" do + before do + allow(error_tracking_setting) + .to receive(service_call) + .and_return( + error: 'Sentry API response is missing keys. key not found: "id"', + error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_MISSING_KEYS + ) + end + + it 'returns the error with correct http_status' do + expect(result).to eq( + status: :error, + http_status: :internal_server_error, + message: 'Sentry API response is missing keys. key not found: "id"' + ) + end + end +end + +shared_examples 'error tracking service unauthorized user' do + context 'with unauthorized user' do + let(:unauthorized_user) { create(:user) } + + subject { described_class.new(project, unauthorized_user) } + + it 'returns error' do + result = subject.execute + + expect(result).to include( + status: :error, + message: 'Access denied', + http_status: :unauthorized + ) + end + end +end + +shared_examples 'error tracking service disabled' do + context 'with error tracking disabled' do + before do + error_tracking_setting.enabled = false + end + + it 'raises error' do + result = subject.execute + + expect(result).to include(status: :error, message: 'Error Tracking is not enabled') + end + end +end |