Commit 4feeb5f3 authored by Yorick Peterse's avatar Yorick Peterse

Refactor how a few ActiveRecord enums are defined

In a few models we define ActiveRecord enums that are redefined in EE
using the following pattern:

    enum :some_enum, {
      ...
    }.merge(EE_ENUM_VALUES)

This particular approach is problematic to deal with, because it
requires that we `prepend` and EE module _before_ defining the enum.
This typically translates to the `prepend` being the first line in the
model in EE, but this can easily lead to merge conflicts when developers
add more `include` and/or `prepend` lines.

As part of https://gitlab.com/gitlab-org/gitlab-ee/issues/8244 and
https://gitlab.com/gitlab-org/gitlab-ee/issues/8241 we are moving
`prepend` to the last line in a file, reducing the chances of running
into merge conflicts. This poses a bit of a problem with the pattern
above, because this pattern does not allow us to move the `prepend`
further down a file.

To resolve this problem, we simply move the Hash value of the enum to a
separate class method. This method is defined in a separate module where
necessary, allowing us to use it like so:

    enum :failure_reasons, ::SomeModelEnums.failure_reasons

The method in turn is defined in a very straightforward manner:

    module SomeModelEnums
      def self.failure_reasons
        {
          ...
        }
      end
    end

This makes it easy for EE to add values without requiring the `prepend`
to be placed before the `enum` is defined.

For more information, see the following issues and merge requests:

