Deprecate issues and merge requests state column

This is the second patch of the plan to migrate issues and
merge requests state column from string to integer type.
Deprecates usage  of old state:string column in ruby code.

app/finders/issuable_finder.rb

@@ -161,7 +161,7 @@ class IssuableFinder
     labels_count = label_names.any? ? label_names.count : 1
     labels_count = 1 if use_cte_for_search?
 
-    finder.execute.reorder(nil).group(:state).count.each do |key, value|
+    finder.execute.reorder(nil).group(:state_id).count.each do |key, value|
       counts[count_key(key)] += value / labels_count
     end
 
@@ -385,7 +385,8 @@ class IssuableFinder
   end
 
   def count_key(value)
-    Array(value).last.to_sym
+    value = Array(value).last
+    klass.available_states.key(value)
   end
 
   # Negates all params found in `negatable_params`
@@ -444,7 +445,6 @@ class IssuableFinder
     items
   end
 
-  # rubocop: disable CodeReuse/ActiveRecord
   def by_state(items)
     case params[:state].to_s
     when 'closed'
@@ -454,12 +454,11 @@ class IssuableFinder
     when 'opened'
       items.opened
     when 'locked'
-      items.where(state: 'locked')
+      items.with_state(:locked)
     else
       items
     end
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 
   def by_group(items)
     # Selection by group is already covered by `by_project` and `projects`

app/models/concerns/issuable.rb

@@ -32,6 +32,13 @@ module Issuable
   DESCRIPTION_LENGTH_MAX = 1.megabyte
   DESCRIPTION_HTML_LENGTH_MAX = 5.megabytes
 
+  STATE_ID_MAP = {
+    opened: 1,
+    closed: 2,
+    merged: 3,
+    locked: 4
+  }.with_indifferent_access.freeze
+
   # This object is used to gather issuable meta data for displaying
   # upvotes, downvotes, notes and closing merge requests count for issues and merge requests
   # lists avoiding n+1 queries and improving performance.
@@ -173,13 +180,17 @@ module Issuable
       fuzzy_search(query, [:title])
     end
 
-    # Available state values persisted in state_id column using state machine
+    def available_states
+      @available_states ||= STATE_ID_MAP.slice(*available_state_names)
+    end
+
+    # Available state names used to persist 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
+    # Check MergeRequest.available_states_names for example
+    def available_state_names
+      [:opened, :closed]
     end
 
     # Searches for records with a matching title or description.
@@ -298,6 +309,14 @@ module Issuable
     end
   end
 
+  def state
+    self.class.available_states.key(state_id)
+  end
+
+  def state=(value)
+    self.state_id = self.class.available_states[value]
+  end
+
   def resource_parent
     project
   end

app/models/concerns/issuable_states.rb

@@ -4,22 +4,20 @@ module IssuableStates
   extend ActiveSupport::Concern
 
   # 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 should be removed after the state column is removed.
-  # Check https://gitlab.com/gitlab-org/gitlab-foss/issues/51789 for more information
+  # This is a temporary hook to keep state column in sync until it is removed.
+  # Check https: https://gitlab.com/gitlab-org/gitlab/issues/33814 for more information
+  # The state column can be safely removed after 2019-10-27
   included do
-    before_save :set_state_id
+    before_save :sync_issuable_deprecated_state
   end
 
-  def set_state_id
-    return if state.nil? || state.empty?
+  def sync_issuable_deprecated_state
+    return if self.is_a?(Epic)
+    return unless respond_to?(:state)
+    return if state_id.nil?
 
-    # Needed to prevent breaking some migration specs that
-    # rollback database to a point where state_id does not exist.
-    # We can use this guard clause for now since this file will
-    # be removed in the next release.
-    return unless self.has_attribute?(:state_id)
+    deprecated_state = self.class.available_states.key(state_id)
 
-    self.state_id = self.class.available_states[state]
+    self.write_attribute(:state, deprecated_state)
   end
 end

app/models/concerns/milestoneish.rb

@@ -6,7 +6,9 @@ module Milestoneish
   end
 
   def closed_issues_count(user)
-    count_issues_by_state(user)['closed'].to_i
+    closed_state_id = Issue.available_states[:closed]
+
+    count_issues_by_state(user)[closed_state_id].to_i
   end
 
   def complete?(user)
