From c15d7abadff7382d34457059c233a3d1fcfe1be5 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Wed, 8 Sep 2021 10:55:44 +0700
Subject: [PATCH] Validate accidental override by presenters
This commit validates the accidental override by presenters.
---
app/presenters/README.md | 95 ++++++++++++++++--
.../alert_management/alert_presenter.rb | 4 +
app/presenters/award_emoji_presenter.rb | 2 +-
app/presenters/blob_presenter.rb | 2 +-
app/presenters/blobs/unfold_presenter.rb | 2 +
app/presenters/board_presenter.rb | 2 +-
app/presenters/ci/bridge_presenter.rb | 3 +
app/presenters/ci/build_metadata_presenter.rb | 3 +-
app/presenters/ci/build_presenter.rb | 2 +
app/presenters/ci/group_variable_presenter.rb | 2 +-
app/presenters/ci/legacy_stage_presenter.rb | 2 +-
app/presenters/ci/pipeline_presenter.rb | 7 +-
app/presenters/ci/processable_presenter.rb | 1 +
app/presenters/ci/runner_presenter.rb | 5 +-
app/presenters/ci/stage_presenter.rb | 2 +-
app/presenters/ci/trigger_presenter.rb | 3 +-
app/presenters/ci/variable_presenter.rb | 2 +-
app/presenters/clusterable_presenter.rb | 2 +-
app/presenters/clusters/cluster_presenter.rb | 4 +-
.../clusters/integration_presenter.rb | 2 +-
app/presenters/commit_presenter.rb | 2 +-
app/presenters/commit_status_presenter.rb | 2 +-
.../dev_ops_report/metric_presenter.rb | 2 +
app/presenters/environment_presenter.rb | 2 +-
app/presenters/event_presenter.rb | 3 +-
app/presenters/gitlab/blame_presenter.rb | 2 +-
app/presenters/group_clusterable_presenter.rb | 2 +
app/presenters/group_member_presenter.rb | 2 +
.../instance_clusterable_presenter.rb | 2 +
app/presenters/invitation_presenter.rb | 2 +-
app/presenters/issue_presenter.rb | 3 +-
app/presenters/label_presenter.rb | 4 +-
app/presenters/member_presenter.rb | 2 +-
app/presenters/members_presenter.rb | 2 +-
app/presenters/merge_request_presenter.rb | 6 +-
app/presenters/milestone_presenter.rb | 2 +-
app/presenters/pages_domain_presenter.rb | 4 +-
.../project_clusterable_presenter.rb | 2 +
app/presenters/project_hook_presenter.rb | 2 +-
app/presenters/project_member_presenter.rb | 2 +
app/presenters/project_presenter.rb | 5 +-
.../import_export/project_export_presenter.rb | 10 +-
.../settings/deploy_keys_presenter.rb | 2 +-
app/presenters/prometheus_alert_presenter.rb | 2 +-
app/presenters/release_presenter.rb | 6 +-
app/presenters/releases/evidence_presenter.rb | 2 +-
app/presenters/search_service_presenter.rb | 3 +-
app/presenters/sentry_error_presenter.rb | 2 +-
app/presenters/service_hook_presenter.rb | 2 +-
app/presenters/snippet_blob_presenter.rb | 2 +
app/presenters/snippet_presenter.rb | 5 +-
app/presenters/terraform/modules_presenter.rb | 2 +-
app/presenters/todo_presenter.rb | 2 +-
app/presenters/tree_entry_presenter.rb | 2 +-
app/presenters/user_presenter.rb | 2 +-
app/presenters/web_hook_log_presenter.rb | 2 +-
doc/development/reusing_abstractions.md | 2 +
ee/app/presenters/audit_event_presenter.rb | 2 +-
.../presenters/boards/epic_board_presenter.rb | 2 +-
.../presenters/dast/site_profile_presenter.rb | 2 +-
ee/app/presenters/ee/ci/build_presenter.rb | 2 +
ee/app/presenters/ee/ci/pipeline_presenter.rb | 2 +
ee/app/presenters/ee/label_presenter.rb | 3 +
.../presenters/ee/merge_request_presenter.rb | 6 ++
ee/app/presenters/ee/project_presenter.rb | 2 +
ee/app/presenters/epic_issue_presenter.rb | 3 +-
ee/app/presenters/epic_presenter.rb | 3 +-
ee/app/presenters/iteration_presenter.rb | 2 +-
.../merge_request_approver_presenter.rb | 2 +-
.../security/configuration_presenter.rb | 4 +-
ee/app/presenters/security/scan_presenter.rb | 3 +-
ee/app/presenters/subscription_presenter.rb | 2 +-
.../subscriptions/new_plan_presenter.rb | 3 +
.../vulnerabilities/finding_presenter.rb | 2 +-
ee/app/presenters/vulnerability_presenter.rb | 6 +-
lib/gitlab/utils/delegator_override.rb | 48 +++++++++
lib/gitlab/utils/delegator_override/error.rb | 23 +++++
.../utils/delegator_override/validator.rb | 99 +++++++++++++++++++
lib/gitlab/view/presenter/base.rb | 14 ++-
lib/gitlab/view/presenter/delegated.rb | 11 +++
lib/tasks/lint.rake | 1 +
.../utils/delegator_override/error_spec.rb | 13 +++
.../delegator_override/validator_spec.rb | 81 +++++++++++++++
.../gitlab/utils/delegator_override_spec.rb | 97 ++++++++++++++++++
spec/lib/gitlab/view/presenter/base_spec.rb | 34 ++++++-
85 files changed, 657 insertions(+), 65 deletions(-)
create mode 100644 lib/gitlab/utils/delegator_override.rb
create mode 100644 lib/gitlab/utils/delegator_override/error.rb
create mode 100644 lib/gitlab/utils/delegator_override/validator.rb
create mode 100644 spec/lib/gitlab/utils/delegator_override/error_spec.rb
create mode 100644 spec/lib/gitlab/utils/delegator_override/validator_spec.rb
create mode 100644 spec/lib/gitlab/utils/delegator_override_spec.rb
diff --git a/app/presenters/README.md b/app/presenters/README.md
index 62aec4fc8a20..dfd1818f97dd 100644
--- a/app/presenters/README.md
+++ b/app/presenters/README.md
@@ -66,14 +66,15 @@ we gain the following benefits:
### Presenter definition
-Every presenter should inherit from `Gitlab::View::Presenter::Simple`, which
-provides a `.presents` the method which allows you to define an accessor for the
+If you need a presenter class that has only necessary interfaces for the view-related context,
+inherit from `Gitlab::View::Presenter::Simple`.
+It provides a `.presents` the method which allows you to define an accessor for the
presented object. It also includes common helpers like `Gitlab::Routing` and
`Gitlab::Allowable`.
```ruby
class LabelPresenter < Gitlab::View::Presenter::Simple
- presents :label
+ presents ::Label, as: :label
def text_color
label.color.to_s
@@ -85,13 +86,14 @@ class LabelPresenter < Gitlab::View::Presenter::Simple
end
```
-In some cases, it can be more practical to transparently delegate all missing
-method calls to the presented object, in these cases, you can make your
-presenter inherit from `Gitlab::View::Presenter::Delegated`:
+If you need a presenter class that delegates missing method calls to the presented object,
+inherit from `Gitlab::View::Presenter::Delegated`.
+This is more like an "extension" in the sense that the produced object is going to have
+all of interfaces of the presented object **AND** all of the interfaces in the presenter class:
```ruby
class LabelPresenter < Gitlab::View::Presenter::Delegated
- presents :label
+ presents ::Label, as: :label
def text_color
# color is delegated to label
@@ -152,3 +154,82 @@ You can also present the model in the view:
%div{ class: label.text_color }
= render partial: label, label: label
```
+
+### Validate accidental overrides
+
+We use presenters in many places, such as Controller, Haml, GraphQL/Rest API,
+it's very handy to extend the core/backend logic of Active Record models,
+however, there is a risk that it accidentally overrides important logic.
+
+For example, [this production incident](https://gitlab.com/gitlab-com/gl-infra/production/-/issues/5498)
+was caused by [including `ActionView::Helpers::UrlHelper` in a presenter](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69537/diffs#4b581cff00ef3cc9780efd23682af383de302e7d_3_3).
+The `tag` accesor in `Ci::Build` was accidentally overridden by `ActionView::Helpers::TagHelper#tag`,
+and as a conseuqence, a wrong `tag` value was persited into database.
+
+Starting from GitLab 14.4, we validate the presenters (specifically all of the subclasses of `Gitlab::View::Presenter::Delegated`)
+that they do not accidentally override core/backend logic. In such case, a pipeline in merge requests fails with an error message,
+here is an example:
+
+```plaintext
+We've detected that a presetner is overriding a specific method(s) on a subject model.
+There is a risk that it accidentally modifies the backend/core logic that leads to production incident.
+Please follow https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/presenters/README.md#validate-accidental-overrides
+to resolve this error with caution.
+
+Here are the conflict details.
+
+- Ci::PipelinePresenter#tag is overriding Ci::Pipeline#tag. delegator_location: /devkitkat/services/rails/cache/ruby/2.7.0/gems/actionview-6.1.3.2/lib/action_view/helpers/tag_helper.rb:271 original_location: /devkitkat/services/rails/cache/ruby/2.7.0/gems/activemodel-6.1.3.2/lib/active_model/attribute_methods.rb:254
+```
+
+Here are the potential solutions:
+
+- If the conflict happens on an instance method in the presenter:
+ - If you intend to override the core/backend logic, define `delegator_override <method-name>` on top of the conflicted method.
+ This explicitly adds the method to an allowlist.
+ - If you do NOT intend to override the core/backend logic, rename the method name in the presenter.
+- If the conflict happens on an included module in the presenter, remove the module from the presenter and find a workaround.
+
+### How to use the `Gitlab::Utils::DelegatorOverride` validator
+
+If a presenter class inhertis from `Gitlab::View::Presenter::Delegated`,
+you should define what object class is presented:
+
+```ruby
+class WebHookLogPresenter < Gitlab::View::Presenter::Delegated
+ presents ::WebHookLog, as: :web_hook_log # This defines that the presenter presents `WebHookLog` Active Record model.
+```
+
+These presenters are validated not to accidentaly override the methods in the presented object.
+You can run the validation locally with:
+
+```shell
+bundle exec rake lint:static_verification
+```
+
+To add a method to an allowlist, use `delegator_override`. For example:
+
+```ruby
+class VulnerabilityPresenter < Gitlab::View::Presenter::Delegated
+ presents ::Vulnerability, as: :vulnerability
+
+ delegator_override :description # This adds the `description` method to an allowlist that the override is intentional.
+ def description
+ vulnerability.description || finding.description
+ end
+```
+
+To add methods of a module to an allowlist, use `delegator_override_with`. For example:
+
+```ruby
+module Ci
+ class PipelinePresenter < Gitlab::View::Presenter::Delegated
+ include Gitlab::Utils::StrongMemoize
+ include ActionView::Helpers::UrlHelper
+
+ delegator_override_with Gitlab::Utils::StrongMemoize # TODO: Remove `Gitlab::Utils::StrongMemoize` inclusion as it's duplicate
+ delegator_override_with ActionView::Helpers::TagHelper # TODO: Remove `ActionView::Helpers::UrlHelper` inclusion as it overrides `Ci::Pipeline#tag`
+```
+
+Keep in mind that if you use `delegator_override_with`,
+there is a high chance that you're doing **something wrong**.
+Read the [Validate Accidental Overrides](#validate-accidental-overrides) for more information.
diff --git a/app/presenters/alert_management/alert_presenter.rb b/app/presenters/alert_management/alert_presenter.rb
index 93983c989c89..86fe98592716 100644
--- a/app/presenters/alert_management/alert_presenter.rb
+++ b/app/presenters/alert_management/alert_presenter.rb
@@ -5,6 +5,9 @@ class AlertPresenter < Gitlab::View::Presenter::Delegated
include IncidentManagement::Settings
include ActionView::Helpers::UrlHelper
+ presents ::AlertManagement::Alert
+ delegator_override_with Gitlab::Utils::StrongMemoize # TODO: Remove `Gitlab::Utils::StrongMemoize` inclusion as it's duplicate
+
MARKDOWN_LINE_BREAK = " \n"
HORIZONTAL_LINE = "\n\n---\n\n"
@@ -29,6 +32,7 @@ def start_time
started_at&.strftime('%d %B %Y, %-l:%M%p (%Z)')
end
+ delegator_override :details_url
def details_url
details_project_alert_management_url(project, alert.iid)
end
diff --git a/app/presenters/award_emoji_presenter.rb b/app/presenters/award_emoji_presenter.rb
index 98713855d35d..8a7b58e0aba2 100644
--- a/app/presenters/award_emoji_presenter.rb
+++ b/app/presenters/award_emoji_presenter.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class AwardEmojiPresenter < Gitlab::View::Presenter::Delegated
- presents :award_emoji
+ presents ::AwardEmoji, as: :award_emoji
def description
as_emoji['description']
diff --git a/app/presenters/blob_presenter.rb b/app/presenters/blob_presenter.rb
index ecc16e2840c3..c198859aa4c8 100644
--- a/app/presenters/blob_presenter.rb
+++ b/app/presenters/blob_presenter.rb
@@ -7,7 +7,7 @@ class BlobPresenter < Gitlab::View::Presenter::Delegated
include TreeHelper
include ChecksCollaboration
- presents :blob
+ presents ::Blob, as: :blob
def highlight(to: nil, plain: nil)
load_all_blob_data
diff --git a/app/presenters/blobs/unfold_presenter.rb b/app/presenters/blobs/unfold_presenter.rb
index 487c6fe07577..b921b5bf670a 100644
--- a/app/presenters/blobs/unfold_presenter.rb
+++ b/app/presenters/blobs/unfold_presenter.rb
@@ -6,6 +6,8 @@ class UnfoldPresenter < BlobPresenter
include ActiveModel::AttributeAssignment
include Gitlab::Utils::StrongMemoize
+ presents ::Blob
+
attribute :full, :boolean, default: false
attribute :since, :integer, default: 1
attribute :to, :integer, default: 1
diff --git a/app/presenters/board_presenter.rb b/app/presenters/board_presenter.rb
index d7cecd44dd7f..bb3e96b8fafc 100644
--- a/app/presenters/board_presenter.rb
+++ b/app/presenters/board_presenter.rb
@@ -1,5 +1,5 @@
# frozen_string_literal: true
class BoardPresenter < Gitlab::View::Presenter::Delegated
- presents :board
+ presents ::Board, as: :board
end
diff --git a/app/presenters/ci/bridge_presenter.rb b/app/presenters/ci/bridge_presenter.rb
index 724e10c26c33..a62d7cdbbd43 100644
--- a/app/presenters/ci/bridge_presenter.rb
+++ b/app/presenters/ci/bridge_presenter.rb
@@ -2,6 +2,9 @@
module Ci
class BridgePresenter < ProcessablePresenter
+ presents ::Ci::Bridge
+
+ delegator_override :detailed_status
def detailed_status
@detailed_status ||= subject.detailed_status(user)
end
diff --git a/app/presenters/ci/build_metadata_presenter.rb b/app/presenters/ci/build_metadata_presenter.rb
index 4871bb3a9199..2f559adf77dc 100644
--- a/app/presenters/ci/build_metadata_presenter.rb
+++ b/app/presenters/ci/build_metadata_presenter.rb
@@ -9,8 +9,9 @@ class BuildMetadataPresenter < Gitlab::View::Presenter::Delegated
job_timeout_source: 'job'
}.freeze
- presents :metadata
+ presents ::Ci::BuildMetadata, as: :metadata
+ delegator_override :timeout_source
def timeout_source
return unless metadata.timeout_source?
diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 06ed6791bb74..65e1c80085f5 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -2,6 +2,8 @@
module Ci
class BuildPresenter < ProcessablePresenter
+ presents ::Ci::Build
+
def erased_by_user?
# Build can be erased through API, therefore it does not have
# `erased_by` user assigned in that case.
diff --git a/app/presenters/ci/group_variable_presenter.rb b/app/presenters/ci/group_variable_presenter.rb
index 99011150c84e..dea9a42b6224 100644
--- a/app/presenters/ci/group_variable_presenter.rb
+++ b/app/presenters/ci/group_variable_presenter.rb
@@ -2,7 +2,7 @@
module Ci
class GroupVariablePresenter < Gitlab::View::Presenter::Delegated
- presents :variable
+ presents ::Ci::GroupVariable, as: :variable
def placeholder
'GROUP_VARIABLE'
diff --git a/app/presenters/ci/legacy_stage_presenter.rb b/app/presenters/ci/legacy_stage_presenter.rb
index d5c21baba287..c803abfab6a1 100644
--- a/app/presenters/ci/legacy_stage_presenter.rb
+++ b/app/presenters/ci/legacy_stage_presenter.rb
@@ -2,7 +2,7 @@
module Ci
class LegacyStagePresenter < Gitlab::View::Presenter::Delegated
- presents :legacy_stage
+ presents ::Ci::LegacyStage, as: :legacy_stage
def latest_ordered_statuses
preload_statuses(legacy_stage.statuses.latest_ordered)
diff --git a/app/presenters/ci/pipeline_presenter.rb b/app/presenters/ci/pipeline_presenter.rb
index 82f00f746924..e0cb899c9d39 100644
--- a/app/presenters/ci/pipeline_presenter.rb
+++ b/app/presenters/ci/pipeline_presenter.rb
@@ -5,6 +5,9 @@ class PipelinePresenter < Gitlab::View::Presenter::Delegated
include Gitlab::Utils::StrongMemoize
include ActionView::Helpers::UrlHelper
+ delegator_override_with Gitlab::Utils::StrongMemoize # TODO: Remove `Gitlab::Utils::StrongMemoize` inclusion as it's duplicate
+ delegator_override_with ActionView::Helpers::TagHelper # TODO: Remove `ActionView::Helpers::UrlHelper` inclusion as it overrides `Ci::Pipeline#tag`
+
# We use a class method here instead of a constant, allowing EE to redefine
# the returned `Hash` more easily.
def self.failure_reasons
@@ -20,8 +23,9 @@ def self.failure_reasons
user_blocked: 'The user who created this pipeline is blocked.' }
end
- presents :pipeline
+ presents ::Ci::Pipeline, as: :pipeline
+ delegator_override :failed_builds
def failed_builds
return [] unless can?(current_user, :read_build, pipeline)
@@ -30,6 +34,7 @@ def failed_builds
end
end
+ delegator_override :failure_reason
def failure_reason
return unless pipeline.failure_reason?
diff --git a/app/presenters/ci/processable_presenter.rb b/app/presenters/ci/processable_presenter.rb
index 5a8a66490711..9f3a83a00f3e 100644
--- a/app/presenters/ci/processable_presenter.rb
+++ b/app/presenters/ci/processable_presenter.rb
@@ -2,5 +2,6 @@
module Ci
class ProcessablePresenter < CommitStatusPresenter
+ presents ::Ci::Processable
end
end
diff --git a/app/presenters/ci/runner_presenter.rb b/app/presenters/ci/runner_presenter.rb
index 273328afc53f..ad889d444f86 100644
--- a/app/presenters/ci/runner_presenter.rb
+++ b/app/presenters/ci/runner_presenter.rb
@@ -2,11 +2,14 @@
module Ci
class RunnerPresenter < Gitlab::View::Presenter::Delegated
- presents :runner
+ presents ::Ci::Runner, as: :runner
+ delegator_override :locked?
def locked?
read_attribute(:locked) && project_type?
end
+
+ delegator_override :locked
alias_method :locked, :locked?
end
end
diff --git a/app/presenters/ci/stage_presenter.rb b/app/presenters/ci/stage_presenter.rb
index 21bda86cdeda..bd5bf08abbca 100644
--- a/app/presenters/ci/stage_presenter.rb
+++ b/app/presenters/ci/stage_presenter.rb
@@ -2,7 +2,7 @@
module Ci
class StagePresenter < Gitlab::View::Presenter::Delegated
- presents :stage
+ presents ::Ci::Stage, as: :stage
PRELOADED_RELATIONS = [:pipeline, :metadata, :tags, :job_artifacts_archive, :downstream_pipeline].freeze
diff --git a/app/presenters/ci/trigger_presenter.rb b/app/presenters/ci/trigger_presenter.rb
index 605c8f328a47..5f0bd4b3a8bb 100644
--- a/app/presenters/ci/trigger_presenter.rb
+++ b/app/presenters/ci/trigger_presenter.rb
@@ -2,12 +2,13 @@
module Ci
class TriggerPresenter < Gitlab::View::Presenter::Delegated
- presents :trigger
+ presents ::Ci::Trigger, as: :trigger
def has_token_exposed?
can?(current_user, :admin_trigger, trigger)
end
+ delegator_override :token
def token
if has_token_exposed?
trigger.token
diff --git a/app/presenters/ci/variable_presenter.rb b/app/presenters/ci/variable_presenter.rb
index f027f3aa5600..ec04dd5e9ff0 100644
--- a/app/presenters/ci/variable_presenter.rb
+++ b/app/presenters/ci/variable_presenter.rb
@@ -2,7 +2,7 @@
module Ci
class VariablePresenter < Gitlab::View::Presenter::Delegated
- presents :variable
+ presents ::Ci::Variable, as: :variable
def placeholder
'PROJECT_VARIABLE'
diff --git a/app/presenters/clusterable_presenter.rb b/app/presenters/clusterable_presenter.rb
index 03e26b929229..4b645510b515 100644
--- a/app/presenters/clusterable_presenter.rb
+++ b/app/presenters/clusterable_presenter.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class ClusterablePresenter < Gitlab::View::Presenter::Delegated
- presents :clusterable
+ presents ::Project, ::Group, ::Clusters::Instance, as: :clusterable
def self.fabricate(clusterable, **attributes)
presenter_class = "#{clusterable.class.name}ClusterablePresenter".constantize
diff --git a/app/presenters/clusters/cluster_presenter.rb b/app/presenters/clusters/cluster_presenter.rb
index eb4bd8532afe..892ea14267bc 100644
--- a/app/presenters/clusters/cluster_presenter.rb
+++ b/app/presenters/clusters/cluster_presenter.rb
@@ -7,7 +7,9 @@ class ClusterPresenter < Gitlab::View::Presenter::Delegated
include ActionView::Helpers::UrlHelper
include IconsHelper
- presents :cluster
+ delegator_override_with ::Gitlab::Utils::StrongMemoize # TODO: Remove `::Gitlab::Utils::StrongMemoize` inclusion as it's duplicate
+
+ presents ::Clusters::Cluster, as: :cluster
# We do not want to show the group path for clusters belonging to the
# clusterable, only for the ancestor clusters.
diff --git a/app/presenters/clusters/integration_presenter.rb b/app/presenters/clusters/integration_presenter.rb
index 57608be29b15..f7be59f00f3c 100644
--- a/app/presenters/clusters/integration_presenter.rb
+++ b/app/presenters/clusters/integration_presenter.rb
@@ -2,7 +2,7 @@
module Clusters
class IntegrationPresenter < Gitlab::View::Presenter::Delegated
- presents :integration
+ presents ::Clusters::Integrations::Prometheus, ::Clusters::Integrations::ElasticStack, as: :integration
def application_type
integration.class.name.demodulize.underscore
diff --git a/app/presenters/commit_presenter.rb b/app/presenters/commit_presenter.rb
index c14dcab60007..7df45ca03bba 100644
--- a/app/presenters/commit_presenter.rb
+++ b/app/presenters/commit_presenter.rb
@@ -3,7 +3,7 @@
class CommitPresenter < Gitlab::View::Presenter::Delegated
include GlobalID::Identification
- presents :commit
+ presents ::Commit, as: :commit
def status_for(ref)
return unless can?(current_user, :read_commit_status, commit.project)
diff --git a/app/presenters/commit_status_presenter.rb b/app/presenters/commit_status_presenter.rb
index 3c39470b730c..2d524fd611ec 100644
--- a/app/presenters/commit_status_presenter.rb
+++ b/app/presenters/commit_status_presenter.rb
@@ -33,7 +33,7 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
private_constant :CALLOUT_FAILURE_MESSAGES
- presents :build
+ presents ::CommitStatus, as: :build
def self.callout_failure_messages
CALLOUT_FAILURE_MESSAGES
diff --git a/app/presenters/dev_ops_report/metric_presenter.rb b/app/presenters/dev_ops_report/metric_presenter.rb
index 4d7ac1cd3ec9..55326f8f678f 100644
--- a/app/presenters/dev_ops_report/metric_presenter.rb
+++ b/app/presenters/dev_ops_report/metric_presenter.rb
@@ -2,6 +2,8 @@
module DevOpsReport
class MetricPresenter < Gitlab::View::Presenter::Simple
+ presents ::DevOpsReport::Metric
+
delegate :created_at, to: :subject
def cards
diff --git a/app/presenters/environment_presenter.rb b/app/presenters/environment_presenter.rb
index 6f67bbe2a5a9..6c8da86187c0 100644
--- a/app/presenters/environment_presenter.rb
+++ b/app/presenters/environment_presenter.rb
@@ -3,7 +3,7 @@
class EnvironmentPresenter < Gitlab::View::Presenter::Delegated
include ActionView::Helpers::UrlHelper
- presents :environment
+ presents ::Environment, as: :environment
def path
project_environment_path(project, self)
diff --git a/app/presenters/event_presenter.rb b/app/presenters/event_presenter.rb
index c37721f7213d..4c787b88e20f 100644
--- a/app/presenters/event_presenter.rb
+++ b/app/presenters/event_presenter.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class EventPresenter < Gitlab::View::Presenter::Delegated
- presents :event
+ presents ::Event, as: :event
def initialize(subject, **attributes)
super
@@ -10,6 +10,7 @@ def initialize(subject, **attributes)
end
# Caching `visible_to_user?` method in the presenter beause it might be called multiple times.
+ delegator_override :visible_to_user?
def visible_to_user?(user = nil)
@visible_to_user_cache.fetch(user&.id) { super(user) }
end
diff --git a/app/presenters/gitlab/blame_presenter.rb b/app/presenters/gitlab/blame_presenter.rb
index 1f2445b04a1f..e9340a42e51f 100644
--- a/app/presenters/gitlab/blame_presenter.rb
+++ b/app/presenters/gitlab/blame_presenter.rb
@@ -12,7 +12,7 @@ class BlamePresenter < Gitlab::View::Presenter::Simple
include TreeHelper
include IconsHelper
- presents :blame
+ presents nil, as: :blame
CommitData = Struct.new(
:author_avatar,
diff --git a/app/presenters/group_clusterable_presenter.rb b/app/presenters/group_clusterable_presenter.rb
index 34e7084ab029..207767af245e 100644
--- a/app/presenters/group_clusterable_presenter.rb
+++ b/app/presenters/group_clusterable_presenter.rb
@@ -4,6 +4,8 @@ class GroupClusterablePresenter < ClusterablePresenter
extend ::Gitlab::Utils::Override
include ActionView::Helpers::UrlHelper
+ presents ::Group
+
override :cluster_status_cluster_path
def cluster_status_cluster_path(cluster, params = {})
cluster_status_group_cluster_path(clusterable, cluster, params)
diff --git a/app/presenters/group_member_presenter.rb b/app/presenters/group_member_presenter.rb
index 5ab4b51f472b..88facc3608de 100644
--- a/app/presenters/group_member_presenter.rb
+++ b/app/presenters/group_member_presenter.rb
@@ -1,6 +1,8 @@
# frozen_string_literal: true
class GroupMemberPresenter < MemberPresenter
+ presents ::GroupMember
+
private
def admin_member_permission
diff --git a/app/presenters/instance_clusterable_presenter.rb b/app/presenters/instance_clusterable_presenter.rb
index 56d91f90b2ea..77ca171156d6 100644
--- a/app/presenters/instance_clusterable_presenter.rb
+++ b/app/presenters/instance_clusterable_presenter.rb
@@ -4,6 +4,8 @@ class InstanceClusterablePresenter < ClusterablePresenter
extend ::Gitlab::Utils::Override
include ActionView::Helpers::UrlHelper
+ presents ::Clusters::Instance
+
def self.fabricate(clusterable, **attributes)
attributes_with_presenter_class = attributes.merge(presenter_class: InstanceClusterablePresenter)
diff --git a/app/presenters/invitation_presenter.rb b/app/presenters/invitation_presenter.rb
index d8c07f327dd6..ada8227a4771 100644
--- a/app/presenters/invitation_presenter.rb
+++ b/app/presenters/invitation_presenter.rb
@@ -1,5 +1,5 @@
# frozen_string_literal: true
class InvitationPresenter < Gitlab::View::Presenter::Delegated
- presents :invitation
+ presents nil, as: :invitation
end
diff --git a/app/presenters/issue_presenter.rb b/app/presenters/issue_presenter.rb
index b7f4ac0555d4..72fe14d224de 100644
--- a/app/presenters/issue_presenter.rb
+++ b/app/presenters/issue_presenter.rb
@@ -1,12 +1,13 @@
# frozen_string_literal: true
class IssuePresenter < Gitlab::View::Presenter::Delegated
- presents :issue
+ presents ::Issue, as: :issue
def issue_path
url_builder.build(issue, only_path: true)
end
+ delegator_override :subscribed?
def subscribed?
issue.subscribed?(current_user, issue.project)
end
diff --git a/app/presenters/label_presenter.rb b/app/presenters/label_presenter.rb
index 9e51e6fa4bae..fafade2828f0 100644
--- a/app/presenters/label_presenter.rb
+++ b/app/presenters/label_presenter.rb
@@ -1,9 +1,11 @@
# frozen_string_literal: true
class LabelPresenter < Gitlab::View::Presenter::Delegated
- presents :label
+ presents ::Label, as: :label
delegate :name, :full_name, to: :label_subject, prefix: :subject
+ delegator_override :subject # TODO: Fix `Gitlab::View::Presenter::Delegated#subject` not to override `Label#subject`.
+
def edit_path
case label
when GroupLabel then edit_group_label_path(label.group, label)
diff --git a/app/presenters/member_presenter.rb b/app/presenters/member_presenter.rb
index b37a43bf251c..67d044dd01c3 100644
--- a/app/presenters/member_presenter.rb
+++ b/app/presenters/member_presenter.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class MemberPresenter < Gitlab::View::Presenter::Delegated
- presents :member
+ presents ::Member, as: :member
def access_level_roles
member.class.access_level_roles
diff --git a/app/presenters/members_presenter.rb b/app/presenters/members_presenter.rb
index 03ebea36d49b..b572cf962358 100644
--- a/app/presenters/members_presenter.rb
+++ b/app/presenters/members_presenter.rb
@@ -3,7 +3,7 @@
class MembersPresenter < Gitlab::View::Presenter::Delegated
include Enumerable
- presents :members
+ presents nil, as: :members
def to_ary
to_a
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index fc8a290f5f78..d19d4964524e 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -8,9 +8,11 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
include ChecksCollaboration
include Gitlab::Utils::StrongMemoize
+ delegator_override_with Gitlab::Utils::StrongMemoize # TODO: Remove `Gitlab::Utils::StrongMemoize` inclusion as it's duplicate
+
APPROVALS_WIDGET_BASE_TYPE = 'base'
- presents :merge_request
+ presents ::MergeRequest, as: :merge_request
def ci_status
if pipeline
@@ -183,6 +185,7 @@ def can_push_to_source_branch?
.can_push_to_branch?(source_branch)
end
+ delegator_override :can_remove_source_branch?
def can_remove_source_branch?
source_branch_exists? && merge_request.can_remove_source_branch?(current_user)
end
@@ -202,6 +205,7 @@ def mergeable_discussions_state
end
end
+ delegator_override :subscribed?
def subscribed?
merge_request.subscribed?(current_user, merge_request.target_project)
end
diff --git a/app/presenters/milestone_presenter.rb b/app/presenters/milestone_presenter.rb
index 6bf8203702fe..4084c8740f0e 100644
--- a/app/presenters/milestone_presenter.rb
+++ b/app/presenters/milestone_presenter.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class MilestonePresenter < Gitlab::View::Presenter::Delegated
- presents :milestone
+ presents ::Milestone, as: :milestone
def milestone_path
url_builder.build(milestone, only_path: true)
diff --git a/app/presenters/pages_domain_presenter.rb b/app/presenters/pages_domain_presenter.rb
index 6ef89760bec1..0523f7024161 100644
--- a/app/presenters/pages_domain_presenter.rb
+++ b/app/presenters/pages_domain_presenter.rb
@@ -1,7 +1,9 @@
# frozen_string_literal: true
class PagesDomainPresenter < Gitlab::View::Presenter::Delegated
- presents :pages_domain
+ presents ::PagesDomain, as: :pages_domain
+
+ delegator_override :subject # TODO: Fix `Gitlab::View::Presenter::Delegated#subject` not to override `PagesDomain#subject`.
def needs_verification?
Gitlab::CurrentSettings.pages_domain_verification_enabled? && unverified?
diff --git a/app/presenters/project_clusterable_presenter.rb b/app/presenters/project_clusterable_presenter.rb
index 920304e743ed..05344f2da965 100644
--- a/app/presenters/project_clusterable_presenter.rb
+++ b/app/presenters/project_clusterable_presenter.rb
@@ -4,6 +4,8 @@ class ProjectClusterablePresenter < ClusterablePresenter
extend ::Gitlab::Utils::Override
include ActionView::Helpers::UrlHelper
+ presents ::Project
+
override :cluster_status_cluster_path
def cluster_status_cluster_path(cluster, params = {})
cluster_status_project_cluster_path(clusterable, cluster, params)
diff --git a/app/presenters/project_hook_presenter.rb b/app/presenters/project_hook_presenter.rb
index a65c7221b5a4..a696e9fd0ec8 100644
--- a/app/presenters/project_hook_presenter.rb
+++ b/app/presenters/project_hook_presenter.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class ProjectHookPresenter < Gitlab::View::Presenter::Delegated
- presents :project_hook
+ presents ::ProjectHook, as: :project_hook
def logs_details_path(log)
project_hook_hook_log_path(project, self, log)
diff --git a/app/presenters/project_member_presenter.rb b/app/presenters/project_member_presenter.rb
index 17947266ed79..91d3ae96877c 100644
--- a/app/presenters/project_member_presenter.rb
+++ b/app/presenters/project_member_presenter.rb
@@ -1,6 +1,8 @@
# frozen_string_literal: true
class ProjectMemberPresenter < MemberPresenter
+ presents ::ProjectMember
+
private
def admin_member_permission
diff --git a/app/presenters/project_presenter.rb b/app/presenters/project_presenter.rb
index 066f4786cff5..bbd8c715f5cb 100644
--- a/app/presenters/project_presenter.rb
+++ b/app/presenters/project_presenter.rb
@@ -12,7 +12,10 @@ class ProjectPresenter < Gitlab::View::Presenter::Delegated
include Gitlab::Utils::StrongMemoize
include Gitlab::Experiment::Dsl
- presents :project
+ delegator_override_with GitlabRoutingHelper # TODO: Remove `GitlabRoutingHelper` inclusion as it's duplicate
+ delegator_override_with Gitlab::Utils::StrongMemoize # TODO: Remove `Gitlab::Utils::StrongMemoize` inclusion as it's duplicate
+
+ presents ::Project, as: :project
AnchorData = Struct.new(:is_link, :label, :link, :class_modifier, :icon, :itemprop, :data)
MAX_TOPICS_TO_SHOW = 3
diff --git a/app/presenters/projects/import_export/project_export_presenter.rb b/app/presenters/projects/import_export/project_export_presenter.rb
index f56760b55df4..7b2ffb6d7557 100644
--- a/app/presenters/projects/import_export/project_export_presenter.rb
+++ b/app/presenters/projects/import_export/project_export_presenter.rb
@@ -5,16 +5,24 @@ module ImportExport
class ProjectExportPresenter < Gitlab::View::Presenter::Delegated
include ActiveModel::Serializers::JSON
- presents :project
+ presents ::Project, as: :project
+ # TODO: Remove `ActiveModel::Serializers::JSON` inclusion as it's duplicate
+ delegator_override_with ActiveModel::Serializers::JSON
+ delegator_override_with ActiveModel::Naming
+ delegator_override :include_root_in_json, :include_root_in_json?
+
+ delegator_override :project_members
def project_members
super + converted_group_members
end
+ delegator_override :description
def description
self.respond_to?(:override_description) ? override_description : super
end
+ delegator_override :protected_branches
def protected_branches
project.exported_protected_branches
end
diff --git a/app/presenters/projects/settings/deploy_keys_presenter.rb b/app/presenters/projects/settings/deploy_keys_presenter.rb
index 13290a8e632f..e3323b751881 100644
--- a/app/presenters/projects/settings/deploy_keys_presenter.rb
+++ b/app/presenters/projects/settings/deploy_keys_presenter.rb
@@ -5,7 +5,7 @@ module Settings
class DeployKeysPresenter < Gitlab::View::Presenter::Simple
include Gitlab::Utils::StrongMemoize
- presents :project
+ presents ::Project, as: :project
delegate :size, to: :enabled_keys, prefix: true
delegate :size, to: :available_project_keys, prefix: true
delegate :size, to: :available_public_keys, prefix: true
diff --git a/app/presenters/prometheus_alert_presenter.rb b/app/presenters/prometheus_alert_presenter.rb
index 99e24bdcdb9c..714329ede71e 100644
--- a/app/presenters/prometheus_alert_presenter.rb
+++ b/app/presenters/prometheus_alert_presenter.rb
@@ -3,7 +3,7 @@
class PrometheusAlertPresenter < Gitlab::View::Presenter::Delegated
include ActionView::Helpers::UrlHelper
- presents :prometheus_alert
+ presents ::PrometheusAlert, as: :prometheus_alert
def humanized_text
operator_text =
diff --git a/app/presenters/release_presenter.rb b/app/presenters/release_presenter.rb
index ac27e997b41c..c919c7f4c60d 100644
--- a/app/presenters/release_presenter.rb
+++ b/app/presenters/release_presenter.rb
@@ -3,8 +3,10 @@
class ReleasePresenter < Gitlab::View::Presenter::Delegated
include ActionView::Helpers::UrlHelper
- presents :release
+ presents ::Release, as: :release
+ # TODO: Remove `delegate` as it's redundant due to SimpleDelegator.
+ delegator_override :tag, :project
delegate :project, :tag, to: :release
def commit_path
@@ -51,6 +53,7 @@ def edit_url
edit_project_release_url(project, release)
end
+ delegator_override :assets_count
def assets_count
if can_download_code?
release.assets_count
@@ -59,6 +62,7 @@ def assets_count
end
end
+ delegator_override :name
def name
can_download_code? ? release.name : "Release-#{release.id}"
end
diff --git a/app/presenters/releases/evidence_presenter.rb b/app/presenters/releases/evidence_presenter.rb
index a00cbacb7d83..bdc053a303be 100644
--- a/app/presenters/releases/evidence_presenter.rb
+++ b/app/presenters/releases/evidence_presenter.rb
@@ -4,7 +4,7 @@ module Releases
class EvidencePresenter < Gitlab::View::Presenter::Delegated
include ActionView::Helpers::UrlHelper
- presents :evidence
+ presents ::Releases::Evidence, as: :evidence
def filepath
release = evidence.release
diff --git a/app/presenters/search_service_presenter.rb b/app/presenters/search_service_presenter.rb
index ab43800b9f23..72f967b8beb1 100644
--- a/app/presenters/search_service_presenter.rb
+++ b/app/presenters/search_service_presenter.rb
@@ -3,7 +3,7 @@
class SearchServicePresenter < Gitlab::View::Presenter::Delegated
include RendersCommits
- presents :search_service
+ presents ::SearchService, as: :search_service
SCOPE_PRELOAD_METHOD = {
projects: :with_web_entity_associations,
@@ -18,6 +18,7 @@ class SearchServicePresenter < Gitlab::View::Presenter::Delegated
SORT_ENABLED_SCOPES = %w(issues merge_requests epics).freeze
+ delegator_override :search_objects
def search_objects
@search_objects ||= begin
objects = search_service.search_objects(SCOPE_PRELOAD_METHOD[scope.to_sym])
diff --git a/app/presenters/sentry_error_presenter.rb b/app/presenters/sentry_error_presenter.rb
index 669bcb68b7cf..5862e54dfc7c 100644
--- a/app/presenters/sentry_error_presenter.rb
+++ b/app/presenters/sentry_error_presenter.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class SentryErrorPresenter < Gitlab::View::Presenter::Delegated
- presents :error
+ presents nil, as: :error
FrequencyStruct = Struct.new(:time, :count, keyword_init: true)
diff --git a/app/presenters/service_hook_presenter.rb b/app/presenters/service_hook_presenter.rb
index 8f2ba1a905f1..91911eb3dff2 100644
--- a/app/presenters/service_hook_presenter.rb
+++ b/app/presenters/service_hook_presenter.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class ServiceHookPresenter < Gitlab::View::Presenter::Delegated
- presents :service_hook
+ presents ::ServiceHook, as: :service_hook
def logs_details_path(log)
project_service_hook_log_path(integration.project, integration, log)
diff --git a/app/presenters/snippet_blob_presenter.rb b/app/presenters/snippet_blob_presenter.rb
index ab8fc0f905b7..4072696eb890 100644
--- a/app/presenters/snippet_blob_presenter.rb
+++ b/app/presenters/snippet_blob_presenter.rb
@@ -3,6 +3,8 @@
class SnippetBlobPresenter < BlobPresenter
include GitlabRoutingHelper
+ presents ::SnippetBlob
+
def rich_data
return unless blob.rich_viewer
diff --git a/app/presenters/snippet_presenter.rb b/app/presenters/snippet_presenter.rb
index 695aa266e2ca..8badbe7f54ac 100644
--- a/app/presenters/snippet_presenter.rb
+++ b/app/presenters/snippet_presenter.rb
@@ -1,16 +1,18 @@
# frozen_string_literal: true
class SnippetPresenter < Gitlab::View::Presenter::Delegated
- presents :snippet
+ presents ::Snippet, as: :snippet
def raw_url
url_builder.build(snippet, raw: true)
end
+ delegator_override :ssh_url_to_repo
def ssh_url_to_repo
snippet.ssh_url_to_repo if snippet.repository_exists?
end
+ delegator_override :http_url_to_repo
def http_url_to_repo
snippet.http_url_to_repo if snippet.repository_exists?
end
@@ -31,6 +33,7 @@ def can_report_as_spam?
snippet.submittable_as_spam_by?(current_user)
end
+ delegator_override :blob
def blob
return snippet.blob if snippet.empty_repo?
diff --git a/app/presenters/terraform/modules_presenter.rb b/app/presenters/terraform/modules_presenter.rb
index 608f69e20197..9e9c6a5cd2b6 100644
--- a/app/presenters/terraform/modules_presenter.rb
+++ b/app/presenters/terraform/modules_presenter.rb
@@ -4,7 +4,7 @@ module Terraform
class ModulesPresenter < Gitlab::View::Presenter::Simple
attr_accessor :packages, :system
- presents :modules
+ presents nil, as: :modules
def initialize(packages, system)
@packages = packages
diff --git a/app/presenters/todo_presenter.rb b/app/presenters/todo_presenter.rb
index 291be7848e25..cb8d37be22d8 100644
--- a/app/presenters/todo_presenter.rb
+++ b/app/presenters/todo_presenter.rb
@@ -1,5 +1,5 @@
# frozen_string_literal: true
class TodoPresenter < Gitlab::View::Presenter::Delegated
- presents :todo
+ presents ::Todo, as: :todo
end
diff --git a/app/presenters/tree_entry_presenter.rb b/app/presenters/tree_entry_presenter.rb
index 216b3b0d4c91..0b313d813601 100644
--- a/app/presenters/tree_entry_presenter.rb
+++ b/app/presenters/tree_entry_presenter.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class TreeEntryPresenter < Gitlab::View::Presenter::Delegated
- presents :tree
+ presents nil, as: :tree
def web_url
Gitlab::Routing.url_helpers.project_tree_url(tree.repository.project, File.join(tree.commit_id, tree.path))
diff --git a/app/presenters/user_presenter.rb b/app/presenters/user_presenter.rb
index 7cd94082bacb..5a99f10b6e74 100644
--- a/app/presenters/user_presenter.rb
+++ b/app/presenters/user_presenter.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class UserPresenter < Gitlab::View::Presenter::Delegated
- presents :user
+ presents ::User, as: :user
def group_memberships
should_be_private? ? GroupMember.none : user.group_members
diff --git a/app/presenters/web_hook_log_presenter.rb b/app/presenters/web_hook_log_presenter.rb
index fca03ddb5d76..a51665890737 100644
--- a/app/presenters/web_hook_log_presenter.rb
+++ b/app/presenters/web_hook_log_presenter.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class WebHookLogPresenter < Gitlab::View::Presenter::Delegated
- presents :web_hook_log
+ presents ::WebHookLog, as: :web_hook_log
def details_path
web_hook.present.logs_details_path(self)
diff --git a/doc/development/reusing_abstractions.md b/doc/development/reusing_abstractions.md
index ded6b0743248..568e8a9d1235 100644
--- a/doc/development/reusing_abstractions.md
+++ b/doc/development/reusing_abstractions.md
@@ -183,6 +183,8 @@ queries they produce.
Everything in `app/presenters`, used for exposing complex data to a Rails view,
without having to create many instance variables.
+See [the documentation](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/presenters/README.md) for more information.
+
### Serializers
Everything in `app/serializers`, used for presenting the response to a request,
diff --git a/ee/app/presenters/audit_event_presenter.rb b/ee/app/presenters/audit_event_presenter.rb
index 7c42cbb3a38c..48170b89c1e7 100644
--- a/ee/app/presenters/audit_event_presenter.rb
+++ b/ee/app/presenters/audit_event_presenter.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class AuditEventPresenter < Gitlab::View::Presenter::Simple
- presents :audit_event
+ presents ::AuditEvent, as: :audit_event
def author_name
audit_event.author_name
diff --git a/ee/app/presenters/boards/epic_board_presenter.rb b/ee/app/presenters/boards/epic_board_presenter.rb
index 4b791a1e8930..a788a6709934 100644
--- a/ee/app/presenters/boards/epic_board_presenter.rb
+++ b/ee/app/presenters/boards/epic_board_presenter.rb
@@ -2,6 +2,6 @@
module Boards
class EpicBoardPresenter < Gitlab::View::Presenter::Delegated
- presents :epic_board
+ presents ::Boards::EpicBoard, as: :epic_board
end
end
diff --git a/ee/app/presenters/dast/site_profile_presenter.rb b/ee/app/presenters/dast/site_profile_presenter.rb
index 7fe17745aaeb..ca1f40873a9c 100644
--- a/ee/app/presenters/dast/site_profile_presenter.rb
+++ b/ee/app/presenters/dast/site_profile_presenter.rb
@@ -5,7 +5,7 @@ class SiteProfilePresenter < Gitlab::View::Presenter::Delegated
REDACTED_PASSWORD = '••••••••'
REDACTED_REQUEST_HEADERS = '••••••••'
- presents :site_profile
+ presents ::DastSiteProfile, as: :site_profile
def password
return unless site_profile.secret_variables.any? { |variable| variable.key == ::Dast::SiteProfileSecretVariable::PASSWORD }
diff --git a/ee/app/presenters/ee/ci/build_presenter.rb b/ee/app/presenters/ee/ci/build_presenter.rb
index 679224e7a397..1c739f66e599 100644
--- a/ee/app/presenters/ee/ci/build_presenter.rb
+++ b/ee/app/presenters/ee/ci/build_presenter.rb
@@ -4,7 +4,9 @@ module EE
module Ci
module BuildPresenter
extend ActiveSupport::Concern
+ extend ::Gitlab::Utils::DelegatorOverride
+ delegator_override :retryable?
def retryable?
!merge_train_pipeline? && super
end
diff --git a/ee/app/presenters/ee/ci/pipeline_presenter.rb b/ee/app/presenters/ee/ci/pipeline_presenter.rb
index d31cb811239c..e1b23cdc446e 100644
--- a/ee/app/presenters/ee/ci/pipeline_presenter.rb
+++ b/ee/app/presenters/ee/ci/pipeline_presenter.rb
@@ -4,6 +4,7 @@ module EE
module Ci
module PipelinePresenter
extend ActiveSupport::Concern
+ extend ::Gitlab::Utils::DelegatorOverride
def expose_security_dashboard?
return false unless can?(current_user, :read_security_resource, pipeline.project)
@@ -18,6 +19,7 @@ def degradation_threshold(file_type)
end
end
+ delegator_override :retryable?
def retryable?
!merge_train_pipeline? && super
end
diff --git a/ee/app/presenters/ee/label_presenter.rb b/ee/app/presenters/ee/label_presenter.rb
index fd3a4572077f..30180ca8e69e 100644
--- a/ee/app/presenters/ee/label_presenter.rb
+++ b/ee/app/presenters/ee/label_presenter.rb
@@ -2,6 +2,9 @@
module EE
module LabelPresenter
+ extend ::Gitlab::Utils::DelegatorOverride
+
+ delegator_override :scoped_label?
def scoped_label?
label.scoped_label? && context_subject && context_subject.feature_available?(:scoped_labels)
end
diff --git a/ee/app/presenters/ee/merge_request_presenter.rb b/ee/app/presenters/ee/merge_request_presenter.rb
index 3a9dad8fb5d2..2a15d8241943 100644
--- a/ee/app/presenters/ee/merge_request_presenter.rb
+++ b/ee/app/presenters/ee/merge_request_presenter.rb
@@ -4,6 +4,9 @@ module EE
module MergeRequestPresenter
include ::VisibleApprovable
extend ::Gitlab::Utils::Override
+ extend ::Gitlab::Utils::DelegatorOverride
+
+ delegator_override_with ::VisibleApprovable # TODO: Remove `::VisibleApprovable` inclusion as it's duplicate
APPROVALS_WIDGET_FULL_TYPE = 'full'
@@ -33,6 +36,7 @@ def merge_immediately_docs_path
help_page_path('ci/pipelines/merge_trains.md', anchor: 'immediately-merge-a-merge-request-with-a-merge-train')
end
+ delegator_override :target_project
def target_project
merge_request.target_project.present(current_user: current_user)
end
@@ -41,6 +45,7 @@ def code_owner_rules_with_users
@code_owner_rules ||= merge_request.approval_rules.code_owner.with_users.to_a
end
+ delegator_override :approver_groups
def approver_groups
::ApproverGroup.filtered_approver_groups(merge_request.approver_groups, current_user)
end
@@ -54,6 +59,7 @@ def approvals_widget_type
expose_mr_approval_path? ? APPROVALS_WIDGET_FULL_TYPE : super
end
+ delegator_override :missing_security_scan_types
def missing_security_scan_types
merge_request.missing_security_scan_types if expose_missing_security_scan_types?
end
diff --git a/ee/app/presenters/ee/project_presenter.rb b/ee/app/presenters/ee/project_presenter.rb
index d82df27e156f..2cea06454d75 100644
--- a/ee/app/presenters/ee/project_presenter.rb
+++ b/ee/app/presenters/ee/project_presenter.rb
@@ -3,6 +3,7 @@
module EE
module ProjectPresenter
extend ::Gitlab::Utils::Override
+ extend ::Gitlab::Utils::DelegatorOverride
override :statistics_buttons
def statistics_buttons(show_auto_devops_callout:)
@@ -13,6 +14,7 @@ def extra_statistics_buttons
[sast_anchor_data.presence].compact
end
+ delegator_override :approver_groups
def approver_groups
::ApproverGroup.filtered_approver_groups(project.approver_groups, current_user)
end
diff --git a/ee/app/presenters/epic_issue_presenter.rb b/ee/app/presenters/epic_issue_presenter.rb
index f54904196a56..a0338354556a 100644
--- a/ee/app/presenters/epic_issue_presenter.rb
+++ b/ee/app/presenters/epic_issue_presenter.rb
@@ -1,7 +1,8 @@
# frozen_string_literal: true
class EpicIssuePresenter < Gitlab::View::Presenter::Delegated
- presents :issue
+ delegator_override :issue
+ presents ::EpicIssue, as: :issue
def group_epic_issue_path(current_user)
return unless can_admin_issue_link?(current_user)
diff --git a/ee/app/presenters/epic_presenter.rb b/ee/app/presenters/epic_presenter.rb
index 16e53e49134b..ae6bed4ebbb7 100644
--- a/ee/app/presenters/epic_presenter.rb
+++ b/ee/app/presenters/epic_presenter.rb
@@ -4,7 +4,7 @@ class EpicPresenter < Gitlab::View::Presenter::Delegated
include GitlabRoutingHelper
include EntityDateHelper
- presents :epic
+ presents ::Epic, as: :epic
def show_data(base_data: {}, author_icon: nil)
{
@@ -35,6 +35,7 @@ def epic_reference(full: false)
end
end
+ delegator_override :subscribed?
def subscribed?
epic.subscribed?(current_user)
end
diff --git a/ee/app/presenters/iteration_presenter.rb b/ee/app/presenters/iteration_presenter.rb
index 4c2763b3345f..63871c7cc432 100644
--- a/ee/app/presenters/iteration_presenter.rb
+++ b/ee/app/presenters/iteration_presenter.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class IterationPresenter < Gitlab::View::Presenter::Delegated
- presents :iteration
+ presents ::Iteration, as: :iteration
def iteration_path
url_builder.build(iteration, only_path: true)
diff --git a/ee/app/presenters/merge_request_approver_presenter.rb b/ee/app/presenters/merge_request_approver_presenter.rb
index 6c0e5423d1be..1efa1a71864a 100644
--- a/ee/app/presenters/merge_request_approver_presenter.rb
+++ b/ee/app/presenters/merge_request_approver_presenter.rb
@@ -12,7 +12,7 @@ class MergeRequestApproverPresenter < Gitlab::View::Presenter::Simple
include ActionView::RecordIdentifier
include Gitlab::Utils::StrongMemoize
- presents :merge_request
+ presents ::MergeRequest, as: :merge_request
attr_reader :skip_user
diff --git a/ee/app/presenters/projects/security/configuration_presenter.rb b/ee/app/presenters/projects/security/configuration_presenter.rb
index d32e3a706f6d..2ecb20bcf05b 100644
--- a/ee/app/presenters/projects/security/configuration_presenter.rb
+++ b/ee/app/presenters/projects/security/configuration_presenter.rb
@@ -7,7 +7,9 @@ class ConfigurationPresenter < Gitlab::View::Presenter::Delegated
include AutoDevopsHelper
include LatestPipelineInformation
- presents :project
+ delegator_override_with Gitlab::Utils::StrongMemoize # TODO: Remove `Gitlab::Utils::StrongMemoize` inclusion as it's duplicate
+
+ presents ::Project, as: :project
def to_h
{
diff --git a/ee/app/presenters/security/scan_presenter.rb b/ee/app/presenters/security/scan_presenter.rb
index 9f59738d1446..36871023b980 100644
--- a/ee/app/presenters/security/scan_presenter.rb
+++ b/ee/app/presenters/security/scan_presenter.rb
@@ -4,8 +4,9 @@ module Security
class ScanPresenter < Gitlab::View::Presenter::Delegated
ERROR_MESSAGE_FORMAT = '[%<type>s] %<message>s'
- presents :scan
+ presents ::Security::Scan, as: :scan
+ delegator_override :errors
def errors
info['errors'].to_a.map { |error| format(ERROR_MESSAGE_FORMAT, error.symbolize_keys) }
end
diff --git a/ee/app/presenters/subscription_presenter.rb b/ee/app/presenters/subscription_presenter.rb
index b726876e4aa7..aa42356c72db 100644
--- a/ee/app/presenters/subscription_presenter.rb
+++ b/ee/app/presenters/subscription_presenter.rb
@@ -3,7 +3,7 @@
class SubscriptionPresenter < Gitlab::View::Presenter::Delegated
GRACE_PERIOD_EXTENSION_DAYS = 14.days
- presents :subscription
+ presents ::Subscription, as: :subscription
def block_changes?
will_block_changes? && (block_changes_at < Date.today)
diff --git a/ee/app/presenters/subscriptions/new_plan_presenter.rb b/ee/app/presenters/subscriptions/new_plan_presenter.rb
index 90502c66c63e..858c22dec848 100644
--- a/ee/app/presenters/subscriptions/new_plan_presenter.rb
+++ b/ee/app/presenters/subscriptions/new_plan_presenter.rb
@@ -2,11 +2,14 @@
module Subscriptions
class NewPlanPresenter < Gitlab::View::Presenter::Delegated
+ presents ::Plan
+
NEW_PLAN_TITLES = {
silver: 'Premium (Formerly Silver)',
gold: 'Ultimate (Formerly Gold)'
}.freeze
+ delegator_override :title
def title
NEW_PLAN_TITLES.fetch(plan_key, super)
end
diff --git a/ee/app/presenters/vulnerabilities/finding_presenter.rb b/ee/app/presenters/vulnerabilities/finding_presenter.rb
index 9235f696b167..c3e094413aa4 100644
--- a/ee/app/presenters/vulnerabilities/finding_presenter.rb
+++ b/ee/app/presenters/vulnerabilities/finding_presenter.rb
@@ -2,7 +2,7 @@
module Vulnerabilities
class FindingPresenter < Gitlab::View::Presenter::Delegated
- presents :finding
+ presents ::Vulnerabilities::Finding, as: :finding
def title
name
diff --git a/ee/app/presenters/vulnerability_presenter.rb b/ee/app/presenters/vulnerability_presenter.rb
index 295462cc4faa..eadf6a7228f3 100644
--- a/ee/app/presenters/vulnerability_presenter.rb
+++ b/ee/app/presenters/vulnerability_presenter.rb
@@ -1,12 +1,14 @@
# frozen_string_literal: true
class VulnerabilityPresenter < Gitlab::View::Presenter::Delegated
- presents :vulnerability
+ presents ::Vulnerability, as: :vulnerability
+ delegator_override :links
def links
vulnerability.links.map(&:with_indifferent_access)
end
+ delegator_override :remediations
def remediations
vulnerability.remediations.to_a.compact.map(&:with_indifferent_access)
end
@@ -31,6 +33,7 @@ def raw_path
path_with_line_number(project_raw_path(vulnerability.project, File.join(pipeline_branch, file)))
end
+ delegator_override :blob_path
def blob_path
return unless file
@@ -52,6 +55,7 @@ def jira_issue_description
)
end
+ delegator_override :description
def description
vulnerability.description || finding.description
end
diff --git a/lib/gitlab/utils/delegator_override.rb b/lib/gitlab/utils/delegator_override.rb
new file mode 100644
index 000000000000..15ba29d39160
--- /dev/null
+++ b/lib/gitlab/utils/delegator_override.rb
@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Utils
+ # This module is to validate that delegator classes (`SimpleDelegator`) do not
+ # accidentally override important logic on the fabricated object.
+ module DelegatorOverride
+ def delegator_target(target_class)
+ return unless ENV['STATIC_VERIFICATION']
+
+ unless self < ::SimpleDelegator
+ raise ArgumentError, "'#{self}' is not a subclass of 'SimpleDelegator' class."
+ end
+
+ DelegatorOverride.validator(self).add_target(target_class)
+ end
+
+ def delegator_override(*names)
+ return unless ENV['STATIC_VERIFICATION']
+ raise TypeError unless names.all? { |n| n.is_a?(Symbol) }
+
+ DelegatorOverride.validator(self).add_allowlist(names)
+ end
+
+ def delegator_override_with(mod)
+ return unless ENV['STATIC_VERIFICATION']
+ raise TypeError unless mod.is_a?(Module)
+
+ DelegatorOverride.validator(self).add_allowlist(mod.instance_methods)
+ end
+
+ def self.validator(delegator_class)
+ validators[delegator_class] ||= Validator.new(delegator_class)
+ end
+
+ def self.validators
+ @validators ||= {}
+ end
+
+ def self.verify!
+ validators.each_value do |validator|
+ validator.expand_on_ancestors(validators)
+ validator.validate_overrides!
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/utils/delegator_override/error.rb b/lib/gitlab/utils/delegator_override/error.rb
new file mode 100644
index 000000000000..dfe8d5468b47
--- /dev/null
+++ b/lib/gitlab/utils/delegator_override/error.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Utils
+ module DelegatorOverride
+ class Error
+ attr_accessor :method_name, :target_class, :target_location, :delegator_class, :delegator_location
+
+ def initialize(method_name, target_class, target_location, delegator_class, delegator_location)
+ @method_name = method_name
+ @target_class = target_class
+ @target_location = target_location
+ @delegator_class = delegator_class
+ @delegator_location = delegator_location
+ end
+
+ def to_s
+ "#{delegator_class}##{method_name} is overriding #{target_class}##{method_name}. delegator_location: #{delegator_location} target_location: #{target_location}"
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/utils/delegator_override/validator.rb b/lib/gitlab/utils/delegator_override/validator.rb
new file mode 100644
index 000000000000..825b3efa2034
--- /dev/null
+++ b/lib/gitlab/utils/delegator_override/validator.rb
@@ -0,0 +1,99 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Utils
+ module DelegatorOverride
+ class Validator
+ UnexpectedDelegatorOverrideError = Class.new(StandardError)
+
+ attr_reader :delegator_class, :target_classes
+
+ OVERRIDE_ERROR_MESSAGE = <<~EOS
+ We've detected that the delegator is overriding a specific method(s) on the target class.
+ Please make sure if it's intentional and handle this error accordingly.
+ See https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/presenters/README.md#validate-accidental-overrides for more information.
+ EOS
+
+ def initialize(delegator_class)
+ @delegator_class = delegator_class
+ @target_classes = []
+ end
+
+ def add_allowlist(names)
+ allowed_method_names.concat(names)
+ end
+
+ def allowed_method_names
+ @allowed_method_names ||= []
+ end
+
+ def add_target(target_class)
+ @target_classes << target_class if target_class
+ end
+
+ # This will make sure allowlist we put into ancestors are all included
+ def expand_on_ancestors(validators)
+ delegator_class.ancestors.each do |ancestor|
+ next if delegator_class == ancestor # ancestor includes itself
+
+ validator_ancestor = validators[ancestor]
+
+ next unless validator_ancestor
+
+ add_allowlist(validator_ancestor.allowed_method_names)
+ end
+ end
+
+ def validate_overrides!
+ return if target_classes.empty?
+
+ errors = []
+
+ # Workaround to fully load the instance methods in the target class.
+ # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69823#note_678887402
+ target_classes.map(&:new) rescue nil
+
+ (delegator_class.instance_methods - allowlist).each do |method_name|
+ target_classes.each do |target_class|
+ next unless target_class.instance_methods.include?(method_name)
+
+ errors << generate_error(method_name, target_class, delegator_class)
+ end
+ end
+
+ return if errors.empty?
+
+ details = errors.map { |error| "- #{error}" }.join("\n")
+
+ raise UnexpectedDelegatorOverrideError,
+ <<~TEXT
+ #{OVERRIDE_ERROR_MESSAGE}
+ Here are the conflict details.
+
+ #{details}
+ TEXT
+ end
+
+ private
+
+ def generate_error(method_name, target_class, delegator_class)
+ target_location = extract_location(target_class, method_name)
+ delegator_location = extract_location(delegator_class, method_name)
+ Error.new(method_name, target_class, target_location, delegator_class, delegator_location)
+ end
+
+ def extract_location(klass, method_name)
+ klass.instance_method(method_name).source_location&.join(':') || 'unknown'
+ end
+
+ def allowlist
+ [].tap do |allowed|
+ allowed.concat(allowed_method_names)
+ allowed.concat(Object.instance_methods)
+ allowed.concat(::Delegator.instance_methods)
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/view/presenter/base.rb b/lib/gitlab/view/presenter/base.rb
index 9dc687f7740f..3bacad720500 100644
--- a/lib/gitlab/view/presenter/base.rb
+++ b/lib/gitlab/view/presenter/base.rb
@@ -47,8 +47,18 @@ def presenter?
true
end
- def presents(name)
- define_method(name) { subject }
+ def presents(*target_classes, as: nil)
+ if target_classes.any? { |k| k.is_a?(Symbol) }
+ raise ArgumentError, "Unsupported target class type: #{target_classes}."
+ end
+
+ if self < ::Gitlab::View::Presenter::Delegated
+ target_classes.each { |k| delegator_target(k) }
+ elsif self < ::Gitlab::View::Presenter::Simple
+ # no-op
+ end
+
+ define_method(as) { subject } if as
end
end
end
diff --git a/lib/gitlab/view/presenter/delegated.rb b/lib/gitlab/view/presenter/delegated.rb
index d14f8cc4e5ed..259cf0cf457f 100644
--- a/lib/gitlab/view/presenter/delegated.rb
+++ b/lib/gitlab/view/presenter/delegated.rb
@@ -4,7 +4,18 @@ module Gitlab
module View
module Presenter
class Delegated < SimpleDelegator
+ extend ::Gitlab::Utils::DelegatorOverride
+
+ # TODO: Stop including auxiliary methods/modules in `Presenter::Base` as
+ # it overrides many methods in the Active Record models.
+ # See https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/presenters/README.md#validate-accidental-overrides
+ # for more information.
include Gitlab::View::Presenter::Base
+ delegator_override_with Gitlab::Routing.url_helpers
+ delegator_override :can?
+ delegator_override :declarative_policy_delegate
+ delegator_override :present
+ delegator_override :web_url
def initialize(subject, **attributes)
@subject = subject
diff --git a/lib/tasks/lint.rake b/lib/tasks/lint.rake
index 976ec0890115..7d1721f1df8d 100644
--- a/lib/tasks/lint.rake
+++ b/lib/tasks/lint.rake
@@ -12,6 +12,7 @@ unless Rails.env.production?
dev:load
] do
Gitlab::Utils::Override.verify!
+ Gitlab::Utils::DelegatorOverride.verify!
end
desc "GitLab | Lint | Lint JavaScript files using ESLint"
diff --git a/spec/lib/gitlab/utils/delegator_override/error_spec.rb b/spec/lib/gitlab/utils/delegator_override/error_spec.rb
new file mode 100644
index 000000000000..59b67676eff4
--- /dev/null
+++ b/spec/lib/gitlab/utils/delegator_override/error_spec.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+RSpec.describe Gitlab::Utils::DelegatorOverride::Error do
+ let(:error) { described_class.new('foo', 'Target', '/path/to/target', 'Delegator', '/path/to/delegator') }
+
+ describe '#to_s' do
+ subject { error.to_s }
+
+ it { is_expected.to eq("Delegator#foo is overriding Target#foo. delegator_location: /path/to/delegator target_location: /path/to/target") }
+ end
+end
diff --git a/spec/lib/gitlab/utils/delegator_override/validator_spec.rb b/spec/lib/gitlab/utils/delegator_override/validator_spec.rb
new file mode 100644
index 000000000000..4cd1b18de82b
--- /dev/null
+++ b/spec/lib/gitlab/utils/delegator_override/validator_spec.rb
@@ -0,0 +1,81 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+RSpec.describe Gitlab::Utils::DelegatorOverride::Validator do
+ let(:delegator_class) do
+ Class.new(::SimpleDelegator) do
+ extend(::Gitlab::Utils::DelegatorOverride)
+
+ def foo
+ end
+ end.prepend(ee_delegator_extension)
+ end
+
+ let(:ee_delegator_extension) do
+ Module.new do
+ extend(::Gitlab::Utils::DelegatorOverride)
+
+ def bar
+ end
+ end
+ end
+
+ let(:target_class) do
+ Class.new do
+ def foo
+ end
+
+ def bar
+ end
+ end
+ end
+
+ let(:validator) { described_class.new(delegator_class) }
+
+ describe '#add_allowlist' do
+ it 'adds a method name to the allowlist' do
+ validator.add_allowlist([:foo])
+
+ expect(validator.allowed_method_names).to contain_exactly(:foo)
+ end
+ end
+
+ describe '#add_target' do
+ it 'adds the target class' do
+ validator.add_target(target_class)
+
+ expect(validator.target_classes).to contain_exactly(target_class)
+ end
+ end
+
+ describe '#expand_on_ancestors' do
+ it 'adds the allowlist in the ancestors' do
+ ancestor_validator = described_class.new(ee_delegator_extension)
+ ancestor_validator.add_allowlist([:bar])
+ validator.expand_on_ancestors( { ee_delegator_extension => ancestor_validator })
+
+ expect(validator.allowed_method_names).to contain_exactly(:bar)
+ end
+ end
+
+ describe '#validate_overrides!' do
+ before do
+ validator.add_target(target_class)
+ end
+
+ it 'does not raise an error when the overrides are allowed' do
+ validator.add_allowlist([:foo])
+ ancestor_validator = described_class.new(ee_delegator_extension)
+ ancestor_validator.add_allowlist([:bar])
+ validator.expand_on_ancestors( { ee_delegator_extension => ancestor_validator })
+
+ expect { validator.validate_overrides! }.not_to raise_error
+ end
+
+ it 'raises an error when there is an override' do
+ expect { validator.validate_overrides! }
+ .to raise_error(described_class::UnexpectedDelegatorOverrideError)
+ end
+ end
+end
diff --git a/spec/lib/gitlab/utils/delegator_override_spec.rb b/spec/lib/gitlab/utils/delegator_override_spec.rb
new file mode 100644
index 000000000000..af4c7fa5d8ec
--- /dev/null
+++ b/spec/lib/gitlab/utils/delegator_override_spec.rb
@@ -0,0 +1,97 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+RSpec.describe Gitlab::Utils::DelegatorOverride do
+ let(:delegator_class) do
+ Class.new(::SimpleDelegator) do
+ extend(::Gitlab::Utils::DelegatorOverride)
+
+ def foo
+ end
+ end
+ end
+
+ let(:target_class) do
+ Class.new do
+ def foo
+ end
+
+ def bar
+ end
+ end
+ end
+
+ let(:dummy_module) do
+ Module.new do
+ def foobar
+ end
+ end
+ end
+
+ before do
+ stub_env('STATIC_VERIFICATION', 'true')
+ end
+
+ describe '.delegator_target' do
+ subject { delegator_class.delegator_target(target_class) }
+
+ it 'sets the delegator target to the validator' do
+ expect(described_class.validator(delegator_class))
+ .to receive(:add_target).with(target_class)
+
+ subject
+ end
+
+ context 'when the class does not inherit SimpleDelegator' do
+ let(:delegator_class) do
+ Class.new do
+ extend(::Gitlab::Utils::DelegatorOverride)
+ end
+ end
+
+ it 'raises an error' do
+ expect { subject }.to raise_error(ArgumentError, /not a subclass of 'SimpleDelegator' class/)
+ end
+ end
+ end
+
+ describe '.delegator_override' do
+ subject { delegator_class.delegator_override(:foo) }
+
+ it 'adds the method name to the allowlist' do
+ expect(described_class.validator(delegator_class))
+ .to receive(:add_allowlist).with([:foo])
+
+ subject
+ end
+ end
+
+ describe '.delegator_override_with' do
+ subject { delegator_class.delegator_override_with(dummy_module) }
+
+ it 'adds the method names of the module to the allowlist' do
+ expect(described_class.validator(delegator_class))
+ .to receive(:add_allowlist).with([:foobar])
+
+ subject
+ end
+ end
+
+ describe '.verify!' do
+ subject { described_class.verify! }
+
+ it 'does not raise an error when an override is in allowlist' do
+ delegator_class.delegator_target(target_class)
+ delegator_class.delegator_override(:foo)
+
+ expect { subject }.not_to raise_error
+ end
+
+ it 'raises an error when there is an override' do
+ delegator_class.delegator_target(target_class)
+
+ expect { subject }.to raise_error(Gitlab::Utils::DelegatorOverride::Validator::UnexpectedDelegatorOverrideError)
+ end
+ end
+end
diff --git a/spec/lib/gitlab/view/presenter/base_spec.rb b/spec/lib/gitlab/view/presenter/base_spec.rb
index 97d5e2b280de..a7083bd27223 100644
--- a/spec/lib/gitlab/view/presenter/base_spec.rb
+++ b/spec/lib/gitlab/view/presenter/base_spec.rb
@@ -18,11 +18,43 @@
describe '.presents' do
it 'exposes #subject with the given keyword' do
- presenter_class.presents(:foo)
+ presenter_class.presents(Object, as: :foo)
presenter = presenter_class.new(project)
expect(presenter.foo).to eq(project)
end
+
+ it 'raises an error when symbol is passed' do
+ expect { presenter_class.presents(:foo) }.to raise_error(ArgumentError)
+ end
+
+ context 'when the presenter class inherits Presenter::Delegated' do
+ let(:presenter_class) do
+ Class.new(::Gitlab::View::Presenter::Delegated) do
+ include(::Gitlab::View::Presenter::Base)
+ end
+ end
+
+ it 'sets the delegator target' do
+ expect(presenter_class).to receive(:delegator_target).with(Object)
+
+ presenter_class.presents(Object, as: :foo)
+ end
+ end
+
+ context 'when the presenter class inherits Presenter::Simple' do
+ let(:presenter_class) do
+ Class.new(::Gitlab::View::Presenter::Simple) do
+ include(::Gitlab::View::Presenter::Base)
+ end
+ end
+
+ it 'does not set the delegator target' do
+ expect(presenter_class).not_to receive(:delegator_target).with(Object)
+
+ presenter_class.presents(Object, as: :foo)
+ end
+ end
end
describe '#can?' do
--
GitLab