From 590ff80f839179c1e178e941cd57946e9bb91b79 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 13 Nov 2018 17:29:14 +0100 Subject: 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 --- app/presenters/ci/pipeline_presenter.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'app/presenters/ci/pipeline_presenter.rb') 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 -- cgit v1.2.1