@@ -117,7 +119,7 @@ module Milestoneish
 
   def count_issues_by_state(user)
     memoize_per_user(user, :count_issues_by_state) do
-      issues_visible_to_user(user).reorder(nil).group(:state).count
+      issues_visible_to_user(user).reorder(nil).group(:state_id).count
     end
   end
 

app/models/issue.rb

@@ -71,7 +71,7 @@ class Issue < ApplicationRecord
   attr_spammable :title, spam_title: true
   attr_spammable :description, spam_description: true
 
-  state_machine :state, initial: :opened do
+  state_machine :state_id, initial: :opened do
     event :close do
       transition [:opened] => :closed
     end
@@ -80,8 +80,8 @@ class Issue < ApplicationRecord
       transition closed: :opened
     end
 
-    state :opened
-    state :closed
+    state :opened, value: Issue.available_states[:opened]
+    state :closed, value: Issue.available_states[:closed]
 
     before_transition any => :closed do |issue|
       issue.closed_at = issue.system_note_timestamp
@@ -93,6 +93,13 @@ class Issue < ApplicationRecord
     end
   end
 
+  # Alias to state machine .with_state_id method
+  # This needs to be defined after the state machine block to avoid errors
+  class << self
+    alias_method :with_state, :with_state_id
+    alias_method :with_states, :with_state_ids
+  end
+
   def self.relative_positioning_query_base(issue)
     in_projects(issue.parent_ids)
   end

app/models/merge_request.rb

@@ -85,7 +85,13 @@ class MergeRequest < ApplicationRecord
   # when creating new merge request
   attr_accessor :can_be_created, :compare_commits, :diff_options, :compare
 
-  state_machine :state, initial: :opened do
+  # Keep states definition to be evaluated before the state_machine block to avoid spec failures.
+  # If this gets evaluated after, the `merged` and `locked` states which are overrided can be nil.
+  def self.available_state_names
+    super + [:merged, :locked]
+  end
+
+  state_machine :state_id, initial: :opened do
     event :close do
       transition [:opened] => :closed
     end
@@ -116,10 +122,17 @@ class MergeRequest < ApplicationRecord
       end
     end
 
-    state :opened
-    state :closed
-    state :merged
-    state :locked
+    state :opened, value: MergeRequest.available_states[:opened]
+    state :closed, value: MergeRequest.available_states[:closed]
+    state :merged, value: MergeRequest.available_states[:merged]
+    state :locked, value: MergeRequest.available_states[:locked]
+  end
+
+  # Alias to state machine .with_state_id method
+  # This needs to be defined after the state machine block to avoid errors
+  class << self
+    alias_method :with_state, :with_state_id
+    alias_method :with_states, :with_state_ids
   end
 
   state_machine :merge_status, initial: :unchecked do
@@ -211,10 +224,6 @@ class MergeRequest < ApplicationRecord
     '!'
   end
 
-  def self.available_states
-    @available_states ||= super.merge(merged: 3, locked: 4)
-  end
-
   # Returns the top 100 target branches
   #
   # The returned value is a Array containing branch names

app/models/merge_request_diff.rb

@@ -83,7 +83,7 @@ class MergeRequestDiff < ApplicationRecord
 
     metrics_join = mr_diffs.join(mr_metrics).on(metrics_join_condition)
 
-    condition = MergeRequest.arel_table[:state].eq(:merged)
+    condition = MergeRequest.arel_table[:state_id].eq(MergeRequest.available_states[:merged])
       .and(MergeRequest::Metrics.arel_table[:merged_at].lteq(before))
       .and(MergeRequest::Metrics.arel_table[:merged_at].not_eq(nil))
 
@@ -91,7 +91,7 @@ class MergeRequestDiff < ApplicationRecord
   end
 
   scope :old_closed_diffs, -> (before) do
-    condition = MergeRequest.arel_table[:state].eq(:closed)
+    condition = MergeRequest.arel_table[:state_id].eq(MergeRequest.available_states[:closed])
       .and(MergeRequest::Metrics.arel_table[:latest_closed_at].lteq(before))
 
     joins(merge_request: :metrics).where(condition)

app/models/project_services/hipchat_service.rb

@@ -161,7 +161,7 @@ class HipchatService < Service
     obj_attr = data[:object_attributes]
     obj_attr = HashWithIndifferentAccess.new(obj_attr)
     title = render_line(obj_attr[:title])
