Refactor how a few ActiveRecord enums are definedrefactor-enums-for-ee

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

7 files changed, 82 insertions, 34 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 56010e899a4..9512ba42f67 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -58,15 +58,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
-    }
+    # 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,
@@ -74,10 +68,9 @@ module Ci
       auto_devops_source: 2
     }
 
-    enum failure_reason: {
-      unknown_failure: 0,
-      config_error: 1
-    }
+    # 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
diff --git a/app/models/ci/pipeline_enums.rb b/app/models/ci/pipeline_enums.rb
new file mode 100644
index 00000000000..8d8d16e2ec1
--- /dev/null
+++ b/app/models/ci/pipeline_enums.rb
@@ -0,0 +1,28 @@
+# 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
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 755f8bd4d06..0f50bd39131 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -42,18 +42,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
-  }
+  # 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.
diff --git a/app/models/commit_status_enums.rb b/app/models/commit_status_enums.rb
new file mode 100644
index 00000000000..152105d9429
--- /dev/null
+++ b/app/models/commit_status_enums.rb
@@ -0,0 +1,20 @@
+# 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
diff --git a/app/models/user_callout.rb b/app/models/user_callout.rb
index 1cd05cf3aac..76e7bc06b4e 100644
--- a/app/models/user_callout.rb
+++ b/app/models/user_callout.rb
@@ -3,11 +3,9 @@
 class UserCallout < ActiveRecord::Base
   belongs_to :user
 
-  enum feature_name: {
-    gke_cluster_integration: 1,
-    gcp_signup_offer: 2,
-    cluster_security_warning: 3
-  }
+  # 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,
diff --git a/app/models/user_callout_enums.rb b/app/models/user_callout_enums.rb
new file mode 100644
index 00000000000..b9373ae6166
--- /dev/null
+++ b/app/models/user_callout_enums.rb
@@ -0,0 +1,16 @@
+# 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
diff --git a/app/presenters/ci/pipeline_presenter.rb b/app/presenters/ci/pipeline_presenter.rb
index 93a38f92073..57daf04efc6 100644
--- a/app/presenters/ci/pipeline_presenter.rb
+++ b/app/presenters/ci/pipeline_presenter.rb
@@ -4,9 +4,11 @@ module Ci
   class PipelinePresenter < Gitlab::View::Presenter::Delegated
     include Gitlab::Utils::StrongMemoize
 
-    FAILURE_REASONS = {
-      config_error: 'CI/CD YAML configuration error!'
-    }.freeze
+    # 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
 
@@ -21,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