Skip to content

[Corrective action] Ban `include ActionView::Helpers::UrlHelper`

Related to #340358 (closed).

Background

This issue is a corrective action for a recent production incident on CI/CD pipelines.

Problem

UrlHelper is meant for use in the view layer. Mixing that into a model, or even a presenter is arguably wrong (we shouldn't mix in a view layer concern into lower levels).

This is also bad because in most cases we probably only want link_to but including UrlHelper adds 40 methods into the included object:

[5] pry(main)> ActionView::Helpers::TagHelper.instance_methods.size + ActionView::Helpers::TagHelper.instance_methods.size
=> 40

Here are the current includes:

app/controllers/projects/mattermosts_controller.rb:  include ActionView::Helpers::AssetUrlHelper
app/helpers/environments_helper.rb:  include ActionView::Helpers::AssetUrlHelper
app/models/integrations/asana.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/bamboo.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/bugzilla.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/campfire.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/confluence.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/custom_issue_tracker.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/datadog.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/discord.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/ewm.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/external_wiki.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/flowdock.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/hangouts_chat.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/irker.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/jenkins.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/jira.rb:    include ActionView::Helpers::AssetUrlHelper
app/models/integrations/mattermost.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/pivotaltracker.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/redmine.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/webex_teams.rb:    include ActionView::Helpers::UrlHelper
app/models/integrations/youtrack.rb:    include ActionView::Helpers::UrlHelper
app/presenters/alert_management/alert_presenter.rb:    include ActionView::Helpers::UrlHelper
app/presenters/ci/pipeline_presenter.rb:    include ActionView::Helpers::UrlHelper
app/presenters/clusters/cluster_presenter.rb:    include ActionView::Helpers::UrlHelper
app/presenters/environment_presenter.rb:  include ActionView::Helpers::UrlHelper
app/presenters/gitlab/blame_presenter.rb:    include ActionView::Helpers::UrlHelper
app/presenters/group_clusterable_presenter.rb:  include ActionView::Helpers::UrlHelper
app/presenters/instance_clusterable_presenter.rb:  include ActionView::Helpers::UrlHelper
app/presenters/merge_request_presenter.rb:  include ActionView::Helpers::UrlHelper
app/presenters/project_clusterable_presenter.rb:  include ActionView::Helpers::UrlHelper
app/presenters/project_presenter.rb:  include ActionView::Helpers::UrlHelper
app/presenters/prometheus_alert_presenter.rb:  include ActionView::Helpers::UrlHelper
app/presenters/release_presenter.rb:  include ActionView::Helpers::UrlHelper
app/presenters/releases/evidence_presenter.rb:    include ActionView::Helpers::UrlHelper
ee/app/helpers/license_helper.rb:  include ActionView::Helpers::UrlHelper
ee/app/models/integrations/github.rb:    include ActionView::Helpers::UrlHelper
ee/app/presenters/merge_request_approver_presenter.rb:  include ActionView::Helpers::UrlHelper
ee/spec/features/markdown/metrics_spec.rb:  include MetricsDashboardUrlHelpers
ee/spec/helpers/ee/integrations_helper_spec.rb:      include ActionView::Helpers::AssetUrlHelper
ee/spec/helpers/ee/projects/security/configuration_helper_spec.rb:  include ActionView::Helpers::UrlHelper
ee/spec/lib/banzai/filter/cross_project_issuable_information_filter_spec.rb:  include ActionView::Helpers::UrlHelper
ee/spec/lib/banzai/filter/issuable_state_filter_spec.rb:  include ActionView::Helpers::UrlHelper
lib/gitlab/ci/badge/metadata.rb:      include ActionView::Helpers::UrlHelper
lib/gitlab/email/message/in_product_marketing/helper.rb:          include ActionView::Helpers::UrlHelper
spec/features/markdown/metrics_spec.rb:  include MetricsDashboardUrlHelpers
spec/helpers/merge_requests_helper_spec.rb:  include ActionView::Helpers::UrlHelper
spec/helpers/nav/top_nav_helper_spec.rb:  include ActionView::Helpers::UrlHelper
spec/helpers/notify_helper_spec.rb:  include ActionView::Helpers::UrlHelper
spec/lib/banzai/filter/issuable_state_filter_spec.rb:  include ActionView::Helpers::UrlHelper
spec/lib/banzai/filter/reference_redactor_filter_spec.rb:  include ActionView::Helpers::UrlHelper
spec/lib/banzai/reference_redactor_spec.rb:    include ActionView::Helpers::UrlHelper

Solution

Can we pass in a view_context to the relevant object so that we can use view_context.link_to ?

/cc @shinya.maeda @hfyngvason @igor.drozdov

Edited by Thong Kuah