From 26f40aefb09b96538fa99f74d46542ad39bb1679 Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Wed, 13 Feb 2019 17:31:14 -0200
Subject: [PATCH] Code improvements

---
 app/models/concerns/issuable.rb               |  9 ++++++++
 app/models/concerns/issuable_states.rb        | 21 +------------------
 app/models/merge_request.rb                   |  5 ++++-
 ...0190211131150_add_state_id_to_issuables.rb |  8 +++----
 4 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 7bf553a0221..83bd8cc6478 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -132,6 +132,15 @@ module Issuable
       fuzzy_search(query, [:title])
     end
 
+    # Available state values persisted in state_id column using state machine
+    #
+    # Override this on subclasses if different states are needed
+    #
+    # Check MergeRequest.available_states for example
+    def available_states
+      @available_states ||= { opened: 1, closed: 2 }.with_indifferent_access
+    end
+
     # Searches for records with a matching title or description.
     #
     # This method uses ILIKE on PostgreSQL and LIKE on MySQL.
diff --git a/app/models/concerns/issuable_states.rb b/app/models/concerns/issuable_states.rb
index f9f5797065d..d7992f8e6db 100644
--- a/app/models/concerns/issuable_states.rb
+++ b/app/models/concerns/issuable_states.rb
@@ -1,25 +1,6 @@
-# frozen_string_literal: true
-
-# == IssuableStates concern
-#
-# Defines states shared by issuables which are persisted on state_id column
-# using the state machine.
-#
-# Used by EE::Epic, Issue and MergeRequest
-#
 module IssuableStates
   extend ActiveSupport::Concern
 
-  # Override this constant on model where different states are needed
-  # Check MergeRequest::AVAILABLE_STATES
-  AVAILABLE_STATES = { opened: 1, closed: 2 }.freeze
-
-  class_methods do
-    def states
-      @states ||= OpenStruct.new(self::AVAILABLE_STATES)
-    end
-  end
-
   # The state:string column is being migrated to state_id:integer column
   # This is a temporary hook to populate state_id column with new values
   # and can be removed after the state column is removed.
@@ -30,7 +11,7 @@ module IssuableStates
   def set_state_id
     return if state.nil? || state.empty?
 
-    states_hash = self.class.states.to_h.with_indifferent_access
+    states_hash = self.class.available_states
 
     self.state_id = states_hash[state]
   end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index ece31b359d1..6baf479d8d1 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -22,7 +22,6 @@ class MergeRequest < ActiveRecord::Base
   self.reactive_cache_lifetime = 10.minutes
 
   SORTING_PREFERENCE_FIELD = :merge_requests_sort
-  AVAILABLE_STATES = AVAILABLE_STATES.merge(merged: 3, locked: 4).freeze
 
   ignore_column :locked_at,
                 :ref_fetched,
@@ -194,6 +193,10 @@ class MergeRequest < ActiveRecord::Base
     '!'
   end
 
+  def self.available_states
+    @states ||= super.merge(locked: 3, merged: 4)
+  end
+
   def rebase_in_progress?
     strong_memoize(:rebase_in_progress) do
       # The source project can be deleted
diff --git a/db/migrate/20190211131150_add_state_id_to_issuables.rb b/db/migrate/20190211131150_add_state_id_to_issuables.rb
index d23c946cf88..440f577e1a3 100644
--- a/db/migrate/20190211131150_add_state_id_to_issuables.rb
+++ b/db/migrate/20190211131150_add_state_id_to_issuables.rb
@@ -8,9 +8,9 @@ class AddStateIdToIssuables < ActiveRecord::Migration[5.0]
   # 2019-02-12 Gitlab.com issuable numbers
   # issues count: 13587305
   # merge requests count: 18925274
-  # Using this 50000 as batch size should take around 13 hours
+  # Using this 25000 as batch size should take around 26 hours
   # to migrate both issues and merge requests
-  BATCH_SIZE = 50000
+  BATCH_SIZE = 25000
   DELAY_INTERVAL = 5.minutes.to_i
 
   class Issue < ActiveRecord::Base
@@ -26,8 +26,8 @@ class AddStateIdToIssuables < ActiveRecord::Migration[5.0]
   end
 
   def up
-    add_column :issues, :state_id, :integer, limit: 1
-    add_column :merge_requests, :state_id, :integer, limit: 1
+    add_column :issues, :state_id, :integer, limit: 2
+    add_column :merge_requests, :state_id, :integer, limit: 2
 
     # Is this safe?
     # Added to avoid an warning about jobs running inside transactions.