diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-08-12 09:09:53 +0300 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-08-12 09:09:53 +0300 |
| commit | 90156f527b43a15e794c3351ecfc59aff42c440a (patch) | |
| tree | a59318246adf4297b3911f442b6a789fd38d35a6 | |
| parent | 737684a392db1178770ad5b1d20b64386aadcac5 (diff) | |
Add latest changes from gitlab-org/gitlab@master
67 files changed, 660 insertions, 440 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index 9fb46e87c34..6529fc88e1e 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -443,6 +443,11 @@ RSpec/HaveGitlabHttpStatus: Style/MultilineWhenThen: Enabled: false +# We use EnforcedStyle of comparison here due to it being better +# performing code as seen in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36221#note_375659681 +Style/NumericPredicate: + EnforcedStyle: comparison + Style/FloatDivision: Enabled: false diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 67ed227a284..21784a993b1 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -596,66 +596,6 @@ Style/Next: Style/NumericLiteralPrefix: Enabled: false -# Offense count: 130 -# Cop supports --auto-correct. -# Configuration parameters: AutoCorrect, EnforcedStyle, IgnoredMethods. -# SupportedStyles: predicate, comparison -# We use EnforcedStyle of comparison here due to it being better -# performing code as seen https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36221#note_375659681 -Style/NumericPredicate: - EnforcedStyle: comparison - Exclude: - - 'app/controllers/concerns/issuable_collections.rb' - - 'app/controllers/concerns/paginated_collection.rb' - - 'app/helpers/graph_helper.rb' - - 'app/helpers/timeboxes_helper.rb' - - 'app/models/ci/pipeline.rb' - - 'app/models/ci/stage.rb' - - 'app/models/concerns/update_project_statistics.rb' - - 'app/models/merge_request_diff.rb' - - 'app/models/milestone.rb' - - 'app/models/network/graph.rb' - - 'app/models/postgresql/replication_slot.rb' - - 'app/models/project.rb' - - 'app/models/suggestion.rb' - - 'app/models/user.rb' - - 'app/serializers/merge_request_widget_entity.rb' - - 'app/services/boards/issues/move_service.rb' - - 'app/services/cohorts_service.rb' - - 'app/services/discussions/resolve_service.rb' - - 'app/services/issues/reorder_service.rb' - - 'app/services/notes/create_service.rb' - - 'app/services/packages/nuget/metadata_extraction_service.rb' - - 'app/services/packages/nuget/search_service.rb' - - 'app/services/projects/auto_devops/disable_service.rb' - - 'app/services/projects/update_pages_service.rb' - - 'app/services/search_service.rb' - - 'app/workers/admin_email_worker.rb' - - 'app/workers/gitlab/import/advance_stage.rb' - - 'config/initializers/validate_puma.rb' - - 'ee/app/controllers/security/projects_controller.rb' - - 'ee/app/graphql/mutations/instance_security_dashboard/remove_project.rb' - - 'ee/app/helpers/ee/timeboxes_helper.rb' - - 'ee/app/models/ci/minutes/quota.rb' - - 'ee/app/models/ee/ci/runner.rb' - - 'ee/app/models/geo_node_status.rb' - - 'ee/app/models/license.rb' - - 'ee/app/models/namespace_statistics.rb' - - 'ee/app/services/ee/issues/base_service.rb' - - 'ee/app/services/ee/merge_requests/approval_service.rb' - - 'ee/app/services/ee/quick_actions/target_service.rb' - - 'ee/app/services/elastic/indexing_control_service.rb' - - 'ee/app/services/geo/hashed_storage_migration_service.rb' - - 'ee/app/services/geo/prune_event_log_service.rb' - - 'ee/app/services/security/waf_anomaly_summary_service.rb' - - 'ee/app/services/update_build_minutes_service.rb' - - 'ee/app/workers/geo/container_repository_sync_dispatch_worker.rb' - - 'ee/app/workers/geo/file_download_dispatch_worker.rb' - - 'ee/app/workers/geo/registry_sync_worker.rb' - - 'ee/app/workers/geo/repository_shard_sync_worker.rb' - - 'ee/app/workers/geo/repository_verification/primary/shard_worker.rb' - - 'ee/app/models/ee/project.rb' - # Offense count: 117 # Cop supports --auto-correct. Style/ParallelAssignment: diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue b/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue index 7b703c5ede1..5002a8fe85d 100644 --- a/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue +++ b/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue @@ -1,5 +1,5 @@ <script> -import { GlTable, GlDeprecatedButton, GlModalDirective, GlIcon } from '@gitlab/ui'; +import { GlTable, GlButton, GlModalDirective, GlIcon } from '@gitlab/ui'; import { s__, __ } from '~/locale'; import { mapState, mapActions } from 'vuex'; import { ADD_CI_VARIABLE_MODAL_ID } from '../constants'; @@ -51,7 +51,7 @@ export default { ], components: { GlTable, - GlDeprecatedButton, + GlButton, GlIcon, CiVariablePopover, }, @@ -147,14 +147,14 @@ export default { </div> </template> <template #cell(actions)="{ item }"> - <gl-deprecated-button + <gl-button ref="edit-ci-variable" v-gl-modal-directive="$options.modalId" + icon="pencil" + :aria-label="__('Edit')" data-qa-selector="edit_ci_variable_button" @click="editVariable(item)" - > - <gl-icon :size="$options.iconSize" name="pencil" /> - </gl-deprecated-button> + /> </template> <template #empty> <p ref="empty-variables" class="text-center empty-variables text-plain"> @@ -166,20 +166,21 @@ export default { class="ci-variable-actions d-flex justify-content-end" :class="{ 'justify-content-center': !tableIsNotEmpty }" > - <gl-deprecated-button + <gl-button v-if="tableIsNotEmpty" ref="secret-value-reveal-button" data-qa-selector="reveal_ci_variable_value_button" class="gl-mr-3" @click="toggleValues(!valuesHidden)" - >{{ valuesButtonText }}</gl-deprecated-button + >{{ valuesButtonText }}</gl-button > - <gl-deprecated-button + <gl-button ref="add-ci-variable" v-gl-modal-directive="$options.modalId" data-qa-selector="add_ci_variable_button" variant="success" - >{{ __('Add Variable') }}</gl-deprecated-button + category="primary" + >{{ __('Add Variable') }}</gl-button > </div> </div> diff --git a/app/assets/javascripts/monitoring/components/alert_widget.vue b/app/assets/javascripts/monitoring/components/alert_widget.vue index 5562981fe1c..3476b180699 100644 --- a/app/assets/javascripts/monitoring/components/alert_widget.vue +++ b/app/assets/javascripts/monitoring/components/alert_widget.vue @@ -174,8 +174,8 @@ export default { handleSetApiAction(apiAction) { this.apiAction = apiAction; }, - handleCreate({ operator, threshold, prometheus_metric_id }) { - const newAlert = { operator, threshold, prometheus_metric_id }; + handleCreate({ operator, threshold, prometheus_metric_id, runbookUrl }) { + const newAlert = { operator, threshold, prometheus_metric_id, runbookUrl }; this.isLoading = true; this.service .createAlert(newAlert) @@ -189,8 +189,8 @@ export default { this.isLoading = false; }); }, - handleUpdate({ alert, operator, threshold }) { - const updatedAlert = { operator, threshold }; + handleUpdate({ alert, operator, threshold, runbookUrl }) { + const updatedAlert = { operator, threshold, runbookUrl }; this.isLoading = true; this.service .updateAlert(alert, updatedAlert) diff --git a/app/assets/javascripts/monitoring/components/alert_widget_form.vue b/app/assets/javascripts/monitoring/components/alert_widget_form.vue index 60558c0ba14..ef9dfce4aa9 100644 --- a/app/assets/javascripts/monitoring/components/alert_widget_form.vue +++ b/app/assets/javascripts/monitoring/components/alert_widget_form.vue @@ -88,6 +88,7 @@ export default { operator: null, threshold: null, prometheusMetricId: null, + runbookUrl: null, selectedAlert: {}, alertQuery: '', }; @@ -116,7 +117,8 @@ export default { this.operator && this.threshold === Number(this.threshold) && (this.operator !== this.selectedAlert.operator || - this.threshold !== this.selectedAlert.threshold) + this.threshold !== this.selectedAlert.threshold || + this.runbookUrl !== this.selectedAlert.runbookUrl) ); }, submitAction() { @@ -153,13 +155,17 @@ export default { const existingAlert = this.alertsToManage[existingAlertPath]; if (existingAlert) { + const { operator, threshold, runbookUrl } = existingAlert; + this.selectedAlert = existingAlert; - this.operator = existingAlert.operator; - this.threshold = existingAlert.threshold; + this.operator = operator; + this.threshold = threshold; + this.runbookUrl = runbookUrl; } else { this.selectedAlert = {}; this.operator = this.operators.greaterThan; this.threshold = null; + this.runbookUrl = null; } this.prometheusMetricId = queryId; @@ -168,13 +174,13 @@ export default { this.resetAlertData(); this.$emit('cancel'); }, - handleSubmit(e) { - e.preventDefault(); + handleSubmit() { this.$emit(this.submitAction, { alert: this.selectedAlert.alert_path, operator: this.operator, threshold: this.threshold, prometheus_metric_id: this.prometheusMetricId, + runbookUrl: this.runbookUrl, }); }, handleShown() { @@ -189,6 +195,7 @@ export default { this.threshold = null; this.prometheusMetricId = null; this.selectedAlert = {}; + this.runbookUrl = null; }, getAlertFormActionTrackingOption() { const label = `${this.submitAction}_alert`; @@ -217,7 +224,7 @@ export default { :modal-id="modalId" :ok-variant="submitAction === 'delete' ? 'danger' : 'success'" :ok-disabled="formDisabled" - @ok="handleSubmit" + @ok.prevent="handleSubmit" @hidden="handleHidden" @shown="handleShown" > @@ -259,7 +266,7 @@ export default { </gl-deprecated-dropdown-item> </gl-deprecated-dropdown> </gl-form-group> - <gl-button-group class="mb-2" :label="s__('PrometheusAlerts|Operator')"> + <gl-button-group class="mb-3" :label="s__('PrometheusAlerts|Operator')"> <gl-deprecated-button :class="{ active: operator === operators.greaterThan }" :disabled="formDisabled" @@ -296,11 +303,16 @@ export default { </gl-form-group> <gl-form-group v-if="glFeatures.alertRunbooks" - :label="s__('PrometheusAlerts|Runbook')" + :label="s__('PrometheusAlerts|Runbook URL (optional)')" label-for="alert-runbook" - data-testid="alertRunbookField" > - <gl-form-input id="alert-runbook" :disabled="formDisabled" type="text" /> + <gl-form-input + id="alert-runbook" + v-model="runbookUrl" + :disabled="formDisabled" + data-testid="alertRunbookField" + type="text" + /> </gl-form-group> </div> <template #modal-ok> diff --git a/app/assets/javascripts/monitoring/services/alerts_service.js b/app/assets/javascripts/monitoring/services/alerts_service.js index 4b7337972fe..a67675f1a3d 100644 --- a/app/assets/javascripts/monitoring/services/alerts_service.js +++ b/app/assets/javascripts/monitoring/services/alerts_service.js @@ -1,28 +1,39 @@ import axios from '~/lib/utils/axios_utils'; +const mapAlert = ({ runbook_url, ...alert }) => { + return { runbookUrl: runbook_url, ...alert }; +}; + export default class AlertsService { constructor({ alertsEndpoint }) { this.alertsEndpoint = alertsEndpoint; } getAlerts() { - return axios.get(this.alertsEndpoint).then(resp => resp.data); + return axios.get(this.alertsEndpoint).then(resp => mapAlert(resp.data)); } - createAlert({ prometheus_metric_id, operator, threshold }) { + createAlert({ prometheus_metric_id, operator, threshold, runbookUrl }) { return axios - .post(this.alertsEndpoint, { prometheus_metric_id, operator, threshold }) - .then(resp => resp.data); + .post(this.alertsEndpoint, { + prometheus_metric_id, + operator, + threshold, + runbook_url: runbookUrl, + }) + .then(resp => mapAlert(resp.data)); } // eslint-disable-next-line class-methods-use-this readAlert(alertPath) { - return axios.get(alertPath).then(resp => resp.data); + return axios.get(alertPath).then(resp => mapAlert(resp.data)); } // eslint-disable-next-line class-methods-use-this - updateAlert(alertPath, { operator, threshold }) { - return axios.put(alertPath, { operator, threshold }).then(resp => resp.data); + updateAlert(alertPath, { operator, threshold, runbookUrl }) { + return axios + .put(alertPath, { operator, threshold, runbook_url: runbookUrl }) + .then(resp => mapAlert(resp.data)); } // eslint-disable-next-line class-methods-use-this diff --git a/app/assets/javascripts/monitoring/validators.js b/app/assets/javascripts/monitoring/validators.js index cd426f1a221..c6b323f6360 100644 --- a/app/assets/javascripts/monitoring/validators.js +++ b/app/assets/javascripts/monitoring/validators.js @@ -1,3 +1,12 @@ +import { isSafeURL } from '~/lib/utils/url_utility'; + +const isRunbookUrlValid = runbookUrl => { + if (!runbookUrl) { + return true; + } + return isSafeURL(runbookUrl); +}; + // Prop validator for alert information, expecting an object like the example below. // // { @@ -8,6 +17,7 @@ // query: "rate(http_requests_total[5m])[30m:1m]", // threshold: 0.002, // title: "Core Usage (Total)", +// runbookUrl: "https://www.gitlab.com/my-project/-/wikis/runbook" // } // } export function alertsValidator(value) { @@ -19,7 +29,8 @@ export function alertsValidator(value) { alert.metricId && typeof alert.metricId === 'string' && alert.operator && - typeof alert.threshold === 'number' + typeof alert.threshold === 'number' && + isRunbookUrlValid(alert.runbookUrl) ); }); } diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 4f61e5ed711..89ba2175b60 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -65,7 +65,7 @@ module IssuableCollections def page_count_for_relation(relation, row_count) limit = relation.limit_value.to_f - return 1 if limit.zero? + return 1 if limit == 0 (row_count.to_f / limit).ceil end diff --git a/app/controllers/concerns/paginated_collection.rb b/app/controllers/concerns/paginated_collection.rb index be84215a9e2..fcee4493314 100644 --- a/app/controllers/concerns/paginated_collection.rb +++ b/app/controllers/concerns/paginated_collection.rb @@ -6,7 +6,7 @@ module PaginatedCollection private def redirect_out_of_range(collection, total_pages = collection.total_pages) - return false if total_pages.zero? + return false if total_pages == 0 out_of_range = collection.current_page > total_pages diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 23eb9901805..5076172985d 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -14,6 +14,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController push_frontend_feature_flag(:prometheus_computed_alerts) push_frontend_feature_flag(:disable_metric_dashboard_refresh_rate) + push_frontend_feature_flag(:alert_runbooks) end before_action :authorize_read_environment!, except: [:metrics, :additional_metrics, :metrics_dashboard, :metrics_redirect] before_action :authorize_create_environment!, only: [:new, :create] diff --git a/app/graphql/resolvers/board_list_issues_resolver.rb b/app/graphql/resolvers/board_list_issues_resolver.rb index 2c3308498f3..a7cc367379d 100644 --- a/app/graphql/resolvers/board_list_issues_resolver.rb +++ b/app/graphql/resolvers/board_list_issues_resolver.rb @@ -10,5 +10,10 @@ module Resolvers service = Boards::Issues::ListService.new(list.board.resource_parent, context[:current_user], { board_id: list.board.id, id: list.id }) Gitlab::Graphql::Pagination::OffsetActiveRecordRelationConnection.new(service.execute) end + + # https://gitlab.com/gitlab-org/gitlab/-/issues/235681 + def self.complexity_multiplier(args) + 0.005 + end end end diff --git a/app/helpers/graph_helper.rb b/app/helpers/graph_helper.rb index 49b15cde009..24072d1ab46 100644 --- a/app/helpers/graph_helper.rb +++ b/app/helpers/graph_helper.rb @@ -17,7 +17,7 @@ module GraphHelper end def success_ratio(counts) - return 100 if counts[:failed].zero? + return 100 if counts[:failed] == 0 ratio = (counts[:success].to_f / (counts[:success] + counts[:failed])) * 100 ratio.to_i diff --git a/app/helpers/timeboxes_helper.rb b/app/helpers/timeboxes_helper.rb index 8c437b283a0..34919f994ee 100644 --- a/app/helpers/timeboxes_helper.rb +++ b/app/helpers/timeboxes_helper.rb @@ -155,7 +155,7 @@ module TimeboxesHelper opened = milestone.opened_issues_count closed = milestone.closed_issues_count - return _("Issues") if total.zero? + return _("Issues") if total == 0 content = [] @@ -187,7 +187,7 @@ module TimeboxesHelper def milestone_releases_tooltip_text(milestone) count = milestone.releases.count - return _("Releases") if count.zero? + return _("Releases") if count == 0 n_("%{releases} release", "%{releases} releases", count) % { releases: count } end diff --git a/app/models/audit_event.rb b/app/models/audit_event.rb index 335d66cc7cc..e7cfa30a892 100644 --- a/app/models/audit_event.rb +++ b/app/models/audit_event.rb @@ -5,7 +5,7 @@ class AuditEvent < ApplicationRecord include IgnorableColumns include BulkInsertSafe - PARALLEL_PERSISTENCE_COLUMNS = [:author_name, :entity_path, :target_details, :target_type].freeze + PARALLEL_PERSISTENCE_COLUMNS = [:author_name, :entity_path, :target_details].freeze ignore_column :updated_at, remove_with: '13.4', remove_after: '2020-09-22' diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 30dd88ac72a..c153236dddd 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -409,7 +409,7 @@ module Ci def legacy_stage(name) stage = Ci::LegacyStage.new(self, name: name) - stage unless stage.statuses_count.zero? + stage unless stage.statuses_count == 0 end def ref_exists? @@ -658,7 +658,7 @@ module Ci end def has_warnings? - number_of_warnings.positive? + number_of_warnings > 0 end def number_of_warnings @@ -821,7 +821,7 @@ module Ci return unless started_at seconds = (started_at - created_at).to_i - seconds unless seconds.zero? + seconds unless seconds == 0 end def update_duration diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb index 41215601704..cc9e32aa4c0 100644 --- a/app/models/ci/stage.rb +++ b/app/models/ci/stage.rb @@ -113,7 +113,7 @@ module Ci end def has_warnings? - number_of_warnings.positive? + number_of_warnings > 0 end def number_of_warnings diff --git a/app/models/concerns/update_project_statistics.rb b/app/models/concerns/update_project_statistics.rb index c0fa14d3369..a7028e18451 100644 --- a/app/models/concerns/update_project_statistics.rb +++ b/app/models/concerns/update_project_statistics.rb @@ -75,7 +75,7 @@ module UpdateProjectStatistics end def schedule_update_project_statistic(delta) - return if delta.zero? + return if delta == 0 return if project.nil? run_after_commit do diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 6341a798ecc..90893882fe5 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -631,7 +631,7 @@ class MergeRequestDiff < ApplicationRecord def save_diffs new_attributes = {} - if compare.commits.size.zero? + if compare.commits.empty? new_attributes[:state] = :empty else diff_collection = compare.diffs(Commit.max_diff_options) diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 58adfd5f70b..55326b9a282 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -127,7 +127,7 @@ class Milestone < ApplicationRecord end def can_be_closed? - active? && issues.opened.count.zero? + active? && issues.opened.count == 0 end def author_id diff --git a/app/models/network/graph.rb b/app/models/network/graph.rb index 6b5ea0fc3fc..9da454125eb 100644 --- a/app/models/network/graph.rb +++ b/app/models/network/graph.rb @@ -211,7 +211,7 @@ module Network # Visit branching chains leaves.each do |l| - parents = l.parents(@map).select {|p| p.space.zero?} + parents = l.parents(@map).select {|p| p.space == 0} parents.each do |p| place_chain(p, l.time) end @@ -266,14 +266,14 @@ module Network def take_left_leaves(raw_commit) commit = @map[raw_commit.id] leaves = [] - leaves.push(commit) if commit.space.zero? + leaves.push(commit) if commit.space == 0 loop do - return leaves if commit.parents(@map).count.zero? + return leaves if commit.parents(@map).count == 0 commit = commit.parents(@map).first - return leaves unless commit.space.zero? + return leaves unless commit.space == 0 leaves.push(commit) end diff --git a/app/models/postgresql/replication_slot.rb b/app/models/postgresql/replication_slot.rb index 7a123deb719..a4370eda5ba 100644 --- a/app/models/postgresql/replication_slot.rb +++ b/app/models/postgresql/replication_slot.rb @@ -33,7 +33,7 @@ module Postgresql # If too many replicas are falling behind too much, the availability of a # GitLab instance might suffer. To prevent this from happening we require # at least 1 replica to have data recent enough. - if sizes.any? && too_great.positive? + if sizes.any? && too_great > 0 (sizes.length - too_great) <= 1 else false diff --git a/app/models/project.rb b/app/models/project.rb index fdca74af81d..87171b7debc 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1121,7 +1121,7 @@ class Project < ApplicationRecord limit = creator.projects_limit error = - if limit.zero? + if limit == 0 _('Personal project creation is not allowed. Please contact your administrator with questions') else _('Your project limit is %{limit} projects! Please contact your administrator to increase it') diff --git a/app/models/suggestion.rb b/app/models/suggestion.rb index c027a0d140e..8c72bd5ae7e 100644 --- a/app/models/suggestion.rb +++ b/app/models/suggestion.rb @@ -61,7 +61,7 @@ class Suggestion < ApplicationRecord end def single_line? - lines_above.zero? && lines_below.zero? + lines_above == 0 && lines_below == 0 end def target_line diff --git a/app/models/user.rb b/app/models/user.rb index 8c44494db56..2904dc9da76 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -962,7 +962,7 @@ class User < ApplicationRecord def require_ssh_key? count = Users::KeysCountService.new(self).count - count.zero? && Gitlab::ProtocolAccess.allowed?('ssh') + count == 0 && Gitlab::ProtocolAccess.allowed?('ssh') end # rubocop: enable CodeReuse/ServiceClass diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index 0855c67feb7..b7b9e7d1036 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -137,7 +137,7 @@ class MergeRequestWidgetEntity < Grape::Entity merge_request.source_branch_exists? && merge_request.source_project&.uses_default_ci_config? && !merge_request.source_project.has_ci? && - merge_request.commits_count.positive? && + merge_request.commits_count > 0 && can?(current_user, :read_build, merge_request.source_project) && can?(current_user, :create_pipeline, merge_request.source_project) end diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb index 9e3c84d03ec..14e8683ebdf 100644 --- a/app/services/boards/issues/move_service.rb +++ b/app/services/boards/issues/move_service.rb @@ -130,7 +130,7 @@ module Boards def move_between_ids(move_params) ids = [move_params[:move_after_id], move_params[:move_before_id]] .map(&:to_i) - .map { |m| m.positive? ? m : nil } + .map { |m| m > 0 ? m : nil } ids.any? ? ids : nil end diff --git a/app/services/cohorts_service.rb b/app/services/cohorts_service.rb index 03be87f4cc1..7bc3b267a12 100644 --- a/app/services/cohorts_service.rb +++ b/app/services/cohorts_service.rb @@ -63,7 +63,7 @@ class CohortsService overall_total = month_totals.first month_totals.map do |total| - { total: total, percentage: total.zero? ? 0 : 100 * total / overall_total } + { total: total, percentage: total == 0 ? 0 : 100 * total / overall_total } end end diff --git a/app/services/discussions/resolve_service.rb b/app/services/discussions/resolve_service.rb index 946fb5f1372..cd5925cd9be 100644 --- a/app/services/discussions/resolve_service.rb +++ b/app/services/discussions/resolve_service.rb @@ -56,7 +56,7 @@ module Discussions def process_auto_merge return unless merge_request - return unless @resolved_count.positive? + return unless @resolved_count > 0 return unless discussions_ready_to_merge? AutoMergeProcessWorker.perform_async(merge_request.id) diff --git a/app/services/issues/reorder_service.rb b/app/services/issues/reorder_service.rb index 02c18d31b5e..c82ad6ea501 100644 --- a/app/services/issues/reorder_service.rb +++ b/app/services/issues/reorder_service.rb @@ -40,7 +40,7 @@ module Issues def move_between_ids ids = [params[:move_after_id], params[:move_before_id]] .map(&:to_i) - .map { |m| m.positive? ? m : nil } + .map { |m| m > 0 ? m : nil } ids.any? ? ids : nil end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 5a9f0b7fa68..9c60c54353f 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -72,7 +72,7 @@ module Notes end def do_commands(note, update_params, message, only_commands) - return if quick_actions_service.commands_executed_count.to_i.zero? + return if quick_actions_service.commands_executed_count.to_i == 0 if update_params.present? quick_actions_service.apply_updates(update_params, note) diff --git a/app/services/packages/nuget/metadata_extraction_service.rb b/app/services/packages/nuget/metadata_extraction_service.rb index 6fec398fab0..59125669f7d 100644 --- a/app/services/packages/nuget/metadata_extraction_service.rb +++ b/app/services/packages/nuget/metadata_extraction_service.rb @@ -42,7 +42,7 @@ module Packages def valid_package_file? package_file && package_file.package&.nuget? && - package_file.file.size.positive? + package_file.file.size > 0 # rubocop:disable Style/ZeroLengthPredicate end def extract_metadata(file) diff --git a/app/services/packages/nuget/search_service.rb b/app/services/packages/nuget/search_service.rb index f7e09e11819..b95aa30bec1 100644 --- a/app/services/packages/nuget/search_service.rb +++ b/app/services/packages/nuget/search_service.rb @@ -21,8 +21,8 @@ module Packages @search_term = search_term @options = DEFAULT_OPTIONS.merge(options) - raise ArgumentError, 'negative per_page' if per_page.negative? - raise ArgumentError, 'negative padding' if padding.negative? + raise ArgumentError, 'negative per_page' if per_page < 0 + raise ArgumentError, 'negative padding' if padding < 0 end def execute diff --git a/app/services/projects/auto_devops/disable_service.rb b/app/services/projects/auto_devops/disable_service.rb index c90510c581d..e10668ac9bd 100644 --- a/app/services/projects/auto_devops/disable_service.rb +++ b/app/services/projects/auto_devops/disable_service.rb @@ -23,7 +23,7 @@ module Projects # for more context. # rubocop: disable CodeReuse/ActiveRecord def first_pipeline_failure? - auto_devops_pipelines.success.limit(1).count.zero? && + auto_devops_pipelines.success.limit(1).count == 0 && auto_devops_pipelines.failed.limit(1).count.nonzero? end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index 59389a0fa65..334f5993d15 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -136,7 +136,7 @@ module Projects def max_size max_pages_size = max_size_from_settings - return ::Gitlab::Pages::MAX_SIZE if max_pages_size.zero? + return ::Gitlab::Pages::MAX_SIZE if max_pages_size == 0 max_pages_size end diff --git a/app/services/search_service.rb b/app/services/search_service.rb index 650dc197f8c..278cf389e07 100644 --- a/app/services/search_service.rb +++ b/app/services/search_service.rb @@ -70,7 +70,7 @@ class SearchService def per_page per_page_param = params[:per_page].to_i - return DEFAULT_PER_PAGE unless per_page_param.positive? + return DEFAULT_PER_PAGE unless per_page_param > 0 [MAX_PER_PAGE, per_page_param].min end diff --git a/app/workers/admin_email_worker.rb b/app/workers/admin_email_worker.rb index c84ac60d777..8d589c03259 100644 --- a/app/workers/admin_email_worker.rb +++ b/app/workers/admin_email_worker.rb @@ -18,7 +18,7 @@ class AdminEmailWorker # rubocop:disable Scalability/IdempotentWorker # rubocop: disable CodeReuse/ActiveRecord def send_repository_check_mail repository_check_failed_count = Project.where(last_repository_check_failed: true).count - return if repository_check_failed_count.zero? + return if repository_check_failed_count == 0 RepositoryCheckMailer.notify(repository_check_failed_count).deliver_now end diff --git a/app/workers/gitlab/import/advance_stage.rb b/app/workers/gitlab/import/advance_stage.rb index 3f34437294e..9fc03efe9d0 100644 --- a/app/workers/gitlab/import/advance_stage.rb +++ b/app/workers/gitlab/import/advance_stage.rb @@ -41,7 +41,7 @@ module Gitlab # complete the work fast enough. waiter.wait(BLOCKING_WAIT_TIME) - next unless waiter.jobs_remaining.positive? + next unless waiter.jobs_remaining > 0 new_waiters[waiter.key] = waiter.jobs_remaining end diff --git a/changelogs/unreleased/220322-refactor-target-type.yml b/changelogs/unreleased/220322-refactor-target-type.yml deleted file mode 100644 index 559cd7f8f5c..00000000000 --- a/changelogs/unreleased/220322-refactor-target-type.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Add new target_type column on audit_events table -merge_request: 36198 -author: -type: changed diff --git a/changelogs/unreleased/224457-fix-data-integrity-on-issue_feedback-issue_links.yml b/changelogs/unreleased/224457-fix-data-integrity-on-issue_feedback-issue_links.yml new file mode 100644 index 00000000000..0a9c0c80863 --- /dev/null +++ b/changelogs/unreleased/224457-fix-data-integrity-on-issue_feedback-issue_links.yml @@ -0,0 +1,5 @@ +--- +title: Add migration helper index for Vulnerabilities::Finding table +merge_request: 38898 +author: +type: changed diff --git a/changelogs/unreleased/229548-adjust-telemetry-code-for-incidents.yml b/changelogs/unreleased/229548-adjust-telemetry-code-for-incidents.yml new file mode 100644 index 00000000000..8de574de501 --- /dev/null +++ b/changelogs/unreleased/229548-adjust-telemetry-code-for-incidents.yml @@ -0,0 +1,5 @@ +--- +title: Update incident_issues usage ping to use issue type column +merge_request: 38864 +author: +type: changed diff --git a/config/initializers/validate_puma.rb b/config/initializers/validate_puma.rb index 5abcfbfe6be..ac5678c4b5a 100644 --- a/config/initializers/validate_puma.rb +++ b/config/initializers/validate_puma.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true -if Gitlab::Runtime.puma? && ::Puma.cli_config.options[:workers].to_i.zero? +if Gitlab::Runtime.puma? && ::Puma.cli_config.options[:workers].to_i == 0 raise 'Puma is only supported in Cluster-mode: workers > 0' end diff --git a/db/post_migrate/20200807110237_add_migration_index_to_vulnerabilities_occurrences.rb b/db/post_migrate/20200807110237_add_migration_index_to_vulnerabilities_occurrences.rb new file mode 100644 index 00000000000..e806a337f8f --- /dev/null +++ b/db/post_migrate/20200807110237_add_migration_index_to_vulnerabilities_occurrences.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddMigrationIndexToVulnerabilitiesOccurrences < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :vulnerability_occurrences, + "project_id, report_type, encode(project_fingerprint, 'hex'::text)", + name: 'index_vulnerability_occurrences_for_issue_links_migration' + end + + def down + remove_concurrent_index_by_name :vulnerability_occurrences, + :index_vulnerability_occurrences_for_issue_links_migration + end +end diff --git a/db/post_migrate/20200810100921_add_target_type_to_audit_event.rb b/db/post_migrate/20200810100921_add_target_type_to_audit_event.rb deleted file mode 100644 index 8dde5945f0d..00000000000 --- a/db/post_migrate/20200810100921_add_target_type_to_audit_event.rb +++ /dev/null @@ -1,121 +0,0 @@ -# frozen_string_literal: true - -class AddTargetTypeToAuditEvent < ActiveRecord::Migration[6.0] - include Gitlab::Database::SchemaHelpers - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - SOURCE_TABLE_NAME = 'audit_events' - PARTITIONED_TABLE_NAME = 'audit_events_part_5fc467ac26' - TRIGGER_FUNCTION_NAME = 'table_sync_function_2be879775d' - - def up - with_lock_retries do - # rubocop:disable Migration/AddLimitToTextColumns - add_column('audit_events', :target_type, :text) - add_column('audit_events_part_5fc467ac26', :target_type, :text) - # rubocop:enable Migration/AddLimitToTextColumns - - create_trigger_function(TRIGGER_FUNCTION_NAME, replace: true) do - <<~SQL - IF (TG_OP = 'DELETE') THEN - DELETE FROM #{PARTITIONED_TABLE_NAME} where id = OLD.id; - ELSIF (TG_OP = 'UPDATE') THEN - UPDATE #{PARTITIONED_TABLE_NAME} - SET author_id = NEW.author_id, - type = NEW.type, - entity_id = NEW.entity_id, - entity_type = NEW.entity_type, - details = NEW.details, - ip_address = NEW.ip_address, - author_name = NEW.author_name, - entity_path = NEW.entity_path, - target_details = NEW.target_details, - target_type = NEW.target_type, - created_at = NEW.created_at - WHERE #{PARTITIONED_TABLE_NAME}.id = NEW.id; - ELSIF (TG_OP = 'INSERT') THEN - INSERT INTO #{PARTITIONED_TABLE_NAME} (id, - author_id, - type, - entity_id, - entity_type, - details, - ip_address, - author_name, - entity_path, - target_details, - target_type, - created_at) - VALUES (NEW.id, - NEW.author_id, - NEW.type, - NEW.entity_id, - NEW.entity_type, - NEW.details, - NEW.ip_address, - NEW.author_name, - NEW.entity_path, - NEW.target_details, - NEW.target_type, - NEW.created_at); - END IF; - RETURN NULL; - SQL - end - end - end - - def down - with_lock_retries do - remove_column SOURCE_TABLE_NAME, :target_type - - create_trigger_function(TRIGGER_FUNCTION_NAME, replace: true) do - <<~SQL - IF (TG_OP = 'DELETE') THEN - DELETE FROM #{PARTITIONED_TABLE_NAME} where id = OLD.id; - ELSIF (TG_OP = 'UPDATE') THEN - UPDATE #{PARTITIONED_TABLE_NAME} - SET author_id = NEW.author_id, - type = NEW.type, - entity_id = NEW.entity_id, - entity_type = NEW.entity_type, - details = NEW.details, - ip_address = NEW.ip_address, - author_name = NEW.author_name, - entity_path = NEW.entity_path, - target_details = NEW.target_details, - created_at = NEW.created_at - WHERE #{PARTITIONED_TABLE_NAME}.id = NEW.id; - ELSIF (TG_OP = 'INSERT') THEN - INSERT INTO #{PARTITIONED_TABLE_NAME} (id, - author_id, - type, - entity_id, - entity_type, - details, - ip_address, - author_name, - entity_path, - target_details, - created_at) - VALUES (NEW.id, - NEW.author_id, - NEW.type, - NEW.entity_id, - NEW.entity_type, - NEW.details, - NEW.ip_address, - NEW.author_name, - NEW.entity_path, - NEW.target_details, - NEW.created_at); - END IF; - RETURN NULL; - SQL - end - - remove_column PARTITIONED_TABLE_NAME, :target_type - end - end -end diff --git a/db/post_migrate/20200810101029_add_text_limit_to_audit_event_target_type.rb b/db/post_migrate/20200810101029_add_text_limit_to_audit_event_target_type.rb deleted file mode 100644 index 2a5ea9cd245..00000000000 --- a/db/post_migrate/20200810101029_add_text_limit_to_audit_event_target_type.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -class AddTextLimitToAuditEventTargetType < ActiveRecord::Migration[6.0] - include Gitlab::Database::MigrationHelpers - DOWNTIME = false - SOURCE_TABLE_NAME = 'audit_events' - PARTITIONED_TABLE_NAME = 'audit_events_part_5fc467ac26' - - disable_ddl_transaction! - - def up - add_text_limit(SOURCE_TABLE_NAME, :target_type, 255) - add_text_limit(PARTITIONED_TABLE_NAME, :target_type, 255) - end - - def down - remove_text_limit(SOURCE_TABLE_NAME, :target_type) - remove_text_limit(PARTITIONED_TABLE_NAME, :target_type) - end -end diff --git a/db/schema_migrations/20200807110237 b/db/schema_migrations/20200807110237 new file mode 100644 index 00000000000..fd02f50cdff --- /dev/null +++ b/db/schema_migrations/20200807110237 @@ -0,0 +1 @@ +5aeb0e74859df14e460b39b9f070624a02278c13b1da9393e1671b4c79635f52
\ No newline at end of file diff --git a/db/schema_migrations/20200810100921 b/db/schema_migrations/20200810100921 deleted file mode 100644 index 14dc5a41721..00000000000 --- a/db/schema_migrations/20200810100921 +++ /dev/null @@ -1 +0,0 @@ -fe96df46a5f360cafe8f9816c6dfc2d00afdcf458fb38ace37ce59999dba2413
\ No newline at end of file diff --git a/db/schema_migrations/20200810101029 b/db/schema_migrations/20200810101029 deleted file mode 100644 index 59e0a236f3c..00000000000 --- a/db/schema_migrations/20200810101029 +++ /dev/null @@ -1 +0,0 @@ -8bb03ea2ded957a41aa1efd60a9908a3c597aaaade9190f8f1515bfd2ab9a282
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 0a07834bb44..39479d54154 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -29,7 +29,6 @@ ELSIF (TG_OP = 'UPDATE') THEN author_name = NEW.author_name, entity_path = NEW.entity_path, target_details = NEW.target_details, - target_type = NEW.target_type, created_at = NEW.created_at WHERE audit_events_part_5fc467ac26.id = NEW.id; ELSIF (TG_OP = 'INSERT') THEN @@ -43,7 +42,6 @@ ELSIF (TG_OP = 'INSERT') THEN author_name, entity_path, target_details, - target_type, created_at) VALUES (NEW.id, NEW.author_id, @@ -55,7 +53,6 @@ ELSIF (TG_OP = 'INSERT') THEN NEW.author_name, NEW.entity_path, NEW.target_details, - NEW.target_type, NEW.created_at); END IF; RETURN NULL; @@ -77,10 +74,8 @@ CREATE TABLE public.audit_events_part_5fc467ac26 ( entity_path text, target_details text, created_at timestamp without time zone NOT NULL, - target_type text, CONSTRAINT check_492aaa021d CHECK ((char_length(entity_path) <= 5500)), CONSTRAINT check_83ff8406e2 CHECK ((char_length(author_name) <= 255)), - CONSTRAINT check_97a8c868e7 CHECK ((char_length(target_type) <= 255)), CONSTRAINT check_d493ec90b5 CHECK ((char_length(target_details) <= 5500)) ) PARTITION BY RANGE (created_at); @@ -9475,9 +9470,7 @@ CREATE TABLE public.audit_events ( author_name text, entity_path text, target_details text, - target_type text, CONSTRAINT check_492aaa021d CHECK ((char_length(entity_path) <= 5500)), - CONSTRAINT check_82294106dd CHECK ((char_length(target_type) <= 255)), CONSTRAINT check_83ff8406e2 CHECK ((char_length(author_name) <= 255)), CONSTRAINT check_d493ec90b5 CHECK ((char_length(target_details) <= 5500)) ); @@ -20950,6 +20943,8 @@ CREATE UNIQUE INDEX index_vulnerability_occurrence_identifiers_on_unique_keys ON CREATE INDEX index_vulnerability_occurrence_pipelines_on_pipeline_id ON public.vulnerability_occurrence_pipelines USING btree (pipeline_id); +CREATE INDEX index_vulnerability_occurrences_for_issue_links_migration ON public.vulnerability_occurrences USING btree (project_id, report_type, encode(project_fingerprint, 'hex'::text)); + CREATE INDEX index_vulnerability_occurrences_on_primary_identifier_id ON public.vulnerability_occurrences USING btree (primary_identifier_id); CREATE INDEX index_vulnerability_occurrences_on_scanner_id ON public.vulnerability_occurrences USING btree (scanner_id); diff --git a/doc/api/namespaces.md b/doc/api/namespaces.md index e38e725fb97..ba59d467bc8 100644 --- a/doc/api/namespaces.md +++ b/doc/api/namespaces.md @@ -31,7 +31,14 @@ Example response: "name": "user1", "path": "user1", "kind": "user", - "full_path": "user1" + "full_path": "user1", + "parent_id": null, + "avatar_url": "https://secure.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "web_url": "https://gitlab.example.com/user1", + "billable_members_count": 1, + "plan": "default", + "trial_ends_on": null, + "trial": false }, { "id": 2, @@ -40,7 +47,13 @@ Example response: "kind": "group", "full_path": "group1", "parent_id": null, - "members_count_with_descendants": 2 + "avatar_url": null, + "web_url": "https://gitlab.example.com/groups/group1", + "members_count_with_descendants": 2, + "billable_members_count": 2, + "plan": "default", + "trial_ends_on": null, + "trial": false }, { "id": 3, @@ -49,7 +62,13 @@ Example response: "kind": "group", "full_path": "foo/bar", "parent_id": 9, - "members_count_with_descendants": 5 + "avatar_url": null, + "web_url": "https://gitlab.example.com/groups/foo/bar", + "members_count_with_descendants": 5, + "billable_members_count": 5, + "plan": "default", + "trial_ends_on": null, + "trial": false } ] ``` @@ -100,7 +119,13 @@ Example response: "kind": "group", "full_path": "twitter", "parent_id": null, - "members_count_with_descendants": 2 + "avatar_url": null, + "web_url": "https://gitlab.example.com/groups/twitter", + "members_count_with_descendants": 2, + "billable_members_count": 2, + "plan": "default", + "trial_ends_on": null, + "trial": false } ] ``` @@ -133,7 +158,13 @@ Example response: "kind": "group", "full_path": "group1", "parent_id": null, - "members_count_with_descendants": 2 + "avatar_url": null, + "web_url": "https://gitlab.example.com/groups/group1", + "members_count_with_descendants": 2, + "billable_members_count": 2, + "plan": "default", + "trial_ends_on": null, + "trial": false } ``` @@ -153,6 +184,12 @@ Example response: "kind": "group", "full_path": "group1", "parent_id": null, - "members_count_with_descendants": 2 + "avatar_url": null, + "web_url": "https://gitlab.example.com/groups/group1", + "members_count_with_descendants": 2, + "billable_members_count": 2, + "plan": "default", + "trial_ends_on": null, + "trial": false } ``` diff --git a/doc/operations/metrics/dashboards/img/metrics_dashboard_panel_preview_v13_3.png b/doc/operations/metrics/dashboards/img/metrics_dashboard_panel_preview_v13_3.png Binary files differnew file mode 100644 index 00000000000..4f6d3b3dfa4 --- /dev/null +++ b/doc/operations/metrics/dashboards/img/metrics_dashboard_panel_preview_v13_3.png diff --git a/doc/operations/metrics/dashboards/index.md b/doc/operations/metrics/dashboards/index.md index b9008c18e13..d9d244ac7de 100644 --- a/doc/operations/metrics/dashboards/index.md +++ b/doc/operations/metrics/dashboards/index.md @@ -72,6 +72,51 @@ NOTE: **Note:** Configuration files nested under subdirectories of `.gitlab/dashboards` are not supported and won't be available in the UI. +## Add a new metrics panel to a dashboard + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/228761) in GitLab 13.3 behind a disabled [feature flag](../../../administration/feature_flags.md): `metrics_dashboard_new_panel_page`. + +The metrics dashboard supports various [multiple panel types](../../../operations/metrics/dashboards/panel_types.md). +You can quickly test how a panel configuration would display in your metrics dashboard +with the **Add Panel** page: + +1. Sign in to GitLab as a user with Maintainer or Owner + [permissions](../../../user/permissions.md#project-members-permissions) on a + project that has the [feature flag enabled](#enable-or-disable-testing-metrics-panels). +1. Open the URL `https://example.com/PROJECT/-/metrics/panel/new`, replacing + `example.com` with your domain name, and `PROJECT` with the name of your project, + to display the panel configuration page. +1. In the **Define and preview panel** section, paste in the YAML you want to + preview in the **Panel YAML** field. +1. Click **Preview panel**, and GitLab displays a preview of the chart below the + `Define and preview panel` section: +  + +### Enable or disable testing metrics panels + +Testing metrics panels in the UI is under development and not ready for production use. It's +deployed behind a feature flag that's **disabled by default**. +[GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md) +can enable it for your instance. Testing metrics panels in the UI can be enabled or disabled per-project. + +To enable it: + +```ruby +# Instance-wide +Feature.enable(:metrics_dashboard_new_panel_page) +# or by project +Feature.enable(:metrics_dashboard_new_panel_page, Project.find(metrics_dashboard_new_panel_page)) +``` + +To disable it: + +```ruby +# Instance-wide +Feature.disable(:metrics_dashboard_new_panel_page) +# or by project +Feature.disable(:metrics_dashboard_new_panel_page, Project.find(metrics_dashboard_new_panel_page)) +``` + ## Duplicate a GitLab-defined dashboard > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/37238) in GitLab 12.7. diff --git a/doc/user/analytics/img/vsa_filter_bar_v13.3.png b/doc/user/analytics/img/vsa_filter_bar_v13.3.png Binary files differnew file mode 100644 index 00000000000..71e59892434 --- /dev/null +++ b/doc/user/analytics/img/vsa_filter_bar_v13.3.png diff --git a/doc/user/analytics/value_stream_analytics.md b/doc/user/analytics/value_stream_analytics.md index b11bae98af3..f78d381ea79 100644 --- a/doc/user/analytics/value_stream_analytics.md +++ b/doc/user/analytics/value_stream_analytics.md @@ -48,7 +48,30 @@ There are seven stages that are tracked as part of the Value Stream Analytics ca - **Total** (Total) - Total lifecycle time. That is, the velocity of the project or team. [Previously known](https://gitlab.com/gitlab-org/gitlab/-/issues/38317) as **Production**. -## Date ranges +## Filter the analytics data + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13216) in GitLab 13.3 + +GitLab provides the ability to filter analytics based on the following parameters: + +- Milestones (Group level) +- Labels (Group level) +- Author +- Assignees + +To filter results: + +1. Select a group. +1. Click on the filter bar. +1. Select a parameter to filter by. +1. Select a value from the autocompleted results, or type to refine the results. + + + +NOTE: **Note:** +Filtering is available only for group-level Value Stream Analytics. + +### Date ranges > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13216) in GitLab 12.4. diff --git a/lib/gitlab/metrics/dashboard/validator/client.rb b/lib/gitlab/metrics/dashboard/validator/client.rb index 6143edbd248..5d8ea448ac1 100644 --- a/lib/gitlab/metrics/dashboard/validator/client.rb +++ b/lib/gitlab/metrics/dashboard/validator/client.rb @@ -34,7 +34,7 @@ module Gitlab end def schemer - @schemer ||= JSONSchemer.schema(Pathname.new(schema_path), formats: custom_formats.format_handlers) + @schemer ||= ::JSONSchemer.schema(Pathname.new(schema_path), formats: custom_formats.format_handlers) end def validate_against_schema diff --git a/lib/gitlab/metrics/dashboard/validator/errors.rb b/lib/gitlab/metrics/dashboard/validator/errors.rb index 104eb162209..0f6e687d291 100644 --- a/lib/gitlab/metrics/dashboard/validator/errors.rb +++ b/lib/gitlab/metrics/dashboard/validator/errors.rb @@ -9,19 +9,42 @@ module Gitlab class SchemaValidationError < InvalidDashboardError def initialize(error = {}) - if error.is_a?(Hash) && error.present? - data = error["data"] - data_pointer = error["data_pointer"] - schema = error["schema"] - schema_pointer = error["schema_pointer"] + super(error_message(error)) + end - msg = _("'%{data}' is invalid at '%{data_pointer}'. Should be '%{schema}' due to schema definition at '%{schema_pointer}'") % - { data: data, data_pointer: data_pointer, schema: schema, schema_pointer: schema_pointer } + private + + def error_message(error) + if error.is_a?(Hash) && error.present? + pretty(error) else - msg = "Dashboard failed schema validation" + "Dashboard failed schema validation" end + end + + # based on https://github.com/davishmcclurg/json_schemer/blob/master/lib/json_schemer/errors.rb + # with addition ability to translate error messages + def pretty(error) + data, data_pointer, type, schema = error.values_at('data', 'data_pointer', 'type', 'schema') + location = data_pointer.empty? ? 'root' : data_pointer - super(msg) + case type + when 'required' + keys = error.fetch('details').fetch('missing_keys').join(', ') + _("%{location} is missing required keys: %{keys}") % { location: location, keys: keys } + when 'null', 'string', 'boolean', 'integer', 'number', 'array', 'object' + _("'%{data}' at %{location} is not of type: %{type}") % { data: data, location: location, type: type } + when 'pattern' + _("'%{data}' at %{location} does not match pattern: %{pattern}") % { data: data, location: location, pattern: schema.fetch('pattern') } + when 'format' + _("'%{data}' at %{location} does not match format: %{format}") % { data: data, location: location, format: schema.fetch('format') } + when 'const' + _("'%{data}' at %{location} is not: %{const}") % { data: data, location: location, const: schema.fetch('const').inspect } + when 'enum' + _("'%{data}' at %{location} is not one of: %{enum}") % { data: data, location: location, enum: schema.fetch('enum') } + else + _("'%{data}' at %{location} is invalid: error_type=%{type}") % { data: data, location: location, type: type } + end end end diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 86bf43ea85d..e6a82da86b7 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -63,7 +63,6 @@ module Gitlab # rubocop: disable Metrics/AbcSize # rubocop: disable CodeReuse/ActiveRecord def system_usage_data - alert_bot_incident_count = count(::Issue.authored(::User.alert_bot), start: issue_minimum_id, finish: issue_maximum_id) issues_created_manually_from_alerts = count(Issue.with_alert_management_alerts.not_authored_by(::User.alert_bot), start: issue_minimum_id, finish: issue_maximum_id) { @@ -121,8 +120,8 @@ module Gitlab issues_created_from_alerts: total_alert_issues, issues_created_gitlab_alerts: issues_created_manually_from_alerts, issues_created_manually_from_alerts: issues_created_manually_from_alerts, - incident_issues: alert_bot_incident_count, - alert_bot_incident_issues: alert_bot_incident_count, + incident_issues: count(::Issue.incident, start: issue_minimum_id, finish: issue_maximum_id), + alert_bot_incident_issues: count(::Issue.authored(::User.alert_bot), start: issue_minimum_id, finish: issue_maximum_id), incident_labeled_issues: count(::Issue.with_label_attributes(::IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES), start: issue_minimum_id, finish: issue_maximum_id), keys: count(Key), label_lists: count(List.label), diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fe5821928bf..402e16d0954 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -527,6 +527,9 @@ msgstr "" msgid "%{loadingIcon} Started" msgstr "" +msgid "%{location} is missing required keys: %{keys}" +msgstr "" + msgid "%{lock_path} is locked by GitLab User %{lock_user_id}" msgstr "" @@ -806,7 +809,22 @@ msgstr "" msgid "<project name>" msgstr "" -msgid "'%{data}' is invalid at '%{data_pointer}'. Should be '%{schema}' due to schema definition at '%{schema_pointer}'" +msgid "'%{data}' at %{location} does not match format: %{format}" +msgstr "" + +msgid "'%{data}' at %{location} does not match pattern: %{pattern}" +msgstr "" + +msgid "'%{data}' at %{location} is invalid: error_type=%{type}" +msgstr "" + +msgid "'%{data}' at %{location} is not of type: %{type}" +msgstr "" + +msgid "'%{data}' at %{location} is not one of: %{enum}" +msgstr "" + +msgid "'%{data}' at %{location} is not: %{const}" msgstr "" msgid "'%{level}' is not a valid visibility level" @@ -7593,9 +7611,6 @@ msgstr "" msgid "DastProfiles|Do you want to discard your changes?" msgstr "" -msgid "DastProfiles|Edit feature will come soon. Please create a new profile if changes needed" -msgstr "" - msgid "DastProfiles|Edit site profile" msgstr "" @@ -10948,30 +10963,15 @@ msgstr "" msgid "GeoNodeSyncStatus|Node is slow, overloaded, or it just recovered after an outage." msgstr "" -msgid "GeoNodes|Attachments" -msgstr "" - -msgid "GeoNodes|Checksummed" -msgstr "" - msgid "GeoNodes|Consult Geo troubleshooting information" msgstr "" -msgid "GeoNodes|Container repositories" -msgstr "" - msgid "GeoNodes|Data replication lag" msgstr "" -msgid "GeoNodes|Design repositories" -msgstr "" - msgid "GeoNodes|Does not match the primary storage configuration" msgstr "" -msgid "GeoNodes|Failed" -msgstr "" - msgid "GeoNodes|Full" msgstr "" @@ -10987,12 +10987,6 @@ msgstr "" msgid "GeoNodes|Internal URL" msgstr "" -msgid "GeoNodes|Job artifacts" -msgstr "" - -msgid "GeoNodes|LFS objects" -msgstr "" - msgid "GeoNodes|Last event ID processed by cursor" msgstr "" @@ -11020,12 +11014,6 @@ msgstr "" msgid "GeoNodes|Node's status was updated %{timeAgo}." msgstr "" -msgid "GeoNodes|Not checksummed" -msgstr "" - -msgid "GeoNodes|Package files" -msgstr "" - msgid "GeoNodes|Pausing replication stops the sync process. Are you sure?" msgstr "" @@ -11047,15 +11035,6 @@ msgstr "" msgid "GeoNodes|Replication status" msgstr "" -msgid "GeoNodes|Repositories" -msgstr "" - -msgid "GeoNodes|Repository checksum progress" -msgstr "" - -msgid "GeoNodes|Repository verification progress" -msgstr "" - msgid "GeoNodes|Selective (%{syncLabel})" msgstr "" @@ -11083,27 +11062,12 @@ msgstr "" msgid "GeoNodes|Unused slots" msgstr "" -msgid "GeoNodes|Unverified" -msgstr "" - msgid "GeoNodes|Updated %{timeAgo}" msgstr "" msgid "GeoNodes|Used slots" msgstr "" -msgid "GeoNodes|Verified" -msgstr "" - -msgid "GeoNodes|Wiki checksum progress" -msgstr "" - -msgid "GeoNodes|Wiki verification progress" -msgstr "" - -msgid "GeoNodes|Wikis" -msgstr "" - msgid "GeoNodes|With %{geo} you can install a special read-only and replicated instance anywhere. Before you add nodes, follow the %{instructions} in the exact order they appear." msgstr "" @@ -11116,6 +11080,12 @@ msgstr "" msgid "GeoNodes|secondary nodes" msgstr "" +msgid "Geo|%{itemTitle} checksum progress" +msgstr "" + +msgid "Geo|%{itemTitle} verification progress" +msgstr "" + msgid "Geo|%{label} can't be blank" msgstr "" @@ -19414,7 +19384,7 @@ msgstr "" msgid "PrometheusAlerts|Operator" msgstr "" -msgid "PrometheusAlerts|Runbook" +msgid "PrometheusAlerts|Runbook URL (optional)" msgstr "" msgid "PrometheusAlerts|Select query" diff --git a/spec/controllers/projects/ci/daily_build_group_report_results_controller_spec.rb b/spec/controllers/projects/ci/daily_build_group_report_results_controller_spec.rb index 252ad6ec9c4..594c24bb7e3 100644 --- a/spec/controllers/projects/ci/daily_build_group_report_results_controller_spec.rb +++ b/spec/controllers/projects/ci/daily_build_group_report_results_controller_spec.rb @@ -149,8 +149,4 @@ RSpec.describe Projects::Ci::DailyBuildGroupReportResultsController do date: date ) end - - def csv_response - CSV.parse(response.body) - end end diff --git a/spec/factories/usage_data.rb b/spec/factories/usage_data.rb index 76673095306..d2b8fd94aca 100644 --- a/spec/factories/usage_data.rb +++ b/spec/factories/usage_data.rb @@ -31,8 +31,8 @@ FactoryBot.define do create(:project_error_tracking_setting, project: projects[1], enabled: false) create(:alerts_service, project: projects[0]) create(:alerts_service, :inactive, project: projects[1]) - alert_bot_issues = create_list(:issue, 2, project: projects[0], author: User.alert_bot) - create_list(:issue, 2, project: projects[1], author: User.alert_bot) + alert_bot_issues = create_list(:incident, 2, project: projects[0], author: User.alert_bot) + create_list(:incident, 2, project: projects[1], author: User.alert_bot) issues = create_list(:issue, 4, project: projects[0]) create_list(:prometheus_alert, 2, project: projects[0]) create(:prometheus_alert, project: projects[1]) diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_groups_missing_panels_and_group.yml b/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_groups_missing_panels_and_group.yml new file mode 100644 index 00000000000..746a90f266e --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_groups_missing_panels_and_group.yml @@ -0,0 +1,33 @@ +dashboard: 'Test Dashboard' +panel_groups: +- panels: + - title: "Super Chart A1" + type: "area-chart" + y_label: "y_label" + weight: 1 + max_value: 1 + metrics: + - id: metric_a1 + query_range: |+ + avg( + sum( + container_memory_usage_bytes{ + container_name!="POD", + pod_name=~"^{{ci_environment_slug}}-(.*)", + namespace="{{kube_namespace}}" + } + ) by (job) + ) without (job) + /1024/1024/1024 + unit: unit + label: Legend Label + - title: "Super Chart A2" + type: "area-chart" + y_label: "y_label" + weight: 2 + metrics: + - id: metric_a2 + query_range: 'query' + label: Legend Label + unit: unit +- group: Group B diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_is_an_array.yml b/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_is_an_array.yml new file mode 100644 index 00000000000..7627592553e --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_is_an_array.yml @@ -0,0 +1,15 @@ +--- +- dashboard: 'Test Dashboard' + panel_groups: + - group: Group A + panels: + - title: "Super Chart A2" + type: "area-chart" + y_label: "y_label" + weight: 2 + metrics: + - id: metric_a2 + query_range: 'query' + label: Legend Label + unit: unit +- dashboard: 'second entry' diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_missing_panel_groups.yml b/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_missing_panel_groups.yml new file mode 100644 index 00000000000..6f9e22c3212 --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_missing_panel_groups.yml @@ -0,0 +1,32 @@ +dashboard: 'Test Dashboard' +priority: 1 +links: + - title: Link 1 + url: https://gitlab.com + - title: Link 2 + url: https://docs.gitlab.com +templating: + variables: + text_variable_full_syntax: + label: 'Variable 1' + type: text + options: + default_value: 'default' + text_variable_simple_syntax: 'default value' + custom_variable_simple_syntax: ['value1', 'value2', 'value3'] + custom_variable_full_syntax: + label: 'Variable 2' + type: custom + options: + values: + - value: 'value option 1' + text: 'Option 1' + - value: 'value_option_2' + text: 'Option 2' + default: true + metric_label_values_variable: + label: 'Variable 3' + type: metric_label_values + options: + series_selector: 'backend:haproxy_backend_availability:ratio{env="{{env}}"}' + label: 'backend' diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_panel_is_missing_metrics.yml b/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_panel_is_missing_metrics.yml new file mode 100644 index 00000000000..8f12365dca2 --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_panel_is_missing_metrics.yml @@ -0,0 +1,15 @@ +dashboard: 'Test Dashboard' +panel_groups: +- group: Group A + priority: 1 + panels: + - title: "Super Chart A1" + type: "area-chart" + y_label: "y_label" + weight: 1 + max_value: 1 + - title: "Super Chart A2" + type: "area-chart" + y_label: "y_label" + weight: 2 + metrics: diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_panle_groups_wrong_content_type.yml b/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_panle_groups_wrong_content_type.yml new file mode 100644 index 00000000000..104107fa96e --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/dashboard_panle_groups_wrong_content_type.yml @@ -0,0 +1,33 @@ +dashboard: 'Test Dashboard' +priority: 1 +links: +- title: Link 1 + url: https://gitlab.com +- title: Link 2 + url: https://docs.gitlab.com +templating: + variables: + text_variable_full_syntax: + label: 'Variable 1' + type: text + options: + default_value: 'default' + text_variable_simple_syntax: 'default value' + custom_variable_simple_syntax: ['value1', 'value2', 'value3'] + custom_variable_full_syntax: + label: 'Variable 2' + type: custom + options: + values: + - value: 'value option 1' + text: 'Option 1' + - value: 'value_option_2' + text: 'Option 2' + default: true + metric_label_values_variable: + label: 'Variable 3' + type: metric_label_values + options: + series_selector: 'backend:haproxy_backend_availability:ratio{env="{{env}}"}' + label: 'backend' +panel_groups: this should be an array diff --git a/spec/frontend/monitoring/components/alert_widget_form_spec.js b/spec/frontend/monitoring/components/alert_widget_form_spec.js index 1909c4c7cdb..799a19c3f58 100644 --- a/spec/frontend/monitoring/components/alert_widget_form_spec.js +++ b/spec/frontend/monitoring/components/alert_widget_form_spec.js @@ -1,5 +1,6 @@ import { shallowMount } from '@vue/test-utils'; import { GlLink } from '@gitlab/ui'; +import INVALID_URL from '~/lib/utils/invalid_url'; import AlertWidgetForm from '~/monitoring/components/alert_widget_form.vue'; import ModalStub from '../stubs/modal_stub'; @@ -24,7 +25,13 @@ describe('AlertWidgetForm', () => { const propsWithAlertData = { ...defaultProps, alertsToManage: { - alert: { alert_path: alertPath, operator: '<', threshold: 5, metricId }, + alert: { + alert_path: alertPath, + operator: '<', + threshold: 5, + metricId, + runbookUrl: INVALID_URL, + }, }, configuredAlert: metricId, }; @@ -49,16 +56,11 @@ describe('AlertWidgetForm', () => { const modal = () => wrapper.find(ModalStub); const modalTitle = () => modal().attributes('title'); const submitButton = () => modal().find(GlLink); - const alertRunbookField = () => wrapper.find('[data-testid="alertRunbookField"]'); + const findRunbookField = () => modal().find('[data-testid="alertRunbookField"]'); + const findThresholdField = () => modal().find('[data-qa-selector="alert_threshold_field"]'); const submitButtonTrackingOpts = () => JSON.parse(submitButton().attributes('data-tracking-options')); - const e = { - preventDefault: jest.fn(), - }; - - beforeEach(() => { - e.preventDefault.mockReset(); - }); + const stubEvent = { preventDefault: jest.fn() }; afterEach(() => { if (wrapper) wrapper.destroy(); @@ -85,35 +87,34 @@ describe('AlertWidgetForm', () => { expect(submitButtonTrackingOpts()).toEqual(dataTrackingOptions.create); }); - it('emits a "create" event when form submitted without existing alert', () => { - createComponent(); + it('emits a "create" event when form submitted without existing alert', async () => { + createComponent(defaultProps, { alertRunbooks: true }); - wrapper.vm.selectQuery('9'); - wrapper.setData({ - threshold: 900, - }); + modal().vm.$emit('shown'); - wrapper.vm.handleSubmit(e); + findThresholdField().vm.$emit('input', 900); + findRunbookField().vm.$emit('input', INVALID_URL); + + modal().vm.$emit('ok', stubEvent); expect(wrapper.emitted().create[0]).toEqual([ { alert: undefined, operator: '>', threshold: 900, - prometheus_metric_id: '9', + prometheus_metric_id: '8', + runbookUrl: INVALID_URL, }, ]); - expect(e.preventDefault).toHaveBeenCalledTimes(1); }); it('resets form when modal is dismissed (hidden)', () => { - createComponent(); + createComponent(defaultProps, { alertRunbooks: true }); - wrapper.vm.selectQuery('9'); - wrapper.vm.selectQuery('>'); - wrapper.setData({ - threshold: 800, - }); + modal().vm.$emit('shown'); + + findThresholdField().vm.$emit('input', 800); + findRunbookField().vm.$emit('input', INVALID_URL); modal().vm.$emit('hidden'); @@ -121,6 +122,7 @@ describe('AlertWidgetForm', () => { expect(wrapper.vm.operator).toBe(null); expect(wrapper.vm.threshold).toBe(null); expect(wrapper.vm.prometheusMetricId).toBe(null); + expect(wrapper.vm.runbookUrl).toBe(null); }); it('sets selectedAlert to the provided configuredAlert on modal show', () => { @@ -167,7 +169,7 @@ describe('AlertWidgetForm', () => { beforeEach(() => { createComponent(propsWithAlertData); - wrapper.vm.selectQuery(metricId); + modal().vm.$emit('shown'); }); it('sets tracking options for delete alert', () => { @@ -180,7 +182,7 @@ describe('AlertWidgetForm', () => { }); it('emits "delete" event when form values unchanged', () => { - wrapper.vm.handleSubmit(e); + modal().vm.$emit('ok', stubEvent); expect(wrapper.emitted().delete[0]).toEqual([ { @@ -188,51 +190,58 @@ describe('AlertWidgetForm', () => { operator: '<', threshold: 5, prometheus_metric_id: '8', + runbookUrl: INVALID_URL, }, ]); - expect(e.preventDefault).toHaveBeenCalledTimes(1); }); + }); - it('emits "update" event when form changed', () => { - wrapper.setData({ - threshold: 11, - }); + it('emits "update" event when form changed', () => { + const updatedRunbookUrl = `${INVALID_URL}/test`; - wrapper.vm.handleSubmit(e); + createComponent(propsWithAlertData, { alertRunbooks: true }); - expect(wrapper.emitted().update[0]).toEqual([ - { - alert: 'alert', - operator: '<', - threshold: 11, - prometheus_metric_id: '8', - }, - ]); - expect(e.preventDefault).toHaveBeenCalledTimes(1); - }); + modal().vm.$emit('shown'); + + findRunbookField().vm.$emit('input', updatedRunbookUrl); + findThresholdField().vm.$emit('input', 11); - it('sets tracking options for update alert', () => { - wrapper.setData({ + modal().vm.$emit('ok', stubEvent); + + expect(wrapper.emitted().update[0]).toEqual([ + { + alert: 'alert', + operator: '<', threshold: 11, - }); + prometheus_metric_id: '8', + runbookUrl: updatedRunbookUrl, + }, + ]); + }); - return wrapper.vm.$nextTick(() => { - expect(submitButtonTrackingOpts()).toEqual(dataTrackingOptions.update); - }); - }); + it('sets tracking options for update alert', async () => { + createComponent(propsWithAlertData); + + modal().vm.$emit('shown'); + + findThresholdField().vm.$emit('input', 11); + + await wrapper.vm.$nextTick(); + + expect(submitButtonTrackingOpts()).toEqual(dataTrackingOptions.update); }); describe('alert runbooks feature flag', () => { it('hides the runbook field when the flag is disabled', () => { createComponent(undefined, { alertRunbooks: false }); - expect(alertRunbookField().exists()).toBe(false); + expect(findRunbookField().exists()).toBe(false); }); it('shows the runbook field when the flag is enabled', () => { createComponent(undefined, { alertRunbooks: true }); - expect(alertRunbookField().exists()).toBe(true); + expect(findRunbookField().exists()).toBe(true); }); }); }); diff --git a/spec/lib/gitlab/metrics/dashboard/validator/errors_spec.rb b/spec/lib/gitlab/metrics/dashboard/validator/errors_spec.rb index bf1629e33dd..f0db1bd0d33 100644 --- a/spec/lib/gitlab/metrics/dashboard/validator/errors_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/validator/errors_spec.rb @@ -4,28 +4,130 @@ require 'spec_helper' RSpec.describe Gitlab::Metrics::Dashboard::Validator::Errors do describe Gitlab::Metrics::Dashboard::Validator::Errors::SchemaValidationError do - context 'valid error hash from jsonschemer' do + context 'empty error hash' do + let(:error_hash) { {} } + + it 'uses default error message' do + expect(described_class.new(error_hash).message).to eq('Dashboard failed schema validation') + end + end + + context 'formatted message' do + subject { described_class.new(error_hash).message } + let(:error_hash) do { - 'data' => 'data', - 'data_pointer' => 'data_pointer', - 'schema' => 'schema', - 'schema_pointer' => 'schema_pointer' + 'data' => 'property_name', + 'data_pointer' => pointer, + 'type' => type, + 'schema' => 'schema', + 'details' => details } end - it 'formats message' do - expect(described_class.new(error_hash).message).to eq( - "'data' is invalid at 'data_pointer'. Should be 'schema' due to schema definition at 'schema_pointer'" - ) + context 'for root object' do + let(:pointer) { '' } + + context 'when required keys are missing' do + let(:type) { 'required' } + let(:details) { { 'missing_keys' => ['one'] } } + + it { is_expected.to eq 'root is missing required keys: one' } + end end - end - context 'empty error hash' do - let(:error_hash) { {} } + context 'for nested object' do + let(:pointer) { '/nested_objects/0' } - it 'uses default error message' do - expect(described_class.new(error_hash).message).to eq('Dashboard failed schema validation') + context 'when required keys are missing' do + let(:type) { 'required' } + let(:details) { { 'missing_keys' => ['two'] } } + + it { is_expected.to eq '/nested_objects/0 is missing required keys: two' } + end + + context 'when there is type mismatch' do + %w(null string boolean integer number array object).each do |expected_type| + context "on type: #{expected_type}" do + let(:type) { expected_type } + let(:details) { nil } + + subject { described_class.new(error_hash).message } + + it { is_expected.to eq "'property_name' at /nested_objects/0 is not of type: #{expected_type}" } + end + end + end + + context 'when data does not match pattern' do + let(:type) { 'pattern' } + let(:error_hash) do + { + 'data' => 'property_name', + 'data_pointer' => pointer, + 'type' => type, + 'schema' => { 'pattern' => 'aa.*' } + } + end + + it { is_expected.to eq "'property_name' at /nested_objects/0 does not match pattern: aa.*" } + end + + context 'when data does not match format' do + let(:type) { 'format' } + let(:error_hash) do + { + 'data' => 'property_name', + 'data_pointer' => pointer, + 'type' => type, + 'schema' => { 'format' => 'date-time' } + } + end + + it { is_expected.to eq "'property_name' at /nested_objects/0 does not match format: date-time" } + end + + context 'when data is not const' do + let(:type) { 'const' } + let(:error_hash) do + { + 'data' => 'property_name', + 'data_pointer' => pointer, + 'type' => type, + 'schema' => { 'const' => 'one' } + } + end + + it { is_expected.to eq "'property_name' at /nested_objects/0 is not: \"one\"" } + end + + context 'when data is not included in enum' do + let(:type) { 'enum' } + let(:error_hash) do + { + 'data' => 'property_name', + 'data_pointer' => pointer, + 'type' => type, + 'schema' => { 'enum' => %w(one two) } + } + end + + it { is_expected.to eq "'property_name' at /nested_objects/0 is not one of: [\"one\", \"two\"]" } + end + + context 'when data is not included in enum' do + let(:type) { 'unknown' } + let(:error_hash) do + { + 'data' => 'property_name', + 'data_pointer' => pointer, + 'type' => type, + 'schema' => 'schema' + } + end + + it { is_expected.to eq "'property_name' at /nested_objects/0 is invalid: error_type=unknown" } + end end end end diff --git a/spec/lib/gitlab/metrics/dashboard/validator_spec.rb b/spec/lib/gitlab/metrics/dashboard/validator_spec.rb index 22c6ec310e2..338d6fdcd89 100644 --- a/spec/lib/gitlab/metrics/dashboard/validator_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/validator_spec.rb @@ -34,6 +34,15 @@ RSpec.describe Gitlab::Metrics::Dashboard::Validator do end describe '#validate!' do + shared_examples 'validation failed' do |errors_message| + it 'raises error with corresponding messages', :aggregate_failures do + expect { subject }.to raise_error do |error| + expect(error).to be_kind_of(Gitlab::Metrics::Dashboard::Validator::Errors::InvalidDashboardError) + expect(error.message).to eq(errors_message) + end + end + end + context 'valid dashboard' do it 'returns true' do expect(described_class.validate!(valid_dashboard)).to be true @@ -41,22 +50,37 @@ RSpec.describe Gitlab::Metrics::Dashboard::Validator do end context 'invalid dashboard' do + subject { described_class.validate!(invalid_dashboard) } + context 'invalid schema' do - it 'raises error' do - expect { described_class.validate!(invalid_dashboard) } - .to raise_error(Gitlab::Metrics::Dashboard::Validator::Errors::InvalidDashboardError, - "'this_should_be_a_int' is invalid at '/panel_groups/0/panels/0/weight'."\ - " Should be '{\"type\"=>\"number\"}' due to schema definition at '/properties/weight'") + context 'wrong property type' do + it_behaves_like 'validation failed', "'this_should_be_a_int' at /panel_groups/0/panels/0/weight is not of type: number" + end + + context 'panel groups missing' do + let_it_be(:invalid_dashboard) { load_dashboard_yaml(fixture_file('lib/gitlab/metrics/dashboard/dashboard_missing_panel_groups.yml')) } + + it_behaves_like 'validation failed', 'root is missing required keys: panel_groups' + end + + context 'groups are missing panels and group keys' do + let_it_be(:invalid_dashboard) { load_dashboard_yaml(fixture_file('lib/gitlab/metrics/dashboard/dashboard_groups_missing_panels_and_group.yml')) } + + it_behaves_like 'validation failed', '/panel_groups/0 is missing required keys: group' + end + + context 'panel is missing metrics key' do + let_it_be(:invalid_dashboard) { load_dashboard_yaml(fixture_file('lib/gitlab/metrics/dashboard/dashboard_panel_is_missing_metrics.yml')) } + + it_behaves_like 'validation failed', '/panel_groups/0/panels/0 is missing required keys: metrics' end end context 'duplicate metric ids' do context 'with no project given' do - it 'checks against given dashboard and returns false' do - expect { described_class.validate!(duplicate_id_dashboard) } - .to raise_error(Gitlab::Metrics::Dashboard::Validator::Errors::InvalidDashboardError, - "metric_id must be unique across a project") - end + subject { described_class.validate!(duplicate_id_dashboard) } + + it_behaves_like 'validation failed', 'metric_id must be unique across a project' end end end |