* https://gitlab.com/gitlab-org/gitlab-ee/issues/8244
* https://gitlab.com/gitlab-org/gitlab-ee/issues/8241
* https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/8424
parent 0dfac552
......@@ -69,17 +69,9 @@ module Ci
after_create :keep_around_commits, unless: :importing?
enum_with_nil source: {
unknown: nil,
push: 1,
web: 2,
trigger: 3,
schedule: 4,
api: 5,
external: 6,
pipeline: 7,
chat: 8
}
# We use `Ci::PipelineEnums.sources` here so that EE can more easily extend
# this `Hash` with new values.
enum_with_nil source: ::Ci::PipelineEnums.sources
enum_with_nil config_source: {
unknown_source: nil,
......@@ -87,10 +79,9 @@ module Ci
auto_devops_source: 2
}
enum failure_reason: {
unknown_failure: 0,
config_error: 1
}.merge(EE_FAILURE_REASONS)
# We use `Ci::PipelineEnums.failure_reasons` here so that EE can more easily
# extend this `Hash` with new values.
enum failure_reason: ::Ci::PipelineEnums.failure_reasons
state_machine :status, initial: :created do
event :enqueue do
......
# frozen_string_literal: true
module Ci
module PipelineEnums
# Returns the `Hash` to use for creating the `failure_reason` enum for
# `Ci::Pipeline`.
def self.failure_reasons
{
unknown_failure: 0,
config_error: 1
}
end
# Returns the `Hash` to use for creating the `sources` enum for
# `Ci::Pipeline`.
def self.sources
{
unknown: nil,
push: 1,
web: 2,
trigger: 3,
schedule: 4,
api: 5,
external: 6
}
end
end
end
Ci::PipelineEnums.prepend(EE::Ci::PipelineEnums)
......@@ -44,18 +44,9 @@ class CommitStatus < ActiveRecord::Base
scope :retried_ordered, -> { retried.ordered.includes(project: :namespace) }
scope :after_stage, -> (index) { where('stage_idx > ?', index) }
enum_with_nil failure_reason: {
unknown_failure: nil,
script_failure: 1,
api_failure: 2,
stuck_or_timeout_failure: 3,
runner_system_failure: 4,
missing_dependency_failure: 5,
runner_unsupported: 6,
stale_schedule: 7,
job_execution_timeout: 8,
archived_failure: 9
}.merge(EE_FAILURE_REASONS)
# We use `CommitStatusEnums.failure_reasons` here so that EE can more easily
# extend this `Hash` with new values.
enum_with_nil failure_reason: ::CommitStatusEnums.failure_reasons
##
# We still create some CommitStatuses outside of CreatePipelineService.
......
# frozen_string_literal: true
module CommitStatusEnums
# Returns the Hash to use for creating the `failure_reason` enum for
# `CommitStatus`.
def self.failure_reasons
{
unknown_failure: nil,
script_failure: 1,
api_failure: 2,
stuck_or_timeout_failure: 3,
runner_system_failure: 4,
missing_dependency_failure: 5,
runner_unsupported: 6,
stale_schedule: 7,
job_execution_timeout: 8,
archived_failure: 9
}
end
end
CommitStatusEnums.prepend(EE::CommitStatusEnums)
......@@ -3,12 +3,9 @@
class UserCallout < ActiveRecord::Base
belongs_to :user
enum feature_name: {
gke_cluster_integration: 1,
gcp_signup_offer: 2,
cluster_security_warning: 3,
gold_trial: 4
}
# We use `UserCalloutEnums.feature_names` here so that EE can more easily
# extend this `Hash` with new values.
enum feature_name: ::UserCalloutEnums.feature_names
validates :user, presence: true
validates :feature_name,
......
# frozen_string_literal: true
module UserCalloutEnums
# Returns the `Hash` to use for the `feature_name` enum in the `UserCallout`
# model.
#
# This method is separate from the `UserCallout` model so that it can be
# extended by EE.
def self.feature_names
{
gke_cluster_integration: 1,
gcp_signup_offer: 2,
cluster_security_warning: 3
}
end
end
UserCalloutEnums.prepend(EE::UserCalloutEnums)
......@@ -2,12 +2,13 @@
module Ci
class PipelinePresenter < Gitlab::View::Presenter::Delegated
prepend ::EE::Ci::PipelinePresenter
include Gitlab::Utils::StrongMemoize
FAILURE_REASONS = {
config_error: 'CI/CD YAML configuration error!'
}.merge(EE_FAILURE_REASONS)
# We use a class method here instead of a constant, allowing EE to redefine
# the returned `Hash` more easily.
def self.failure_reasons
{ config_error: 'CI/CD YAML configuration error!' }
end
presents :pipeline
......@@ -22,7 +23,7 @@ module Ci
def failure_reason
return unless pipeline.failure_reason?
FAILURE_REASONS[pipeline.failure_reason.to_sym] ||
self.class.failure_reasons[pipeline.failure_reason.to_sym] ||
pipeline.failure_reason
end
......@@ -33,3 +34,5 @@ module Ci
end
end
end
Ci::PipelinePresenter.prepend(EE::Ci::PipelinePresenter)
# frozen_string_literal: true
module EE
module Ci
module PipelineEnums
extend ActiveSupport::Concern
class_methods do
extend ::Gitlab::Utils::Override
override :failure_reasons
def failure_reasons
super.merge(activity_limit_exceeded: 20, size_limit_exceeded: 21)
end
override :sources
def sources
super.merge(pipeline: 7, chat: 8)
end
end
end
end
end
# frozen_string_literal: true
module EE
module CommitStatusEnums
extend ActiveSupport::Concern
class_methods do
extend ::Gitlab::Utils::Override
override :failure_reasons
def failure_reasons
super.merge(protected_environment_failure: 1_000)
end
end
end
end
# frozen_string_literal: true
module EE
module UserCalloutEnums
extend ActiveSupport::Concern
class_methods do
extend ::Gitlab::Utils::Override
override :feature_names
def feature_names
super.merge(cluster_security_warning: 3, gold_trial: 4)
end
end
end
end
module EE
module Ci
module PipelinePresenter
EE_FAILURE_REASONS = {
activity_limit_exceeded: 'Pipeline activity limit exceeded!',
size_limit_exceeded: 'Pipeline size limit exceeded!'
}.freeze
extend ActiveSupport::Concern
class_methods do
extend ::Gitlab::Utils::Override
override :failure_reasons
def failure_reasons
super.merge(
activity_limit_exceeded: 'Pipeline activity limit exceeded!',
size_limit_exceeded: 'Pipeline size limit exceeded!'
)
end
end
def expose_security_dashboard?
any_report_artifact_for_type(:sast) ||
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment