Merge branch 'issue_28457' into 'master'

Change state to integer for issuables - Patch 2 of 3

See merge request gitlab-org/gitlab!18099

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