From faa19db44a5a4d0fb7a3be07319ca6201caa185a Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 1 Aug 2022 15:11:13 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .rubocop_todo/gitlab/namespaced_class.yml | 3 + .rubocop_todo/layout/hash_alignment.yml | 2 - CHANGELOG.md | 15 +++ GITALY_SERVER_VERSION | 2 +- app/mailers/abuse_report_mailer.rb | 4 +- app/mailers/emails/projects.rb | 6 +- app/models/bulk_imports/configuration.rb | 2 +- app/models/ci/bridge.rb | 2 +- app/models/ci/build.rb | 8 +- app/models/ci/build_trace_metadata.rb | 4 +- app/models/ci/job_artifact.rb | 52 ++++------ app/models/ci/pipeline.rb | 18 ++-- app/models/compare.rb | 2 +- app/models/concerns/ci/has_status.rb | 4 +- .../enums/data_visualization_palette.rb | 22 ++--- app/models/concerns/featurable.rb | 6 +- app/models/concerns/issuable.rb | 2 +- app/models/concerns/triggerable_hooks.rb | 30 +++--- app/models/container_repository.rb | 2 +- app/models/design_management/design.rb | 4 +- app/models/design_management/design_action.rb | 2 +- app/models/event.rb | 42 ++++---- app/models/grafana_integration.rb | 4 +- app/models/group.rb | 2 +- app/models/hooks/web_hook.rb | 8 +- app/models/integrations/emails_on_push.rb | 2 +- app/models/integrations/jira.rb | 2 +- app/models/issuable_severity.rb | 16 ++-- app/models/jira_connect_installation.rb | 4 +- .../loose_foreign_keys/deleted_record.rb | 35 +++---- app/models/merge_request.rb | 20 ++-- app/models/merge_request_diff.rb | 8 +- app/models/pages_domain.rb | 4 +- app/models/project.rb | 2 +- app/models/prometheus_alert.rb | 2 +- app/models/serverless/domain_cluster.rb | 2 +- app/models/snippet.rb | 4 +- app/models/terraform/state.rb | 2 +- app/models/user.rb | 18 ++-- app/models/user_status.rb | 12 +-- app/models/work_items/type.rb | 8 +- app/policies/project_hook_policy.rb | 10 ++ app/policies/system_hook_policy.rb | 8 ++ .../web_hooks/admin_destroy_service.rb | 20 ++++ app/services/web_hooks/destroy_service.rb | 24 ++++- .../projects/commits/_commit_list.html.haml | 21 ++-- .../fix_sliding_list_partitioning.yml | 8 -- doc/development/fe_guide/widgets.md | 2 +- .../merge_request_concepts/index.md | 29 +++--- .../widget_extensions.md | 96 ++----------------- .../new_fe_guide/modules/widget_extensions.md | 93 ++++++++++++++++-- .../ci/reports/coverage_report_generator.rb | 2 +- .../partitioning/sliding_list_strategy.rb | 1 - .../exclusive_lease_helpers/sleeping_lock.rb | 6 +- lib/tasks/gitlab/web_hook.rake | 11 ++- .../sliding_list_strategy_spec.rb | 72 ++++++-------- .../sleeping_lock_spec.rb | 22 +++++ spec/models/ci/build_spec.rb | 2 +- spec/policies/project_hook_policy_spec.rb | 31 ++++++ spec/policies/system_hook_policy_spec.rb | 29 ++++++ .../web_hooks/destroy_service_spec.rb | 75 ++++++++------- spec/tasks/gitlab/web_hook_rake_spec.rb | 4 + 62 files changed, 542 insertions(+), 413 deletions(-) create mode 100644 app/policies/project_hook_policy.rb create mode 100644 app/policies/system_hook_policy.rb create mode 100644 app/services/web_hooks/admin_destroy_service.rb delete mode 100644 config/feature_flags/development/fix_sliding_list_partitioning.yml create mode 100644 spec/policies/project_hook_policy_spec.rb create mode 100644 spec/policies/system_hook_policy_spec.rb diff --git a/.rubocop_todo/gitlab/namespaced_class.yml b/.rubocop_todo/gitlab/namespaced_class.yml index ac5edc05265..ef87efb666a 100644 --- a/.rubocop_todo/gitlab/namespaced_class.yml +++ b/.rubocop_todo/gitlab/namespaced_class.yml @@ -402,12 +402,14 @@ Gitlab/NamespacedClass: - 'app/policies/project_policy.rb' - 'app/policies/project_snippet_policy.rb' - 'app/policies/project_statistics_policy.rb' + - 'app/policies/project_hook_policy.rb' - 'app/policies/prometheus_alert_policy.rb' - 'app/policies/protected_branch_policy.rb' - 'app/policies/release_policy.rb' - 'app/policies/repository_policy.rb' - 'app/policies/resource_label_event_policy.rb' - 'app/policies/suggestion_policy.rb' + - 'app/policies/system_hook_policy.rb' - 'app/policies/timebox_policy.rb' - 'app/policies/timelog_policy.rb' - 'app/policies/todo_policy.rb' @@ -968,6 +970,7 @@ Gitlab/NamespacedClass: - 'ee/app/policies/push_rule_policy.rb' - 'ee/app/policies/saml_provider_policy.rb' - 'ee/app/policies/vulnerability_policy.rb' + - 'ee/app/policies/group_hook_policy.rb' - 'ee/app/presenters/approval_rule_presenter.rb' - 'ee/app/presenters/audit_event_presenter.rb' - 'ee/app/presenters/epic_issue_presenter.rb' diff --git a/.rubocop_todo/layout/hash_alignment.yml b/.rubocop_todo/layout/hash_alignment.yml index fcfffbac505..264619830f4 100644 --- a/.rubocop_todo/layout/hash_alignment.yml +++ b/.rubocop_todo/layout/hash_alignment.yml @@ -141,8 +141,6 @@ Layout/HashAlignment: - 'app/helpers/sorting_helper.rb' - 'app/helpers/todos_helper.rb' - 'app/helpers/wiki_helper.rb' - - 'app/mailers/abuse_report_mailer.rb' - - 'app/mailers/emails/projects.rb' - 'app/models/bulk_imports/configuration.rb' - 'app/models/ci/bridge.rb' - 'app/models/ci/build_trace_metadata.rb' diff --git a/CHANGELOG.md b/CHANGELOG.md index d3bc7625bf5..fa13dc64d47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 15.2.2 (2022-08-01) + +### Fixed (6 changes) + +- [Upgrade Oj to v3.13.19 to fix a seg fault](gitlab-org/gitlab@758dff584369303f4176a96ac130954724a0e9f5) ([merge request](gitlab-org/gitlab!93652)) +- [Gracefully handle nil created_at values in CI pipelines](gitlab-org/gitlab@492a3dcf6c37f8968282a93cf485f2358ecd7943) ([merge request](gitlab-org/gitlab!93652)) +- [Fix CI artifact sizes not logged for some runner endpoints](gitlab-org/gitlab@97ca5e38b1917239bb62cbd338eb689a0ff15fbb) ([merge request](gitlab-org/gitlab!93652)) +- [Fix RescheduleBackfillImportedIssueSearchData migration](gitlab-org/gitlab@015e908479a44276833e5bb40d6bd613c394f460) ([merge request](gitlab-org/gitlab!93652)) +- [Upgrade oj to v3.3.18 to fix illegal instruction errors](gitlab-org/gitlab@5caf005e1315f7acd145bcbb6d5ced5281a10e56) ([merge request](gitlab-org/gitlab!93652)) +- [Use `CREATE OR REPLACE FUNCTION` to define vulnerability reads triggers](gitlab-org/gitlab@dbfa0d51a1851e940ad243f720cbfe1e25c76111) ([merge request](gitlab-org/gitlab!93652)) + +### Changed (1 change) + +- [Fix ES client for nil password](gitlab-org/gitlab@9bd4fa109c06959f5e9b4668c85327e3503bf55a) ([merge request](gitlab-org/gitlab!93652)) **GitLab Enterprise Edition** + ## 15.2.1 (2022-07-28) ### Security (18 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 3e9adc045d5..826d98cc4a7 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -2720ae51a9acae55951c54268649a5cd0e2fa5ba +3eac3e7f6535d6121cb7790870c169d4d3ed736e diff --git a/app/mailers/abuse_report_mailer.rb b/app/mailers/abuse_report_mailer.rb index 20aabb6fe58..1fa85064c57 100644 --- a/app/mailers/abuse_report_mailer.rb +++ b/app/mailers/abuse_report_mailer.rb @@ -11,8 +11,8 @@ class AbuseReportMailer < ApplicationMailer @abuse_report = AbuseReport.find(abuse_report_id) mail( - to: Gitlab::CurrentSettings.abuse_notification_email, - subject: "#{@abuse_report.user.name} (#{@abuse_report.user.username}) was reported for abuse" + to: Gitlab::CurrentSettings.abuse_notification_email, + subject: "#{@abuse_report.user.name} (#{@abuse_report.user.username}) was reported for abuse" ) end diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index ed3fa28b15f..5b8471abb0f 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -51,9 +51,9 @@ module Emails add_project_headers headers['X-GitLab-Author'] = @message.author_username - mail(from: sender(@message.author_id, send_from_user_email: @message.send_from_committer_email?), - reply_to: @message.reply_to, - subject: @message.subject) + mail(from: sender(@message.author_id, send_from_user_email: @message.send_from_committer_email?), + reply_to: @message.reply_to, + subject: @message.subject) end def prometheus_alert_fired_email(project, user, alert) diff --git a/app/models/bulk_imports/configuration.rb b/app/models/bulk_imports/configuration.rb index 6d9f598583e..3b263ed0340 100644 --- a/app/models/bulk_imports/configuration.rb +++ b/app/models/bulk_imports/configuration.rb @@ -9,7 +9,7 @@ class BulkImports::Configuration < ApplicationRecord validates :url, :access_token, length: { maximum: 255 }, presence: true validates :url, public_url: { schemes: %w[http https], enforce_sanitization: true, ascii_only: true }, - allow_nil: true + allow_nil: true attr_encrypted :url, key: Settings.attr_encrypted_db_key_base_32, diff --git a/app/models/ci/bridge.rb b/app/models/ci/bridge.rb index 13af5b1f8d1..80d6d5bff4f 100644 --- a/app/models/ci/bridge.rb +++ b/app/models/ci/bridge.rb @@ -19,7 +19,7 @@ module Ci belongs_to :project belongs_to :trigger_request has_many :sourced_pipelines, class_name: "::Ci::Sources::Pipeline", - foreign_key: :source_job_id + foreign_key: :source_job_id has_one :sourced_pipeline, class_name: "::Ci::Sources::Pipeline", foreign_key: :source_job_id has_one :downstream_pipeline, through: :sourced_pipeline, source: :pipeline diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index fd830536535..0d2cda7c1b9 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -984,7 +984,7 @@ module Ci def collect_test_reports!(test_reports) test_reports.get_suite(test_suite_name).tap do |test_suite| - each_report(Ci::JobArtifact::TEST_REPORT_FILE_TYPES) do |file_type, blob| + each_report(Ci::JobArtifact::REPORT_FILE_TYPES[:test]) do |file_type, blob| Gitlab::Ci::Parsers.fabricate!(file_type).parse!( blob, test_suite, @@ -995,7 +995,7 @@ module Ci end def collect_accessibility_reports!(accessibility_report) - each_report(Ci::JobArtifact::ACCESSIBILITY_REPORT_FILE_TYPES) do |file_type, blob| + each_report(Ci::JobArtifact::REPORT_FILE_TYPES[:accessibility]) do |file_type, blob| Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, accessibility_report) end @@ -1003,7 +1003,7 @@ module Ci end def collect_codequality_reports!(codequality_report) - each_report(Ci::JobArtifact::CODEQUALITY_REPORT_FILE_TYPES) do |file_type, blob| + each_report(Ci::JobArtifact::REPORT_FILE_TYPES[:codequality]) do |file_type, blob| Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, codequality_report) end @@ -1011,7 +1011,7 @@ module Ci end def collect_terraform_reports!(terraform_reports) - each_report(::Ci::JobArtifact::TERRAFORM_REPORT_FILE_TYPES) do |file_type, blob, report_artifact| + each_report(::Ci::JobArtifact::REPORT_FILE_TYPES[:terraform]) do |file_type, blob, report_artifact| ::Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, terraform_reports, artifact: report_artifact) end diff --git a/app/models/ci/build_trace_metadata.rb b/app/models/ci/build_trace_metadata.rb index 1ffa0e31f99..86de90983ff 100644 --- a/app/models/ci/build_trace_metadata.rb +++ b/app/models/ci/build_trace_metadata.rb @@ -39,8 +39,8 @@ module Ci def track_archival!(trace_artifact_id, checksum) update!(trace_artifact_id: trace_artifact_id, - checksum: checksum, - archived_at: Time.current) + checksum: checksum, + archived_at: Time.current) end def archival_attempts_message diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index ee7175a4f69..3a03791a70c 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -13,14 +13,18 @@ module Ci include EachBatch include Gitlab::Utils::StrongMemoize - TEST_REPORT_FILE_TYPES = %w[junit].freeze - COVERAGE_REPORT_FILE_TYPES = %w[cobertura].freeze - CODEQUALITY_REPORT_FILE_TYPES = %w[codequality].freeze - ACCESSIBILITY_REPORT_FILE_TYPES = %w[accessibility].freeze NON_ERASABLE_FILE_TYPES = %w[trace].freeze - TERRAFORM_REPORT_FILE_TYPES = %w[terraform].freeze - SAST_REPORT_TYPES = %w[sast].freeze - SECRET_DETECTION_REPORT_TYPES = %w[secret_detection].freeze + + REPORT_FILE_TYPES = { + sast: %w[sast], + secret_detection: %w[secret_detection], + test: %w[junit], + accessibility: %w[accessibility], + coverage: %w[cobertura], + codequality: %w[codequality], + terraform: %w[terraform] + }.freeze + DEFAULT_FILE_NAMES = { archive: nil, metadata: nil, @@ -152,38 +156,16 @@ module Ci where(file_type: types) end + REPORT_FILE_TYPES.each do |report_type, file_types| + scope "#{report_type}_reports", -> do + with_file_types(file_types) + end + end + scope :all_reports, -> do with_file_types(REPORT_TYPES.keys.map(&:to_s)) end - scope :sast_reports, -> do - with_file_types(SAST_REPORT_TYPES) - end - - scope :secret_detection_reports, -> do - with_file_types(SECRET_DETECTION_REPORT_TYPES) - end - - scope :test_reports, -> do - with_file_types(TEST_REPORT_FILE_TYPES) - end - - scope :accessibility_reports, -> do - with_file_types(ACCESSIBILITY_REPORT_FILE_TYPES) - end - - scope :coverage_reports, -> do - with_file_types(COVERAGE_REPORT_FILE_TYPES) - end - - scope :codequality_reports, -> do - with_file_types(CODEQUALITY_REPORT_FILE_TYPES) - end - - scope :terraform_reports, -> do - with_file_types(TERRAFORM_REPORT_FILE_TYPES) - end - scope :erasable, -> do where(file_type: self.erasable_file_types) end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 81ce3f82796..14cd53d17cd 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -52,15 +52,15 @@ module Ci belongs_to :ci_ref, class_name: 'Ci::Ref', foreign_key: :ci_ref_id, inverse_of: :pipelines has_internal_id :iid, scope: :project, presence: false, - track_if: -> { !importing? }, - ensure_if: -> { !importing? }, - init: ->(pipeline, scope) do - if pipeline - pipeline.project&.all_pipelines&.maximum(:iid) || pipeline.project&.all_pipelines&.count - elsif scope - ::Ci::Pipeline.where(**scope).maximum(:iid) - end - end + track_if: -> { !importing? }, + ensure_if: -> { !importing? }, + init: ->(pipeline, scope) do + if pipeline + pipeline.project&.all_pipelines&.maximum(:iid) || pipeline.project&.all_pipelines&.count + elsif scope + ::Ci::Pipeline.where(**scope).maximum(:iid) + end + end has_many :stages, -> { order(position: :asc) }, inverse_of: :pipeline has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline diff --git a/app/models/compare.rb b/app/models/compare.rb index 7f42e1ee491..f594a796987 100644 --- a/app/models/compare.rb +++ b/app/models/compare.rb @@ -92,7 +92,7 @@ class Compare def diff_refs Gitlab::Diff::DiffRefs.new( - base_sha: @straight ? start_commit_sha : base_commit_sha, + base_sha: @straight ? start_commit_sha : base_commit_sha, start_sha: start_commit_sha, head_sha: head_commit_sha ) diff --git a/app/models/concerns/ci/has_status.rb b/app/models/concerns/ci/has_status.rb index 721cb14201f..910885c833f 100644 --- a/app/models/concerns/ci/has_status.rb +++ b/app/models/concerns/ci/has_status.rb @@ -17,8 +17,8 @@ module Ci ALIVE_STATUSES = (ACTIVE_STATUSES + ['created']).freeze CANCELABLE_STATUSES = (ALIVE_STATUSES + ['scheduled']).freeze STATUSES_ENUM = { created: 0, pending: 1, running: 2, success: 3, - failed: 4, canceled: 5, skipped: 6, manual: 7, - scheduled: 8, preparing: 9, waiting_for_resource: 10 }.freeze + failed: 4, canceled: 5, skipped: 6, manual: 7, + scheduled: 8, preparing: 9, waiting_for_resource: 10 }.freeze UnknownStatusError = Class.new(StandardError) diff --git a/app/models/concerns/enums/data_visualization_palette.rb b/app/models/concerns/enums/data_visualization_palette.rb index 25002e64ba6..6e712e79915 100644 --- a/app/models/concerns/enums/data_visualization_palette.rb +++ b/app/models/concerns/enums/data_visualization_palette.rb @@ -16,17 +16,17 @@ module Enums def self.weights { - '50' => 0, - '100' => 1, - '200' => 2, - '300' => 3, - '400' => 4, - '500' => 5, - '600' => 6, - '700' => 7, - '800' => 8, - '900' => 9, - '950' => 10 + '50' => 0, + '100' => 1, + '200' => 2, + '300' => 3, + '400' => 4, + '500' => 5, + '600' => 6, + '700' => 7, + '800' => 8, + '900' => 9, + '950' => 10 } end end diff --git a/app/models/concerns/featurable.rb b/app/models/concerns/featurable.rb index 08189d83534..0028404e3b6 100644 --- a/app/models/concerns/featurable.rb +++ b/app/models/concerns/featurable.rb @@ -30,9 +30,9 @@ module Featurable STRING_OPTIONS = HashWithIndifferentAccess.new({ 'disabled' => DISABLED, - 'private' => PRIVATE, - 'enabled' => ENABLED, - 'public' => PUBLIC + 'private' => PRIVATE, + 'enabled' => ENABLED, + 'public' => PUBLIC }).freeze class_methods do diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 4dca07132ef..da27757a3ef 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -550,7 +550,7 @@ module Issuable # Returns a Hash of attributes to be used for Twitter card metadata def card_attributes { - 'Author' => author.try(:name), + 'Author' => author.try(:name), 'Assignee' => assignee_list } end diff --git a/app/models/concerns/triggerable_hooks.rb b/app/models/concerns/triggerable_hooks.rb index 8fe34632430..e3800caa43f 100644 --- a/app/models/concerns/triggerable_hooks.rb +++ b/app/models/concerns/triggerable_hooks.rb @@ -2,22 +2,22 @@ module TriggerableHooks AVAILABLE_TRIGGERS = { - repository_update_hooks: :repository_update_events, - push_hooks: :push_events, - tag_push_hooks: :tag_push_events, - issue_hooks: :issues_events, - confidential_note_hooks: :confidential_note_events, + repository_update_hooks: :repository_update_events, + push_hooks: :push_events, + tag_push_hooks: :tag_push_events, + issue_hooks: :issues_events, + confidential_note_hooks: :confidential_note_events, confidential_issue_hooks: :confidential_issues_events, - note_hooks: :note_events, - merge_request_hooks: :merge_requests_events, - job_hooks: :job_events, - pipeline_hooks: :pipeline_events, - wiki_page_hooks: :wiki_page_events, - deployment_hooks: :deployment_events, - feature_flag_hooks: :feature_flag_events, - release_hooks: :releases_events, - member_hooks: :member_events, - subgroup_hooks: :subgroup_events + note_hooks: :note_events, + merge_request_hooks: :merge_requests_events, + job_hooks: :job_events, + pipeline_hooks: :pipeline_events, + wiki_page_hooks: :wiki_page_events, + deployment_hooks: :deployment_events, + feature_flag_hooks: :feature_flag_events, + release_hooks: :releases_events, + member_hooks: :member_events, + subgroup_hooks: :subgroup_events }.freeze extend ActiveSupport::Concern diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb index cdfd24e00aa..1aca763d57f 100644 --- a/app/models/container_repository.rb +++ b/app/models/container_repository.rb @@ -550,7 +550,7 @@ class ContainerRepository < ApplicationRecord def self.find_by_path(path) self.find_by(project: path.repository_project, - name: path.repository_name) + name: path.repository_name) end private diff --git a/app/models/design_management/design.rb b/app/models/design_management/design.rb index feb1bf5438c..317399e780a 100644 --- a/app/models/design_management/design.rb +++ b/app/models/design_management/design.rb @@ -28,8 +28,8 @@ module DesignManagement has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_internal_id :iid, scope: :project, presence: true, - hook_names: %i[create update], # Deal with old records - track_if: -> { !importing? } + hook_names: %i[create update], # Deal with old records + track_if: -> { !importing? } validates :project, :filename, presence: true validates :issue, presence: true, unless: :importing? diff --git a/app/models/design_management/design_action.rb b/app/models/design_management/design_action.rb index 43dcce545d2..eae470a1ae2 100644 --- a/app/models/design_management/design_action.rb +++ b/app/models/design_management/design_action.rb @@ -21,7 +21,7 @@ module DesignManagement validates :action, presence: true, inclusion: { in: EVENT_FOR_GITALY_ACTION.keys } validates :content, absence: { if: :forbids_content?, - message: 'this action forbids content' }, + message: 'this action forbids content' }, presence: { if: :needs_content?, message: 'this action needs content' } diff --git a/app/models/event.rb b/app/models/event.rb index 7760be3e817..5349ad029ec 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -14,18 +14,18 @@ class Event < ApplicationRecord default_scope { reorder(nil) } # rubocop:disable Cop/DefaultScope ACTIONS = HashWithIndifferentAccess.new( - created: 1, - updated: 2, - closed: 3, - reopened: 4, - pushed: 5, - commented: 6, - merged: 7, - joined: 8, # User joined project - left: 9, # User left project - destroyed: 10, - expired: 11, # User left project due to expiry - approved: 12 + created: 1, + updated: 2, + closed: 3, + reopened: 4, + pushed: 5, + commented: 6, + merged: 7, + joined: 8, # User joined project + left: 9, # User left project + destroyed: 10, + expired: 11, # User left project due to expiry + approved: 12 ).freeze private_constant :ACTIONS @@ -36,15 +36,15 @@ class Event < ApplicationRecord ISSUE_ACTIONS = [:created, :updated, :closed, :reopened].freeze TARGET_TYPES = HashWithIndifferentAccess.new( - issue: Issue, - milestone: Milestone, - merge_request: MergeRequest, - note: Note, - project: Project, - snippet: Snippet, - user: User, - wiki: WikiPage::Meta, - design: DesignManagement::Design + issue: Issue, + milestone: Milestone, + merge_request: MergeRequest, + note: Note, + project: Project, + snippet: Snippet, + user: User, + wiki: WikiPage::Meta, + design: DesignManagement::Design ).freeze RESET_PROJECT_ACTIVITY_INTERVAL = 1.hour diff --git a/app/models/grafana_integration.rb b/app/models/grafana_integration.rb index 0358e37c58b..5cd5aa1b085 100644 --- a/app/models/grafana_integration.rb +++ b/app/models/grafana_integration.rb @@ -4,9 +4,9 @@ class GrafanaIntegration < ApplicationRecord belongs_to :project attr_encrypted :token, - mode: :per_attribute_iv, + mode: :per_attribute_iv, algorithm: 'aes-256-gcm', - key: Settings.attr_encrypted_db_key_base_32 + key: Settings.attr_encrypted_db_key_base_32 before_validation :check_token_changes diff --git a/app/models/group.rb b/app/models/group.rb index 122fb48a34b..07badd18965 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -149,7 +149,7 @@ class Group < Namespace add_authentication_token_field :runners_token, encrypted: -> { Feature.enabled?(:groups_tokens_optional_encryption) ? :optional : :required }, - prefix: RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX + prefix: RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX after_create :post_create_hook after_destroy :post_destroy_hook diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index 925a8628196..84ee23d77ce 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -12,14 +12,14 @@ class WebHook < ApplicationRecord BACKOFF_GROWTH_FACTOR = 2.0 attr_encrypted :token, - mode: :per_attribute_iv, + mode: :per_attribute_iv, algorithm: 'aes-256-gcm', - key: Settings.attr_encrypted_db_key_base_32 + key: Settings.attr_encrypted_db_key_base_32 attr_encrypted :url, - mode: :per_attribute_iv, + mode: :per_attribute_iv, algorithm: 'aes-256-gcm', - key: Settings.attr_encrypted_db_key_base_32 + key: Settings.attr_encrypted_db_key_base_32 attr_encrypted :url_variables, mode: :per_attribute_iv, diff --git a/app/models/integrations/emails_on_push.rb b/app/models/integrations/emails_on_push.rb index ed12a3a8d63..25bda8c2bf0 100644 --- a/app/models/integrations/emails_on_push.rb +++ b/app/models/integrations/emails_on_push.rb @@ -71,7 +71,7 @@ module Integrations recipients, push_data, send_from_committer_email: send_from_committer_email?, - disable_diffs: disable_diffs? + disable_diffs: disable_diffs? ) end diff --git a/app/models/integrations/jira.rb b/app/models/integrations/jira.rb index 566bbc456f8..5f68f206e15 100644 --- a/app/models/integrations/jira.rb +++ b/app/models/integrations/jira.rb @@ -505,7 +505,7 @@ module Integrations self.project, entity_type.to_sym ], - id: entity_id, + id: entity_id, host: Settings.gitlab.base_url ) end diff --git a/app/models/issuable_severity.rb b/app/models/issuable_severity.rb index 928301e1da6..cd7e5fafb60 100644 --- a/app/models/issuable_severity.rb +++ b/app/models/issuable_severity.rb @@ -3,18 +3,18 @@ class IssuableSeverity < ApplicationRecord DEFAULT = 'unknown' SEVERITY_LABELS = { - unknown: 'Unknown', - low: 'Low - S4', - medium: 'Medium - S3', - high: 'High - S2', + unknown: 'Unknown', + low: 'Low - S4', + medium: 'Medium - S3', + high: 'High - S2', critical: 'Critical - S1' }.freeze SEVERITY_QUICK_ACTION_PARAMS = { - unknown: %w(Unknown 0), - low: %w(Low S4 4), - medium: %w(Medium S3 3), - high: %w(High S2 2), + unknown: %w(Unknown 0), + low: %w(Low S4 4), + medium: %w(Medium S3 3), + high: %w(High S2 2), critical: %w(Critical S1 1) }.freeze diff --git a/app/models/jira_connect_installation.rb b/app/models/jira_connect_installation.rb index e34543534f3..8befe9a9230 100644 --- a/app/models/jira_connect_installation.rb +++ b/app/models/jira_connect_installation.rb @@ -2,9 +2,9 @@ class JiraConnectInstallation < ApplicationRecord attr_encrypted :shared_secret, - mode: :per_attribute_iv, + mode: :per_attribute_iv, algorithm: 'aes-256-gcm', - key: Settings.attr_encrypted_db_key_base_32 + key: Settings.attr_encrypted_db_key_base_32 has_many :subscriptions, class_name: 'JiraConnectSubscription' diff --git a/app/models/loose_foreign_keys/deleted_record.rb b/app/models/loose_foreign_keys/deleted_record.rb index 6dfd6ea2aae..8534e26e8ff 100644 --- a/app/models/loose_foreign_keys/deleted_record.rb +++ b/app/models/loose_foreign_keys/deleted_record.rb @@ -9,26 +9,27 @@ class LooseForeignKeys::DeletedRecord < Gitlab::Database::SharedModel self.ignored_columns = %i[partition] partitioned_by :partition, strategy: :sliding_list, - next_partition_if: -> (active_partition) do - return false if Feature.disabled?(:lfk_automatic_partition_creation) + next_partition_if: -> (active_partition) do + return false if Feature.disabled?(:lfk_automatic_partition_creation) - oldest_record_in_partition = LooseForeignKeys::DeletedRecord - .select(:id, :created_at) - .for_partition(active_partition) - .order(:id) - .limit(1) - .take + oldest_record_in_partition = LooseForeignKeys::DeletedRecord + .select(:id, :created_at) + .for_partition(active_partition) + .order(:id) + .limit(1) + .take - oldest_record_in_partition.present? && oldest_record_in_partition.created_at < PARTITION_DURATION.ago - end, - detach_partition_if: -> (partition) do - return false if Feature.disabled?(:lfk_automatic_partition_dropping) + oldest_record_in_partition.present? && + oldest_record_in_partition.created_at < PARTITION_DURATION.ago + end, + detach_partition_if: -> (partition) do + return false if Feature.disabled?(:lfk_automatic_partition_dropping) - !LooseForeignKeys::DeletedRecord - .for_partition(partition) - .status_pending - .exists? - end + !LooseForeignKeys::DeletedRecord + .for_partition(partition) + .status_pending + .exists? + end scope :for_table, -> (table) { where(fully_qualified_table_name: table) } scope :for_partition, -> (partition) { where(partition: partition) } diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 47dd0175705..aab861648d6 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -37,7 +37,7 @@ class MergeRequest < ApplicationRecord SORTING_PREFERENCE_FIELD = :merge_requests_sort ALLOWED_TO_USE_MERGE_BASE_PIPELINE_FOR_COMPARISON = { - 'Ci::CompareMetricsReportsService' => ->(project) { true }, + 'Ci::CompareMetricsReportsService' => ->(project) { true }, 'Ci::CompareCodequalityReportsService' => ->(project) { true } }.freeze @@ -47,13 +47,13 @@ class MergeRequest < ApplicationRecord belongs_to :iteration, foreign_key: 'sprint_id' has_internal_id :iid, scope: :target_project, track_if: -> { !importing? }, - init: ->(mr, scope) do - if mr - mr.target_project&.merge_requests&.maximum(:iid) - elsif scope[:project] - where(target_project: scope[:project]).maximum(:iid) - end - end + init: ->(mr, scope) do + if mr + mr.target_project&.merge_requests&.maximum(:iid) + elsif scope[:project] + where(target_project: scope[:project]).maximum(:iid) + end + end has_many :merge_request_diffs, -> { regular }, inverse_of: :merge_request @@ -927,9 +927,9 @@ class MergeRequest < ApplicationRecord # most recent data possible. def repository_diff_refs Gitlab::Diff::DiffRefs.new( - base_sha: branch_merge_base_sha, + base_sha: branch_merge_base_sha, start_sha: target_branch_sha, - head_sha: source_branch_sha + head_sha: source_branch_sha ) end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index e08b2cc2a7d..89c61ea6ebd 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -358,9 +358,9 @@ class MergeRequestDiff < ApplicationRecord return unless start_commit_sha || base_commit_sha Gitlab::Diff::DiffRefs.new( - base_sha: base_commit_sha, + base_sha: base_commit_sha, start_sha: start_commit_sha, - head_sha: head_commit_sha + head_sha: head_commit_sha ) end @@ -381,9 +381,9 @@ class MergeRequestDiff < ApplicationRecord likely_base_commit_sha = (first_commit&.parent || first_commit)&.sha Gitlab::Diff::DiffRefs.new( - base_sha: likely_base_commit_sha, + base_sha: likely_base_commit_sha, start_sha: safe_start_commit_sha, - head_sha: head_commit_sha + head_sha: head_commit_sha ) end diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb index 9e93bff4acf..2e25839c47f 100644 --- a/app/models/pages_domain.rb +++ b/app/models/pages_domain.rb @@ -23,10 +23,10 @@ class PagesDomain < ApplicationRecord validates :domain, uniqueness: { case_sensitive: false } validates :certificate, :key, presence: true, if: :usage_serverless? validates :certificate, presence: { message: 'must be present if HTTPS-only is enabled' }, - if: :certificate_should_be_present? + if: :certificate_should_be_present? validates :certificate, certificate: true, if: ->(domain) { domain.certificate.present? } validates :key, presence: { message: 'must be present if HTTPS-only is enabled' }, - if: :certificate_should_be_present? + if: :certificate_should_be_present? validates :key, certificate_key: true, named_ecdsa_key: true, if: ->(domain) { domain.key.present? } validates :verification_code, presence: true, allow_blank: false diff --git a/app/models/project.rb b/app/models/project.rb index e2b0fdc4427..8e59a3599dd 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2658,7 +2658,7 @@ class Project < ApplicationRecord { repository_storage: repository_storage, - pool_repository: pool_repository || create_new_pool_repository + pool_repository: pool_repository || create_new_pool_repository } end diff --git a/app/models/prometheus_alert.rb b/app/models/prometheus_alert.rb index 684f50d5f58..9080f3d9de1 100644 --- a/app/models/prometheus_alert.rb +++ b/app/models/prometheus_alert.rb @@ -25,7 +25,7 @@ class PrometheusAlert < ApplicationRecord validates :environment, :project, :prometheus_metric, :threshold, :operator, presence: true validates :runbook_url, length: { maximum: 255 }, allow_blank: true, - addressable_url: { enforce_sanitization: true, ascii_only: true } + addressable_url: { enforce_sanitization: true, ascii_only: true } validate :require_valid_environment_project! validate :require_valid_metric_project! diff --git a/app/models/serverless/domain_cluster.rb b/app/models/serverless/domain_cluster.rb index 0d54a97370e..1effabf1c22 100644 --- a/app/models/serverless/domain_cluster.rb +++ b/app/models/serverless/domain_cluster.rb @@ -17,7 +17,7 @@ module Serverless validates :pages_domain, :knative, presence: true validates :uuid, presence: true, uniqueness: true, length: { is: ::Serverless::Domain::UUID_LENGTH }, - format: { with: HEX_REGEXP, message: 'only allows hex characters' } + format: { with: HEX_REGEXP, message: 'only allows hex characters' } default_value_for(:uuid, allows_nil: false) { ::Serverless::Domain.generate_uuid } diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 47b23bbd28a..fd882633a44 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -94,8 +94,8 @@ class Snippet < ApplicationRecord attr_spammable :content, spam_description: true attr_encrypted :secret_token, - key: Settings.attr_encrypted_db_key_base_truncated, - mode: :per_attribute_iv, + key: Settings.attr_encrypted_db_key_base_truncated, + mode: :per_attribute_iv, algorithm: 'aes-256-cbc' class << self diff --git a/app/models/terraform/state.rb b/app/models/terraform/state.rb index 59f7d852ce6..e5c8f4ab32a 100644 --- a/app/models/terraform/state.rb +++ b/app/models/terraform/state.rb @@ -26,7 +26,7 @@ module Terraform validates :project_id, :name, presence: true validates :uuid, presence: true, uniqueness: true, length: { is: UUID_LENGTH }, - format: { with: HEX_REGEXP, message: 'only allows hex characters' } + format: { with: HEX_REGEXP, message: 'only allows hex characters' } default_value_for(:uuid, allows_nil: false) { SecureRandom.hex(UUID_LENGTH / 2) } diff --git a/app/models/user.rb b/app/models/user.rb index f9f3de8e127..166ad2ebf1f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -69,8 +69,8 @@ class User < ApplicationRecord default_value_for :theme_id, gitlab_config.default_theme attr_encrypted :otp_secret, - key: Gitlab::Application.secrets.otp_key_base, - mode: :per_attribute_iv_and_salt, + key: Gitlab::Application.secrets.otp_key_base, + mode: :per_attribute_iv_and_salt, insecure_mode: true, algorithm: 'aes-256-cbc' @@ -272,10 +272,10 @@ class User < ApplicationRecord validate :check_username_format, if: :username_changed? validates :theme_id, allow_nil: true, inclusion: { in: Gitlab::Themes.valid_ids, - message: _("%{placeholder} is not a valid theme") % { placeholder: '%{value}' } } + message: _("%{placeholder} is not a valid theme") % { placeholder: '%{value}' } } validates :color_scheme_id, allow_nil: true, inclusion: { in: Gitlab::ColorSchemes.valid_ids, - message: _("%{placeholder} is not a valid color scheme") % { placeholder: '%{value}' } } + message: _("%{placeholder} is not a valid color scheme") % { placeholder: '%{value}' } } validates :website_url, allow_blank: true, url: true, if: :website_url_changed? @@ -991,12 +991,12 @@ class User < ApplicationRecord def disable_two_factor! transaction do update( - otp_required_for_login: false, - encrypted_otp_secret: nil, - encrypted_otp_secret_iv: nil, - encrypted_otp_secret_salt: nil, + otp_required_for_login: false, + encrypted_otp_secret: nil, + encrypted_otp_secret_iv: nil, + encrypted_otp_secret_salt: nil, otp_grace_period_started_at: nil, - otp_backup_codes: nil + otp_backup_codes: nil ) self.u2f_registrations.destroy_all # rubocop: disable Cop/DestroyAll self.webauthn_registrations.destroy_all # rubocop: disable Cop/DestroyAll diff --git a/app/models/user_status.rb b/app/models/user_status.rb index cb6f4dd9dae..dee976a4497 100644 --- a/app/models/user_status.rb +++ b/app/models/user_status.rb @@ -9,12 +9,12 @@ class UserStatus < ApplicationRecord CLEAR_STATUS_QUICK_OPTIONS = { '30_minutes' => 30.minutes, - '3_hours' => 3.hours, - '8_hours' => 8.hours, - '1_day' => 1.day, - '3_days' => 3.days, - '7_days' => 7.days, - '30_days' => 30.days + '3_hours' => 3.hours, + '8_hours' => 8.hours, + '1_day' => 1.day, + '3_days' => 3.days, + '7_days' => 7.days, + '30_days' => 30.days }.freeze belongs_to :user diff --git a/app/models/work_items/type.rb b/app/models/work_items/type.rb index af0462a9d8f..b05750f77fe 100644 --- a/app/models/work_items/type.rb +++ b/app/models/work_items/type.rb @@ -13,11 +13,11 @@ module WorkItems # Base types need to exist on the DB on app startup # This constant is used by the DB seeder BASE_TYPES = { - issue: { name: 'Issue', icon_name: 'issue-type-issue', enum_value: 0 }, - incident: { name: 'Incident', icon_name: 'issue-type-incident', enum_value: 1 }, - test_case: { name: 'Test Case', icon_name: 'issue-type-test-case', enum_value: 2 }, ## EE-only + issue: { name: 'Issue', icon_name: 'issue-type-issue', enum_value: 0 }, + incident: { name: 'Incident', icon_name: 'issue-type-incident', enum_value: 1 }, + test_case: { name: 'Test Case', icon_name: 'issue-type-test-case', enum_value: 2 }, ## EE-only requirement: { name: 'Requirement', icon_name: 'issue-type-requirements', enum_value: 3 }, ## EE-only - task: { name: 'Task', icon_name: 'issue-type-task', enum_value: 4 } + task: { name: 'Task', icon_name: 'issue-type-task', enum_value: 4 } }.freeze WIDGETS_FOR_TYPE = { diff --git a/app/policies/project_hook_policy.rb b/app/policies/project_hook_policy.rb new file mode 100644 index 00000000000..c177fabb1ba --- /dev/null +++ b/app/policies/project_hook_policy.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class ProjectHookPolicy < ::BasePolicy + delegate(:project) + + rule { can?(:admin_project) }.policy do + enable :read_web_hook + enable :destroy_web_hook + end +end diff --git a/app/policies/system_hook_policy.rb b/app/policies/system_hook_policy.rb new file mode 100644 index 00000000000..ec28d39a5fa --- /dev/null +++ b/app/policies/system_hook_policy.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class SystemHookPolicy < ::BasePolicy + rule { admin }.policy do + enable :read_web_hook + enable :destroy_web_hook + end +end diff --git a/app/services/web_hooks/admin_destroy_service.rb b/app/services/web_hooks/admin_destroy_service.rb new file mode 100644 index 00000000000..e9835801a39 --- /dev/null +++ b/app/services/web_hooks/admin_destroy_service.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module WebHooks + # A variant of the destroy service that can only be used by an administrator + # from a rake task. + class AdminDestroyService < WebHooks::DestroyService + def initialize(rake_task:) + super(nil) + @rake_task = rake_task + end + + def authorized?(web_hook) + @rake_task.present? # Not impossible to circumvent, but you need to provide something + end + + def log_message(hook) + "An administrator scheduled a deletion of logs for hook ID #{hook.id} from #{@rake_task.name}" + end + end +end diff --git a/app/services/web_hooks/destroy_service.rb b/app/services/web_hooks/destroy_service.rb index 54c6c7ea71b..dbd164ab20e 100644 --- a/app/services/web_hooks/destroy_service.rb +++ b/app/services/web_hooks/destroy_service.rb @@ -1,25 +1,41 @@ # frozen_string_literal: true module WebHooks + # Destroy a hook, and schedule the logs for deletion. class DestroyService + include Services::ReturnServiceResponses + attr_accessor :current_user + DENIED = 'Insufficient permissions' + def initialize(current_user) @current_user = current_user end - # Destroy the hook immediately, schedule the logs for deletion def execute(web_hook) + return error(DENIED, 401) unless authorized?(web_hook) + hook_id = web_hook.id if web_hook.destroy WebHooks::LogDestroyWorker.perform_async({ 'hook_id' => hook_id }) - Gitlab::AppLogger.info("User #{current_user&.id} scheduled a deletion of logs for hook ID #{hook_id}") + Gitlab::AppLogger.info(log_message(web_hook)) - ServiceResponse.success(payload: { async: false }) + success({ async: false }) else - ServiceResponse.error(message: "Unable to destroy #{web_hook.model_name.human}") + error("Unable to destroy #{web_hook.model_name.human}", 500) end end + + private + + def log_message(hook) + "User #{current_user&.id} scheduled a deletion of logs for hook ID #{hook.id}" + end + + def authorized?(web_hook) + Ability.allowed?(current_user, :destroy_web_hook, web_hook) + end end end diff --git a/app/views/projects/commits/_commit_list.html.haml b/app/views/projects/commits/_commit_list.html.haml index 6ed65d07202..23b25b5dcbd 100644 --- a/app/views/projects/commits/_commit_list.html.haml +++ b/app/views/projects/commits/_commit_list.html.haml @@ -2,14 +2,15 @@ - hidden = @hidden_commit_count - commits = Commit.decorate(commits, @project) -.card - .card-header += render Pajamas::CardComponent.new(card_options: { class: 'gl-mb-5'}, body_options: { class: 'gl-py-0'}) do |c| + - c.header do Commits (#{@total_commit_count}) - - if hidden > 0 - %ul.content-list - - commits.each do |commit| - = render "projects/commits/inline_commit", commit: commit, project: @project - %li.warning-row.unstyled - #{number_with_delimiter(hidden)} additional commits have been omitted to prevent performance issues. - - else - %ul.content-list= render commits, project: @project, ref: @ref + - c.body do + - if hidden > 0 + %ul.content-list + - commits.each do |commit| + = render "projects/commits/inline_commit", commit: commit, project: @project + %li.warning-row.unstyled + #{number_with_delimiter(hidden)} additional commits have been omitted to prevent performance issues. + - else + %ul.content-list= render commits, project: @project, ref: @ref diff --git a/config/feature_flags/development/fix_sliding_list_partitioning.yml b/config/feature_flags/development/fix_sliding_list_partitioning.yml deleted file mode 100644 index 7d553ea938c..00000000000 --- a/config/feature_flags/development/fix_sliding_list_partitioning.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: fix_sliding_list_partitioning -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/85222 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/362246 -milestone: '15.1' -type: development -group: group::sharding -default_enabled: true diff --git a/doc/development/fe_guide/widgets.md b/doc/development/fe_guide/widgets.md index d0b0183ab9d..d594f26bd87 100644 --- a/doc/development/fe_guide/widgets.md +++ b/doc/development/fe_guide/widgets.md @@ -144,4 +144,4 @@ methods: { ## Merge request widgets -Refer to the documentation specific to the [merge request widget extension framework](../merge_request_concepts/widget_extensions.md). +Refer to the documentation specific to the [merge request widget extension framework](../new_fe_guide/modules/widget_extensions.md). diff --git a/doc/development/merge_request_concepts/index.md b/doc/development/merge_request_concepts/index.md index 8600e3c6364..1320cd6f9f3 100644 --- a/doc/development/merge_request_concepts/index.md +++ b/doc/development/merge_request_concepts/index.md @@ -5,13 +5,11 @@ group: Code Review info: "See the Technical Writers assigned to Development Guidelines: https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments-to-development-guidelines" --- -# Merge Request Concepts +# Merge request concepts -**NOTE**: +NOTE: The documentation below is the single source of truth for the merge request terminology and functionality. -## Overview - The merge request is made up of several different key components and ideas that encompass the overall merge request experience. These concepts sometimes have competing and confusing terminology or overlap with other concepts. The concepts this will cover are: 1. Merge widget @@ -19,9 +17,12 @@ The merge request is made up of several different key components and ideas that 1. Merge checks 1. Approval rules -When developing merge request widgets, refer to the documentation for the [merge request widget extension framework](widget_extensions.md). +When developing new merge request widgets, read the +[merge request widget extension framework](../new_fe_guide/modules/widget_extensions.md) +documentation. All new widgets should use this framework, and older widgets should +be ported to use it. -### Merge widget +## Merge widget The merge widget is the component of the merge request where the `merge` button exists: @@ -29,7 +30,7 @@ The merge widget is the component of the merge request where the `merge` button This area of the merge request is where all of the options and commit messages are defined prior to merging. It also contains information about what is in the merge request, what issues may be closed, and other important information to the merging process. -### Report widgets +## Report widgets Reports are widgets within the merge request that report information about changes within the merge request. These widgets provide information to better help the author understand the changes and further improvements to the proposed changes. @@ -37,19 +38,19 @@ Reports are widgets within the merge request that report information about chang ![merge request reports](../img/merge_request_reports_v14_7.png) -### Merge checks +## Merge checks Merge checks are statuses that can either pass or fail and conditionally control the availability of the merge button being available within a merge request. The key distinguishing factor in a merge check is that users **do not** interact with the merge checks inside of the merge request, but are able to influence whether or not the check passes or fails. Results from the check are processed as true/false to determine whether or not a merge request can be merged. Examples include: -1. merge conflicts -1. pipeline success -1. threads resolution -1. [external status checks](../../user/project/merge_requests/status_checks.md) -1. required approvals +- Merge conflicts. +- Pipeline success. +- Threads resolution. +- [External status checks](../../user/project/merge_requests/status_checks.md). +- Required approvals. When all of the required merge checks are satisfied a merge request becomes mergeable. -### Approvals +## Approvals Approval rules specify users that are required to or can optionally approve a merge request based on some kind of organizational policy. When approvals are required, they effectively become a required merge check. The key differentiator between merge checks and approval rules is that users **do** interact with approval rules, by deciding to approve the merge request. diff --git a/doc/development/merge_request_concepts/widget_extensions.md b/doc/development/merge_request_concepts/widget_extensions.md index 61fe468823c..8a6a2e0c0c1 100644 --- a/doc/development/merge_request_concepts/widget_extensions.md +++ b/doc/development/merge_request_concepts/widget_extensions.md @@ -1,93 +1,11 @@ --- -stage: create -group: code review -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +redirect_to: '../new_fe_guide/modules/widget_extensions.md' +remove_date: '2022-10-27' --- -# Merge request widget extensions +This document was moved to [another location](../new_fe_guide/modules/widget_extensions.md). -The merge request **Overview** page has [a substantial widgets section](index.md#report-widgets) -that allows for other teams to provide content that enhances the Merge Request review experience -based on features and tools enabled on the instance has enabled. - -To enable contributions from other teams, the Code Review team has created an extension -framework to create standardized widgets for display. - -## Telemetry - -The base implementation of the widget extension framework includes some telemetry events. -Each widget reports: - -- `view`: When it is rendered to the screen. -- `expand`: When it is expanded. -- `full_report_clicked`: When an (optional) input is clicked to view the full report. -- Outcome (`expand_success`, `expand_warning`, or `expand_failed`): One of three - additional events relating to the status of the widget when it was expanded. - -### Adding new widgets - -When adding new widgets, the above events must be marked as `known`, and have metrics -created, to be reportable. To generate these known events for a single widget: - -1. Widgets should be named `Widget${CamelName}`. - - For example: a widget for **Test Reports** should be `WidgetTestReports`. -1. Compute the widget name slug by converting the `${CamelName}` to lower-, snake-case. - - The previous example would be `test_reports`. -1. Add the new widget name slug to `lib/gitlab/usage_data_counters/merge_request_widget_extension_counter.rb` - in the `WIDGETS` list. -1. Ensure the GDK is running (`gdk start`). -1. Generate known events on the command line with the following command. - Replace `test_reports` with your appropriate name slug: - - ```shell - bundle exec rails generate gitlab:usage_metric_definition \ - counts.i_code_review_merge_request_widget_test_reports_count_view \ - counts.i_code_review_merge_request_widget_test_reports_count_full_report_clicked \ - counts.i_code_review_merge_request_widget_test_reports_count_expand \ - counts.i_code_review_merge_request_widget_test_reports_count_expand_success \ - counts.i_code_review_merge_request_widget_test_reports_count_expand_warning \ - counts.i_code_review_merge_request_widget_test_reports_count_expand_failed \ - --dir=all - ``` - -1. Modify each newly generated file to match the existing files for the merge request widget extension telemetry. - - Find existing examples by doing a glob search, like: `metrics/**/*_i_code_review_merge_request_widget_*` - - Roughly speaking, each file should have these values: - 1. `description` = A plain English description of this value. Please see existing widget extension telemetry files for examples. - 1. `product_section` = `dev` - 1. `product_stage` = `create` - 1. `product_group` = `code_review` - 1. `product_category` = `code_review` - 1. `introduced_by_url` = `'[your MR]'` - 1. `options.events` = (the event in the command from above that generated this file, like `i_code_review_merge_request_widget_test_reports_count_view`) - - This value is how the telemetry events are linked to "metrics" so this is probably one of the more important values. - 1. `data_source` = `redis` - 1. `data_category` = `optional` -1. Repeat steps 5 and 6 for the HLL metrics. Replace `test_reports` with your appropriate name slug. - - ```shell - bundle exec rails generate gitlab:usage_metric_definition:redis_hll code_review \ - i_code_review_merge_request_widget_test_reports_view \ - i_code_review_merge_request_widget_test_reports_full_report_clicked \ - i_code_review_merge_request_widget_test_reports_expand \ - i_code_review_merge_request_widget_test_reports_expand_success \ - i_code_review_merge_request_widget_test_reports_expand_warning \ - i_code_review_merge_request_widget_test_reports_expand_failed \ - --class_name=RedisHLLMetric - ``` - - - In step 6 for HLL, change the `data_source` to `redis_hll`. -1. Add each of the HLL metrics to `lib/gitlab/usage_data_counters/known_events/code_review_events.yml`: - 1. `name` = (the event) - 1. `redis_slot` = `code_review` - 1. `category` = `code_review` - 1. `aggregation` = `weekly` -1. Add each event to the appropriate aggregates in `config/metrics/aggregates/code_review.yml` - -#### New events - -If you are adding a new event to our known events: - -1. Include it in - `lib/gitlab/usage_data_counters/merge_request_widget_extension_counter.rb`. -1. Update the list of `KNOWN_EVENTS` with the new events. + + + + diff --git a/doc/development/new_fe_guide/modules/widget_extensions.md b/doc/development/new_fe_guide/modules/widget_extensions.md index 4bae0ac70c4..3b663fac47a 100644 --- a/doc/development/new_fe_guide/modules/widget_extensions.md +++ b/doc/development/new_fe_guide/modules/widget_extensions.md @@ -8,8 +8,6 @@ info: To determine the technical writer assigned to the Stage/Group associated w > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44616) in GitLab 13.6. -## Summary - Extensions in the merge request widget enable you to add new features into the merge request widget that match the design framework. With extensions we get a lot of benefits out of the box without much effort required, like: @@ -37,6 +35,7 @@ export default { expandEvent: '', // Optional: RedisHLL event name to track expanding content enablePolling: false, // Optional: Tells extension to poll for data modalComponent: null, // Optional: The component to use for the modal + telemetry: true, // Optional: Reports basic telemetry for the extension. Set to false to disable telemetry computed: { summary(data) {}, // Required: Level 1 summary text statusIcon(data) {}, // Required: Level 1 status icon @@ -77,7 +76,7 @@ data fetching methods to be used, such as GraphQL or REST API calls. ### API calls For performance reasons, it is best if the collapsed state fetches only the data required to -render the collapsed state. This fetching happens within the `fetchCollapsedData` method. +render the collapsed state. This fetching happens in the `fetchCollapsedData` method. This method is called with the props as an argument, so you can easily access any paths set in the state. @@ -88,7 +87,7 @@ method. When the user clicks **Expand**, the `fetchFullData` method is called. This method also gets called with the props as an argument. This method **must** also return -the full data. However, this data needs to be correctly formatted to match the format +the full data. However, this data must be correctly formatted to match the format mentioned in the data structure section. #### Technical debt @@ -233,7 +232,7 @@ export default { }; ``` -If the extension needs to poll multiple endpoints at the same time, then `fetchMultiData` +If the extension must poll multiple endpoints at the same time, then `fetchMultiData` can be used to return an array of functions. A new `poll` object is created for each endpoint and they are polled separately. After all endpoints are resolved, polling is stopped and `setCollapsedData` is called with an array of `response.data`. @@ -253,7 +252,8 @@ export default { }; ``` -**Important** The function needs to return a `Promise` that resolves the `response` object. +WARNING: +The function must return a `Promise` that resolves the `response` object. The implementation relies on the `POLL-INTERVAL` header to keep polling, therefore it is important not to alter the status code and headers. @@ -280,6 +280,87 @@ export default { }; ``` +## Telemetry + +The base implementation of the widget extension framework includes some telemetry events. +Each widget reports: + +- `view`: When it is rendered to the screen. +- `expand`: When it is expanded. +- `full_report_clicked`: When an (optional) input is clicked to view the full report. +- Outcome (`expand_success`, `expand_warning`, or `expand_failed`): One of three + additional events relating to the status of the widget when it was expanded. + +### Add new widgets + +When adding new widgets, the above events must be marked as `known`, and have metrics +created, to be reportable. + +NOTE: +Events that are only for EE should include `--ee` at the end of both shell commands below. + +To generate these known events for a single widget: + +1. Widgets should be named `Widget${CamelName}`. + - For example: a widget for **Test Reports** should be `WidgetTestReports`. +1. Compute the widget name slug by converting the `${CamelName}` to lower-, snake-case. + - The previous example would be `test_reports`. +1. Add the new widget name slug to `lib/gitlab/usage_data_counters/merge_request_widget_extension_counter.rb` + in the `WIDGETS` list. +1. Ensure the GDK is running (`gdk start`). +1. Generate known events on the command line with the following command. + Replace `test_reports` with your appropriate name slug: + + ```shell + bundle exec rails generate gitlab:usage_metric_definition \ + counts.i_code_review_merge_request_widget_test_reports_count_view \ + counts.i_code_review_merge_request_widget_test_reports_count_full_report_clicked \ + counts.i_code_review_merge_request_widget_test_reports_count_expand \ + counts.i_code_review_merge_request_widget_test_reports_count_expand_success \ + counts.i_code_review_merge_request_widget_test_reports_count_expand_warning \ + counts.i_code_review_merge_request_widget_test_reports_count_expand_failed \ + --dir=all + ``` + +1. Modify each newly generated file to match the existing files for the merge request widget extension telemetry. + - Find existing examples by doing a glob search, like: `metrics/**/*_i_code_review_merge_request_widget_*` + - Roughly speaking, each file should have these values: + 1. `description` = A plain English description of this value. Review existing widget extension telemetry files for examples. + 1. `product_section` = `dev` + 1. `product_stage` = `create` + 1. `product_group` = `code_review` + 1. `product_category` = `code_review` + 1. `introduced_by_url` = `'[your MR]'` + 1. `options.events` = (the event in the command from above that generated this file, like `i_code_review_merge_request_widget_test_reports_count_view`) + - This value is how the telemetry events are linked to "metrics" so this is probably one of the more important values. + 1. `data_source` = `redis` + 1. `data_category` = `optional` +1. Repeat steps 5 and 6 for the HLL metrics. Replace `test_reports` with your appropriate name slug. + + ```shell + bundle exec rails generate gitlab:usage_metric_definition:redis_hll code_review \ + i_code_review_merge_request_widget_test_reports_view \ + i_code_review_merge_request_widget_test_reports_full_report_clicked \ + i_code_review_merge_request_widget_test_reports_expand \ + i_code_review_merge_request_widget_test_reports_expand_success \ + i_code_review_merge_request_widget_test_reports_expand_warning \ + i_code_review_merge_request_widget_test_reports_expand_failed \ + --class_name=RedisHLLMetric + ``` + + - In step 6 for HLL, change the `data_source` to `redis_hll`. +1. Add each of the HLL metrics to `lib/gitlab/usage_data_counters/known_events/code_review_events.yml`: + 1. `name` = (the event) + 1. `redis_slot` = `code_review` + 1. `category` = `code_review` + 1. `aggregation` = `weekly` +1. Add each event to the appropriate aggregates in `config/metrics/aggregates/code_review.yml` + +### Add new events + +If you are adding a new event to our known events, include the new event in the +`KNOWN_EVENTS` list in `lib/gitlab/usage_data_counters/merge_request_widget_extension_counter.rb`. + ## Icons Level 1 and all subsequent levels can have their own status icons. To keep with diff --git a/lib/gitlab/ci/reports/coverage_report_generator.rb b/lib/gitlab/ci/reports/coverage_report_generator.rb index 76992a48b0a..e3f17dc7b14 100644 --- a/lib/gitlab/ci/reports/coverage_report_generator.rb +++ b/lib/gitlab/ci/reports/coverage_report_generator.rb @@ -20,7 +20,7 @@ module Gitlab coverage_report.tap do |coverage_report| report_builds.find_each do |build| - build.each_report(::Ci::JobArtifact::COVERAGE_REPORT_FILE_TYPES) do |file_type, blob| + build.each_report(::Ci::JobArtifact::REPORT_FILE_TYPES[:coverage]) do |file_type, blob| Gitlab::Ci::Parsers.fabricate!(file_type).parse!( blob, coverage_report, diff --git a/lib/gitlab/database/partitioning/sliding_list_strategy.rb b/lib/gitlab/database/partitioning/sliding_list_strategy.rb index 5cf32d3272c..4b5349f0327 100644 --- a/lib/gitlab/database/partitioning/sliding_list_strategy.rb +++ b/lib/gitlab/database/partitioning/sliding_list_strategy.rb @@ -77,7 +77,6 @@ module Gitlab end def validate_and_fix - return unless Feature.enabled?(:fix_sliding_list_partitioning) return if no_partitions_exist? old_default_value = current_default_value diff --git a/lib/gitlab/exclusive_lease_helpers/sleeping_lock.rb b/lib/gitlab/exclusive_lease_helpers/sleeping_lock.rb index 52035220a71..7ef3e738481 100644 --- a/lib/gitlab/exclusive_lease_helpers/sleeping_lock.rb +++ b/lib/gitlab/exclusive_lease_helpers/sleeping_lock.rb @@ -5,6 +5,8 @@ module Gitlab # Wrapper around ExclusiveLease that adds retry logic class SleepingLock delegate :cancel, to: :@lease + MAX_ATTEMPTS = 65 + DEFAULT_ATTEMPTS = 10 def initialize(key, timeout:, delay:) @lease = ::Gitlab::ExclusiveLease.new(key, timeout: timeout) @@ -12,9 +14,9 @@ module Gitlab @attempts = 0 end - def obtain(max_attempts) + def obtain(max_attempts = DEFAULT_ATTEMPTS) until held? - raise FailedToObtainLockError, 'Failed to obtain a lock' if attempts >= max_attempts + raise FailedToObtainLockError, 'Failed to obtain a lock' if attempts >= [max_attempts, MAX_ATTEMPTS].min sleep(sleep_sec) unless first_attempt? try_obtain diff --git a/lib/tasks/gitlab/web_hook.rake b/lib/tasks/gitlab/web_hook.rake index fc17c7d0177..9aa0f07de5f 100644 --- a/lib/tasks/gitlab/web_hook.rake +++ b/lib/tasks/gitlab/web_hook.rake @@ -22,11 +22,13 @@ namespace :gitlab do end end - desc "GitLab | Webhook | Remove a webhook from the projects" - task rm: :environment do + desc "GitLab | Webhook | Remove a webhook from a namespace" + task rm: :environment do |task| web_hook_url = ENV['URL'] namespace_path = ENV['NAMESPACE'] + raise ArgumentError, 'URL is required' unless web_hook_url + web_hooks = find_web_hooks(namespace_path) puts "Removing webhooks with the url '#{web_hook_url}' ... " @@ -36,11 +38,12 @@ namespace :gitlab do # we could consider storing a hash of the URL alongside the encrypted # value to speed up searches count = 0 + service = WebHooks::AdminDestroyService.new(rake_task: task) + web_hooks.find_each do |hook| next unless hook.url == web_hook_url - user = hook.parent.owners.first - result = WebHooks::DestroyService.new(user).execute(hook) + result = service.execute(hook) raise "Unable to destroy Web hook" unless result[:status] == :success diff --git a/spec/lib/gitlab/database/partitioning/sliding_list_strategy_spec.rb b/spec/lib/gitlab/database/partitioning/sliding_list_strategy_spec.rb index d8b06ee1a5d..04b9fba5b2f 100644 --- a/spec/lib/gitlab/database/partitioning/sliding_list_strategy_spec.rb +++ b/spec/lib/gitlab/database/partitioning/sliding_list_strategy_spec.rb @@ -48,61 +48,43 @@ RSpec.describe Gitlab::Database::Partitioning::SlidingListStrategy do end describe '#validate_and_fix' do - context 'feature flag is disabled' do - before do - stub_feature_flags(fix_sliding_list_partitioning: false) - end + it 'does not call change_column_default if the partitioning in a valid state' do + expect(strategy.model.connection).not_to receive(:change_column_default) - it 'does not try to fix the default partition value' do - connection.change_column_default(model.table_name, strategy.partitioning_key, 3) - expect(strategy.model.connection).not_to receive(:change_column_default) - strategy.validate_and_fix - end + strategy.validate_and_fix end - context 'feature flag is enabled' do - before do - stub_feature_flags(fix_sliding_list_partitioning: true) - end + it 'calls change_column_default on partition_key with the most default partition number' do + connection.change_column_default(model.table_name, strategy.partitioning_key, 1) - it 'does not call change_column_default if the partitioning in a valid state' do - expect(strategy.model.connection).not_to receive(:change_column_default) + expect(Gitlab::AppLogger).to receive(:warn).with( + message: 'Fixed default value of sliding_list_strategy partitioning_key', + connection_name: 'main', + old_value: 1, + new_value: 2, + table_name: table_name, + column: strategy.partitioning_key + ) - strategy.validate_and_fix - end + expect(strategy.model.connection).to receive(:change_column_default).with( + model.table_name, strategy.partitioning_key, 2 + ).and_call_original - it 'calls change_column_default on partition_key with the most default partition number' do - connection.change_column_default(model.table_name, strategy.partitioning_key, 1) + strategy.validate_and_fix + end - expect(Gitlab::AppLogger).to receive(:warn).with( - message: 'Fixed default value of sliding_list_strategy partitioning_key', - connection_name: 'main', - old_value: 1, - new_value: 2, - table_name: table_name, - column: strategy.partitioning_key - ) + it 'does not change the default column if it has been changed in the meanwhile by another process' do + expect(strategy).to receive(:current_default_value).and_return(1, 2) - expect(strategy.model.connection).to receive(:change_column_default).with( - model.table_name, strategy.partitioning_key, 2 - ).and_call_original + expect(strategy.model.connection).not_to receive(:change_column_default) - strategy.validate_and_fix - end + expect(Gitlab::AppLogger).to receive(:warn).with( + message: 'Table partitions or partition key default value have been changed by another process', + table_name: table_name, + default_value: 2 + ) - it 'does not change the default column if it has been changed in the meanwhile by another process' do - expect(strategy).to receive(:current_default_value).and_return(1, 2) - - expect(strategy.model.connection).not_to receive(:change_column_default) - - expect(Gitlab::AppLogger).to receive(:warn).with( - message: 'Table partitions or partition key default value have been changed by another process', - table_name: table_name, - default_value: 2 - ) - - strategy.validate_and_fix - end + strategy.validate_and_fix end end diff --git a/spec/lib/gitlab/exclusive_lease_helpers/sleeping_lock_spec.rb b/spec/lib/gitlab/exclusive_lease_helpers/sleeping_lock_spec.rb index f74fbf1206f..1f30ac79488 100644 --- a/spec/lib/gitlab/exclusive_lease_helpers/sleeping_lock_spec.rb +++ b/spec/lib/gitlab/exclusive_lease_helpers/sleeping_lock_spec.rb @@ -52,6 +52,28 @@ RSpec.describe Gitlab::ExclusiveLeaseHelpers::SleepingLock, :clean_gitlab_redis_ end end + context 'when the lease is obtained already' do + let!(:lease) { stub_exclusive_lease_taken(key) } + + context 'when retries are not specified' do + it 'retries to obtain a lease and raises an error' do + expect(lease).to receive(:try_obtain).exactly(10).times + + expect { subject.obtain }.to raise_error('Failed to obtain a lock') + end + end + + context 'when specified retries are above the maximum attempts' do + let(:max_attempts) { 100 } + + it 'retries to obtain a lease and raises an error' do + expect(lease).to receive(:try_obtain).exactly(65).times + + expect { subject.obtain(max_attempts) }.to raise_error('Failed to obtain a lock') + end + end + end + context 'when the lease is held elsewhere' do let!(:lease) { stub_exclusive_lease_taken(key) } let(:max_attempts) { 7 } diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index c1a740b103d..6f1865d8703 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -4552,7 +4552,7 @@ RSpec.describe Ci::Build do end describe '#each_report' do - let(:report_types) { Ci::JobArtifact::COVERAGE_REPORT_FILE_TYPES } + let(:report_types) { Ci::JobArtifact::REPORT_FILE_TYPES[:coverage] } let!(:codequality) { create(:ci_job_artifact, :codequality, job: build) } let!(:coverage) { create(:ci_job_artifact, :coverage_gocov_xml, job: build) } diff --git a/spec/policies/project_hook_policy_spec.rb b/spec/policies/project_hook_policy_spec.rb new file mode 100644 index 00000000000..cfa7b6ee4bf --- /dev/null +++ b/spec/policies/project_hook_policy_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ProjectHookPolicy do + let_it_be(:user) { create(:user) } + + let(:hook) { create(:project_hook) } + + subject(:policy) { described_class.new(user, hook) } + + context 'when the user is not a maintainer' do + before do + hook.project.add_developer(user) + end + + it "cannot read and destroy web-hooks" do + expect(policy).to be_disallowed(:read_web_hook, :destroy_web_hook) + end + end + + context 'when the user is a maintainer' do + before do + hook.project.add_maintainer(user) + end + + it "can read and destroy web-hooks" do + expect(policy).to be_allowed(:read_web_hook, :destroy_web_hook) + end + end +end diff --git a/spec/policies/system_hook_policy_spec.rb b/spec/policies/system_hook_policy_spec.rb new file mode 100644 index 00000000000..37f97a8a3d1 --- /dev/null +++ b/spec/policies/system_hook_policy_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe SystemHookPolicy do + let(:hook) { create(:system_hook) } + + subject(:policy) { described_class.new(user, hook) } + + context 'when the user is not an admin' do + let(:user) { create(:user) } + + %i[read_web_hook destroy_web_hook].each do |thing| + it "cannot #{thing}" do + expect(policy).to be_disallowed(thing) + end + end + end + + context 'when the user is an admin', :enable_admin_mode do + let(:user) { create(:admin) } + + %i[read_web_hook destroy_web_hook].each do |thing| + it "can #{thing}" do + expect(policy).to be_allowed(thing) + end + end + end +end diff --git a/spec/services/web_hooks/destroy_service_spec.rb b/spec/services/web_hooks/destroy_service_spec.rb index 4d9bb18e540..ca8cb8a1b75 100644 --- a/spec/services/web_hooks/destroy_service_spec.rb +++ b/spec/services/web_hooks/destroy_service_spec.rb @@ -8,43 +8,54 @@ RSpec.describe WebHooks::DestroyService do subject { described_class.new(user) } describe '#execute' do - %i[system_hook project_hook].each do |factory| - context "deleting a #{factory}" do - let!(:hook) { create(factory) } # rubocop: disable Rails/SaveBang (false-positive!) - let!(:log) { create_list(:web_hook_log, 3, web_hook: hook) } + # Testing with a project hook only - for permission tests, see policy specs. + let!(:hook) { create(:project_hook) } + let!(:log) { create_list(:web_hook_log, 3, web_hook: hook) } - it 'is successful' do - expect(subject.execute(hook)).to be_success + context 'when the user does not have permission' do + it 'is an error' do + expect(subject.execute(hook)) + .to be_error + .and have_attributes(message: described_class::DENIED) + end + end + + context 'when the user does have permission' do + before do + hook.project.add_maintainer(user) + end + + it 'is successful' do + expect(subject.execute(hook)).to be_success + end + + it 'destroys the hook' do + expect { subject.execute(hook) }.to change(WebHook, :count).from(1).to(0) + end + + it 'does not destroy logs' do + expect { subject.execute(hook) }.not_to change(WebHookLog, :count) + end + + it 'schedules the destruction of logs' do + expect(WebHooks::LogDestroyWorker).to receive(:perform_async).with({ 'hook_id' => hook.id }) + expect(Gitlab::AppLogger).to receive(:info).with(match(/scheduled a deletion of logs/)) + + subject.execute(hook) + end + + context 'when the hook fails to destroy' do + before do + allow(hook).to receive(:destroy).and_return(false) end - it 'destroys the hook' do - expect { subject.execute(hook) }.to change(WebHook, :count).from(1).to(0) - end + it 'is not a success' do + expect(WebHooks::LogDestroyWorker).not_to receive(:perform_async) - it 'does not destroy logs' do - expect { subject.execute(hook) }.not_to change(WebHookLog, :count) - end + r = subject.execute(hook) - it 'schedules the destruction of logs' do - expect(WebHooks::LogDestroyWorker).to receive(:perform_async).with({ 'hook_id' => hook.id }) - expect(Gitlab::AppLogger).to receive(:info).with(match(/scheduled a deletion of logs/)) - - subject.execute(hook) - end - - context 'when the hook fails to destroy' do - before do - allow(hook).to receive(:destroy).and_return(false) - end - - it 'is not a success' do - expect(WebHooks::LogDestroyWorker).not_to receive(:perform_async) - - r = subject.execute(hook) - - expect(r).to be_error - expect(r[:message]).to match %r{Unable to destroy} - end + expect(r).to be_error + expect(r[:message]).to match %r{Unable to destroy} end end end diff --git a/spec/tasks/gitlab/web_hook_rake_spec.rb b/spec/tasks/gitlab/web_hook_rake_spec.rb index 2c582dc78f8..cb6a6e72ab1 100644 --- a/spec/tasks/gitlab/web_hook_rake_spec.rb +++ b/spec/tasks/gitlab/web_hook_rake_spec.rb @@ -50,6 +50,10 @@ RSpec.describe 'gitlab:web_hook namespace rake tasks', :silence_stdout do let(:other_url) { 'http://other.example.com' } + it 'complains if URL is not provided' do + expect { run_rake_task('gitlab:web_hook:rm') }.to raise_error(ArgumentError, 'URL is required') + end + it 'removes a web hook from all projects by URL' do stub_env('URL' => url) run_rake_task('gitlab:web_hook:rm')