-    state = obj_attr[:state]
+    state = Issue.available_states.key(obj_attr[:state_id])
     issue_iid = obj_attr[:iid]
     issue_url = obj_attr[:url]
     description = obj_attr[:description]

app/models/push_event.rb

@@ -54,7 +54,7 @@ class PushEvent < Event
       .select(1)
       .where('merge_requests.source_project_id = events.project_id')
       .where('merge_requests.source_branch = push_event_payloads.ref')
-      .where(state: :opened)
+      .with_state(:opened)
 
     # For reasons unknown the use of #eager_load will result in the
     # "push_event_payload" association not being set. Because of this we're

app/workers/namespaceless_project_destroy_worker.rb

@@ -33,6 +33,6 @@ class NamespacelessProjectDestroyWorker
   def unlink_fork(project)
     merge_requests = project.forked_from_project.merge_requests.opened.from_project(project)
 
-    merge_requests.update_all(state: 'closed')
+    merge_requests.update_all(state_id: MergeRequest.available_states[:closed])
   end
 end

app/workers/stuck_merge_jobs_worker.rb

@@ -33,7 +33,7 @@ class StuckMergeJobsWorker
   def apply_current_state!(completed_jids, completed_ids)
     merge_requests = MergeRequest.where(id: completed_ids)
 
-    merge_requests.where.not(merge_commit_sha: nil).update_all(state: :merged)
+    merge_requests.where.not(merge_commit_sha: nil).update_all(state_id: MergeRequest.available_states[:merged])
 
     merge_requests_to_reopen = merge_requests.where(merge_commit_sha: nil)
 

changelogs/unreleased/issue_28457.yml

+---
+title: Deprecate usage of state column for issues and merge requests
+merge_request: 18099
+author:
+type: changed

db/migrate/20191008180203_add_issuable_state_id_indexes.rb

+# frozen_string_literal: true
+
+class AddIssuableStateIdIndexes < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    # Creates the same indexes that are currently using state:string column
+    # for issues and merge_requests tables
+    create_indexes_for_issues
+    create_indexes_for_merge_requests
+  end
+
+  def down
+    # Removes indexes for issues
+    remove_concurrent_index_by_name :issues, 'idx_issues_on_state_id'
+    remove_concurrent_index_by_name :issues, 'idx_issues_on_project_id_and_created_at_and_id_and_state_id'
+    remove_concurrent_index_by_name :issues, 'idx_issues_on_project_id_and_due_date_and_id_and_state_id'
+    remove_concurrent_index_by_name :issues, 'idx_issues_on_project_id_and_rel_position_and_state_id_and_id'
+    remove_concurrent_index_by_name :issues, 'idx_issues_on_project_id_and_updated_at_and_id_and_state_id'
+
+    # Removes indexes from merge_requests
+    remove_concurrent_index_by_name :merge_requests, 'idx_merge_requests_on_id_and_merge_jid'
+    remove_concurrent_index_by_name :merge_requests, 'idx_merge_requests_on_source_project_and_branch_state_opened'
+    remove_concurrent_index_by_name :merge_requests, 'idx_merge_requests_on_state_id_and_merge_status'
+    remove_concurrent_index_by_name :merge_requests, 'idx_merge_requests_on_target_project_id_and_iid_opened'
+  end
+
+  def create_indexes_for_issues
+    add_concurrent_index :issues, :state_id, name: 'idx_issues_on_state_id'
+
+    add_concurrent_index :issues,
+                         [:project_id, :created_at, :id, :state_id],
+                         name: 'idx_issues_on_project_id_and_created_at_and_id_and_state_id'
+
+    add_concurrent_index :issues,
+                         [:project_id, :due_date, :id, :state_id],
+                         where: 'due_date IS NOT NULL',
+                         name: 'idx_issues_on_project_id_and_due_date_and_id_and_state_id'
+
+    add_concurrent_index :issues,
+                         [:project_id, :relative_position, :state_id, :id],
+                         order: { id: :desc },
+                         name: 'idx_issues_on_project_id_and_rel_position_and_state_id_and_id'
+
+    add_concurrent_index :issues,
+                         [:project_id, :updated_at, :id, :state_id],
+                         name: 'idx_issues_on_project_id_and_updated_at_and_id_and_state_id'
+  end
+
+  def create_indexes_for_merge_requests
+    add_concurrent_index :merge_requests,
+                         [:id, :merge_jid],
+                         where: 'merge_jid IS NOT NULL and state_id = 4',
+                         name: 'idx_merge_requests_on_id_and_merge_jid'
+
+    add_concurrent_index :merge_requests,
+                         [:source_project_id, :source_branch],
+                         where: 'state_id = 1',
+                         name: 'idx_merge_requests_on_source_project_and_branch_state_opened'
+
+    add_concurrent_index :merge_requests,
+                         [:state_id, :merge_status],
+                         where: "state_id = 1 AND merge_status = 'can_be_merged'",
+                         name: 'idx_merge_requests_on_state_id_and_merge_status'
+
+    add_concurrent_index :merge_requests,
+                         [:target_project_id, :iid],
+                         where: 'state_id = 1',
+                         name: 'idx_merge_requests_on_target_project_id_and_iid_opened'
+  end
+end

db/migrate/20191008200204_add_state_id_default_value.rb

+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddStateIdDefaultValue < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    change_column_default :issues, :state_id, 1
+    change_column_null :issues, :state_id, false
+    change_column_default :merge_requests, :state_id, 1
+    change_column_null :merge_requests, :state_id, false
+  end
+
+  def down
+    change_column_default :issues, :state_id, nil
+    change_column_null :issues, :state_id, true
+    change_column_default :merge_requests, :state_id, nil
+    change_column_null :merge_requests, :state_id, true
+  end
+end

db/schema.rb

@@ -1928,7 +1928,7 @@ ActiveRecord::Schema.define(version: 2019_10_16_220135) do
     t.boolean "discussion_locked"
     t.datetime_with_timezone "closed_at"
     t.integer "closed_by_id"
-    t.integer "state_id", limit: 2
+    t.integer "state_id", limit: 2, default: 1, null: false
     t.integer "duplicated_to_id"
     t.index ["author_id"], name: "index_issues_on_author_id"
     t.index ["closed_by_id"], name: "index_issues_on_closed_by_id"
@@ -1938,12 +1938,17 @@ ActiveRecord::Schema.define(version: 2019_10_16_220135) do
     t.index ["milestone_id"], name: "index_issues_on_milestone_id"
     t.index ["moved_to_id"], name: "index_issues_on_moved_to_id", where: "(moved_to_id IS NOT NULL)"
     t.index ["project_id", "created_at", "id", "state"], name: "index_issues_on_project_id_and_created_at_and_id_and_state"
+    t.index ["project_id", "created_at", "id", "state_id"], name: "idx_issues_on_project_id_and_created_at_and_id_and_state_id"
     t.index ["project_id", "due_date", "id", "state"], name: "idx_issues_on_project_id_and_due_date_and_id_and_state_partial", where: "(due_date IS NOT NULL)"
+    t.index ["project_id", "due_date", "id", "state_id"], name: "idx_issues_on_project_id_and_due_date_and_id_and_state_id", where: "(due_date IS NOT NULL)"
     t.index ["project_id", "iid"], name: "index_issues_on_project_id_and_iid", unique: true
     t.index ["project_id", "relative_position", "state", "id"], name: "index_issues_on_project_id_and_rel_position_and_state_and_id", order: { id: :desc }
+    t.index ["project_id", "relative_position", "state_id", "id"], name: "idx_issues_on_project_id_and_rel_position_and_state_id_and_id", order: { id: :desc }
     t.index ["project_id", "updated_at", "id", "state"], name: "index_issues_on_project_id_and_updated_at_and_id_and_state"
+    t.index ["project_id", "updated_at", "id", "state_id"], name: "idx_issues_on_project_id_and_updated_at_and_id_and_state_id"
     t.index ["relative_position"], name: "index_issues_on_relative_position"
     t.index ["state"], name: "index_issues_on_state"
+    t.index ["state_id"], name: "idx_issues_on_state_id"
     t.index ["title"], name: "index_issues_on_title_trigram", opclass: :gin_trgm_ops, using: :gin
     t.index ["updated_at"], name: "index_issues_on_updated_at"
     t.index ["updated_by_id"], name: "index_issues_on_updated_by_id", where: "(updated_by_id IS NOT NULL)"
@@ -2288,23 +2293,27 @@ ActiveRecord::Schema.define(version: 2019_10_16_220135) do
     t.boolean "discussion_locked"
     t.integer "latest_merge_request_diff_id"
     t.boolean "allow_maintainer_to_push"
-    t.integer "state_id", limit: 2
+    t.integer "state_id", limit: 2, default: 1, null: false
     t.string "rebase_jid"
     t.index ["assignee_id"], name: "index_merge_requests_on_assignee_id"
     t.index ["author_id"], name: "index_merge_requests_on_author_id"
     t.index ["created_at"], name: "index_merge_requests_on_created_at"
     t.index ["description"], name: "index_merge_requests_on_description_trigram", opclass: :gin_trgm_ops, using: :gin
     t.index ["head_pipeline_id"], name: "index_merge_requests_on_head_pipeline_id"
+    t.index ["id", "merge_jid"], name: "idx_merge_requests_on_id_and_merge_jid", where: "((merge_jid IS NOT NULL) AND (state_id = 4))"
     t.index ["id", "merge_jid"], name: "index_merge_requests_on_id_and_merge_jid", where: "((merge_jid IS NOT NULL) AND ((state)::text = 'locked'::text))"
     t.index ["latest_merge_request_diff_id"], name: "index_merge_requests_on_latest_merge_request_diff_id"
     t.index ["merge_user_id"], name: "index_merge_requests_on_merge_user_id", where: "(merge_user_id IS NOT NULL)"
     t.index ["milestone_id"], name: "index_merge_requests_on_milestone_id"
     t.index ["source_branch"], name: "index_merge_requests_on_source_branch"
+    t.index ["source_project_id", "source_branch"], name: "idx_merge_requests_on_source_project_and_branch_state_opened", where: "(state_id = 1)"
     t.index ["source_project_id", "source_branch"], name: "index_merge_requests_on_source_project_and_branch_state_opened", where: "((state)::text = 'opened'::text)"
     t.index ["source_project_id", "source_branch"], name: "index_merge_requests_on_source_project_id_and_source_branch"
     t.index ["state", "merge_status"], name: "index_merge_requests_on_state_and_merge_status", where: "(((state)::text = 'opened'::text) AND ((merge_status)::text = 'can_be_merged'::text))"
+    t.index ["state_id", "merge_status"], name: "idx_merge_requests_on_state_id_and_merge_status", where: "((state_id = 1) AND ((merge_status)::text = 'can_be_merged'::text))"
     t.index ["target_branch"], name: "index_merge_requests_on_target_branch"
     t.index ["target_project_id", "created_at"], name: "index_merge_requests_target_project_id_created_at"
+    t.index ["target_project_id", "iid"], name: "idx_merge_requests_on_target_project_id_and_iid_opened", where: "(state_id = 1)"
     t.index ["target_project_id", "iid"], name: "index_merge_requests_on_target_project_id_and_iid", unique: true
     t.index ["target_project_id", "iid"], name: "index_merge_requests_on_target_project_id_and_iid_opened", where: "((state)::text = 'opened'::text)"
     t.index ["target_project_id", "merge_commit_sha", "id"], name: "index_merge_requests_on_tp_id_and_merge_commit_sha_and_id"

ee/app/models/approval_merge_request_rule.rb

@@ -14,7 +14,7 @@ class ApprovalMergeRequestRule < ApplicationRecord
     )
   end
   scope :for_unmerged_merge_requests, -> (merge_requests = nil) do
-    query = joins(:merge_request).where.not(merge_requests: { state: 'merged' })
+    query = joins(:merge_request).where.not(merge_requests: { state_id: MergeRequest.available_states[:merged] })
 
     if merge_requests
       query.where(merge_request_id: merge_requests)

ee/app/models/ee/epic.rb

@@ -15,7 +15,11 @@ module EE
       include RelativePositioning
       include UsageStatistics
 
-      enum state_id: { opened: 1, closed: 2 }
+      enum state_id: {
+        opened: ::Epic.available_states[:opened],
+        closed: ::Epic.available_states[:closed]
+      }
+
       alias_attribute :state, :state_id
 
       belongs_to :closed_by, class_name: 'User'

ee/app/serializers/ee/merge_request_widget_entity.rb

@@ -159,7 +159,7 @@ module EE
         visible_mrs = object.visible_blocking_merge_requests(current_user)
         visible_mrs_by_state = visible_mrs
           .map { |mr| represent_blocking_mr(mr) }
-          .group_by { |blocking_mr| blocking_mr.object.state_name }
+          .group_by { |blocking_mr| blocking_mr.object.state_id_name }
 
         {
           total_count: visible_mrs.count + hidden_blocking_count,

ee/lib/gitlab/background_migration/migrate_approver_to_approval_rules.rb

@@ -99,8 +99,16 @@ module Gitlab
           @approver_group_ids ||= ApproverGroup.where(target_type: 'MergeRequest', target_id: id).joins(:group).pluck(distinct(:group_id))
         end
 
+        def merged_state_id
+          3
+        end
+
+        def closed_state_id
+          2
+        end
+
         def sync_code_owners_with_approvers
-          return if state == 'merged' || state == 'closed'
+          return if state_id == merged_state_id || state == closed_state_id
 
           Gitlab::GitalyClient.allow_n_plus_1_calls do
             gl_merge_request = ::MergeRequest.find(id)

ee/spec/lib/ee/gitlab/hook_data/issue_builder_spec.rb

@@ -25,7 +25,7 @@ describe Gitlab::HookData::IssueBuilder do
         moved_to_id
         project_id
         relative_position
-        state
+        state_id
         time_estimate
         title
         updated_at

ee/spec/models/concerns/elastic/issue_spec.rb

@@ -108,12 +108,12 @@ describe Issue, :elastic do
       'description',
       'created_at',
       'updated_at',
-      'state',
       'project_id',
       'author_id',
       'confidential'
     ).merge({
       'type' => issue.es_type,
+      'state' => issue.state,
       'join_field' => {
         'name' => issue.es_type,
         'parent' => issue.es_parent

ee/spec/models/merge_request/blocking_spec.rb

@@ -131,7 +131,7 @@ describe MergeRequest do
 
       context 'MR is merged' do
         before do
-          blocking_mr.update_columns(state: 'merged')
+          blocking_mr.update_columns(state_id: described_class.available_states[:merged])
         end
 
         it 'returns 0 by default' do

ee/spec/serializers/merge_request_widget_entity_spec.rb

@@ -271,7 +271,7 @@ describe MergeRequestWidgetEntity do
       end
 
       it 'does not count a merged and hidden blocking MR' do
-        blocking_mr.update_columns(state: 'merged')
+        blocking_mr.update_columns(state_id: MergeRequest.available_states[:merged])
 
         is_expected.to eq(
           hidden_count: 0,

lib/gitlab/bitbucket_import/importer.rb

@@ -104,7 +104,7 @@ module Gitlab
             iid: issue.iid,
             title: issue.title,
             description: description,
-            state: issue.state,
+            state_id: Issue.available_states[issue.state],
             author_id: gitlab_user_id(project, issue.author),
             milestone: milestone,
             created_at: issue.created_at,

lib/gitlab/google_code_import/importer.rb

@@ -117,7 +117,7 @@ module Gitlab
             description:  body,
             author_id:    project.creator_id,
             assignee_ids: [assignee_id],
-            state:        raw_issue['state'] == 'closed' ? 'closed' : 'opened'
+            state_id:     raw_issue['state'] == 'closed' ? Issue.available_states[:closed] : Issue.available_states[:opened]
           )
 
           issue_labels = ::LabelsFinder.new(nil, project_id: project.id, title: labels).execute(skip_authorization: true)

lib/gitlab/hook_data/issue_builder.rb

@@ -27,7 +27,7 @@ module Gitlab
           duplicated_to_id
           project_id
           relative_position
-          state
+          state_id
           time_estimate
           title
           updated_at
@@ -46,7 +46,8 @@ module Gitlab
             human_time_estimate: issue.human_time_estimate,
             assignee_ids: issue.assignee_ids,
             assignee_id: issue.assignee_ids.first, # This key is deprecated
-            labels: issue.labels_hook_attrs
+            labels: issue.labels_hook_attrs,
+            state: issue.state
         }
 
         issue.attributes.with_indifferent_access.slice(*self.class.safe_hook_attributes)

lib/gitlab/import_export/relation_factory.rb

@@ -292,7 +292,11 @@ module Gitlab
 
             existing_object
           else
-            relation_class.new(parsed_relation_hash)
+            object = relation_class.new
+
+            # Use #assign_attributes here to call object custom setters
+            object.assign_attributes(parsed_relation_hash)
+            object
           end
         end
       end

spec/factories/issues.rb

@@ -12,7 +12,7 @@ FactoryBot.define do
     end
 
     trait :opened do
-      state { :opened }
+      state_id { Issue.available_states[:opened] }
     end
 
     trait :locked do
@@ -20,10 +20,14 @@ FactoryBot.define do
     end
 
     trait :closed do
-      state { :closed }
+      state_id { Issue.available_states[:closed] }
       closed_at { Time.now }
     end
 
+    after(:build) do |issue, evaluator|
+      issue.state_id = Issue.available_states[evaluator.state]
+    end
+
     factory :closed_issue, traits: [:closed]
     factory :reopened_issue, traits: [:opened]
 

spec/factories/merge_requests.rb

@@ -40,7 +40,7 @@ FactoryBot.define do
     end
 
     trait :merged do
-      state { :merged }
+      state_id { MergeRequest.available_states[:merged] }
     end
 
     trait :merged_target do
@@ -57,7 +57,7 @@ FactoryBot.define do
     end
 
     trait :closed do
-      state { :closed }
+      state_id { MergeRequest.available_states[:closed] }
     end
 
     trait :closed_last_month do
@@ -69,7 +69,7 @@ FactoryBot.define do
     end
 
     trait :opened do
-      state { :opened }
+      state_id { MergeRequest.available_states[:opened] }
     end
 
     trait :invalid do
@@ -78,7 +78,7 @@ FactoryBot.define do
     end
 
     trait :locked do
-      state { :locked }
+      state_id { MergeRequest.available_states[:locked] }
     end
 
     trait :simple do
@@ -186,6 +186,10 @@ FactoryBot.define do
       end
     end
 
+    after(:build) do |merge_request, evaluator|
+      merge_request.state_id = MergeRequest.available_states[evaluator.state]
+    end
+
     after(:create) do |merge_request, evaluator|
       merge_request.cache_merge_request_closes_issues!
     end

spec/fixtures/lib/gitlab/import_export/project.group.json

@@ -129,7 +129,7 @@
       "updated_at": "2017-08-15T18:37:40.807Z",
       "branch_name": null,
       "description": "Quam totam fuga numquam in eveniet.",
-      "state": "opened",
+      "state": "closed",
       "iid": 2,
       "updated_by_id": 1,
       "confidential": false,

spec/lib/gitlab/bitbucket_import/importer_spec.rb

@@ -308,8 +308,8 @@ describe Gitlab::BitbucketImport::Importer do
 
       importer.execute
 
-      expect(project.issues.where(state: "closed").size).to eq(5)
-      expect(project.issues.where(state: "opened").size).to eq(2)
+      expect(project.issues.where(state_id: Issue.available_states[:closed]).size).to eq(5)
+      expect(project.issues.where(state_id: Issue.available_states[:opened]).size).to eq(2)
     end
 
     describe 'wiki import' do

spec/lib/gitlab/hook_data/issue_builder_spec.rb

@@ -26,7 +26,7 @@ describe Gitlab::HookData::IssueBuilder do
         duplicated_to_id
         project_id
         relative_position
-        state
+        state_id
         time_estimate
         title
         updated_at
@@ -41,6 +41,7 @@ describe Gitlab::HookData::IssueBuilder do
       expect(data).to include(:human_time_estimate)
       expect(data).to include(:human_total_time_spent)
       expect(data).to include(:assignee_ids)
+      expect(data).to include(:state)
       expect(data).to include('labels' => [label.hook_attrs])
     end
 

spec/lib/gitlab/import_export/project_tree_restorer_spec.rb

@@ -434,6 +434,11 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
                       labels: 0,
                       milestones: 0,
                       first_issue_labels: 1
+
+      it 'restores issue states' do
+        expect(project.issues.with_state(:closed).count).to eq(1)
+        expect(project.issues.with_state(:opened).count).to eq(1)
+      end
     end
 
     context 'with existing group models' do

spec/lib/gitlab/import_export/relation_factory_spec.rb

@@ -85,7 +85,7 @@ describe Gitlab::ImportExport::RelationFactory do
   class FooModel
     include ActiveModel::Model
 
-    def initialize(params)
+    def initialize(params = {})
       params.each { |key, value| send("#{key}=", value) }
     end
 

spec/models/issue_spec.rb

@@ -138,7 +138,10 @@ describe Issue do
     end
 
     it 'changes the state to closed' do
-      expect { issue.close }.to change { issue.state }.from('opened').to('closed')
+      open_state = described_class.available_states[:opened]
+      closed_state = described_class.available_states[:closed]
+
+      expect { issue.close }.to change { issue.state_id }.from(open_state).to(closed_state)
     end
   end
 
@@ -155,7 +158,7 @@ describe Issue do
     end
 
     it 'changes the state to opened' do
-      expect { issue.reopen }.to change { issue.state }.from('closed').to('opened')
+      expect { issue.reopen }.to change { issue.state_id }.from(described_class.available_states[:closed]).to(described_class.available_states[:opened])
     end
   end
 

spec/models/merge_request_spec.rb

@@ -470,7 +470,7 @@ describe MergeRequest do
       commit = double('commit1', safe_message: "Fixes #{issue.to_reference}")
 
       allow(subject).to receive(:commits).and_return([commit])
-      allow(subject).to receive(:state).and_return("closed")
+      allow(subject).to receive(:state_id).and_return(described_class.available_states[:closed])
 
       expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to change(subject.merge_requests_closing_issues, :count)
     end
@@ -479,7 +479,7 @@ describe MergeRequest do
       issue  = create :issue, project: subject.project
       commit = double('commit1', safe_message: "Fixes #{issue.to_reference}")
       allow(subject).to receive(:commits).and_return([commit])
-      allow(subject).to receive(:state).and_return("merged")
+      allow(subject).to receive(:state_id).and_return(described_class.available_states[:merged])
 
       expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to change(subject.merge_requests_closing_issues, :count)
     end
@@ -2075,7 +2075,7 @@ describe MergeRequest do
     end
 
     it 'refuses to enqueue a job if the MR is not open' do
-      merge_request.update_column(:state, 'foo')
+      merge_request.update_column(:state_id, 5)
 
       expect(RebaseWorker).not_to receive(:perform_async)
 
@@ -2571,32 +2571,32 @@ describe MergeRequest do
 
   describe '#merge_ongoing?' do
     it 'returns true when the merge request is locked' do
-      merge_request = build_stubbed(:merge_request, state: :locked)
+      merge_request = build_stubbed(:merge_request, state_id: described_class.available_states[:locked])
 
       expect(merge_request.merge_ongoing?).to be(true)
     end
 
     it 'returns true when merge_id, MR is not merged and it has no running job' do
-      merge_request = build_stubbed(:merge_request, state: :open, merge_jid: 'foo')
+      merge_request = build_stubbed(:merge_request, state_id: described_class.available_states[:opened], merge_jid: 'foo')
       allow(Gitlab::SidekiqStatus).to receive(:running?).with('foo') { true }
 
       expect(merge_request.merge_ongoing?).to be(true)
     end
 
     it 'returns false when merge_jid is nil' do
-      merge_request = build_stubbed(:merge_request, state: :open, merge_jid: nil)
+      merge_request = build_stubbed(:merge_request, state_id: described_class.available_states[:opened], merge_jid: nil)
 
       expect(merge_request.merge_ongoing?).to be(false)
     end
 
     it 'returns false if MR is merged' do
-      merge_request = build_stubbed(:merge_request, state: :merged, merge_jid: 'foo')
+      merge_request = build_stubbed(:merge_request, state_id: described_class.available_states[:merged], merge_jid: 'foo')
 
       expect(merge_request.merge_ongoing?).to be(false)
     end
 
     it 'returns false if there is no merge job running' do
-      merge_request = build_stubbed(:merge_request, state: :open, merge_jid: 'foo')
+      merge_request = build_stubbed(:merge_request, state_id: described_class.available_states[:opened], merge_jid: 'foo')
       allow(Gitlab::SidekiqStatus).to receive(:running?).with('foo') { false }
 
       expect(merge_request.merge_ongoing?).to be(false)
@@ -2730,7 +2730,7 @@ describe MergeRequest do
 
       context 'closed MR' do
         before do
-          merge_request.update_attribute(:state, :closed)
+          merge_request.update_attribute(:state_id, described_class.available_states[:closed])
         end
 
         it 'is not mergeable' do