Merge branch 'issue_33814' into 'master'

Ignore state column for issues/merge_requests

See merge request gitlab-org/gitlab!19574

app/models/concerns/issuable.rb

@@ -23,7 +23,6 @@ module Issuable
   include Sortable
   include CreatedAtFilterable
   include UpdatedAtFilterable
-  include IssuableStates
   include ClosedAtFilterable
   include VersionedDescription
 

app/models/concerns/issuable_states.rb

-# frozen_string_literal: true
-
-module IssuableStates
-  extend ActiveSupport::Concern
-
-  # The state:string column is being migrated to state_id:integer column
-  # 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 :sync_issuable_deprecated_state
-  end
-
-  def sync_issuable_deprecated_state
-    return if self.is_a?(Epic)
-    return unless respond_to?(:state)
-    return if state_id.nil?
-
-    deprecated_state = self.class.available_states.key(state_id)
-
-    self.write_attribute(:state, deprecated_state)
-  end
-end

app/models/issue.rb

@@ -66,7 +66,10 @@ class Issue < ApplicationRecord
   scope :public_only, -> { where(confidential: false) }
   scope :confidential_only, -> { where(confidential: true) }
 
-  scope :counts_by_state, -> { reorder(nil).group(:state).count }
+  scope :counts_by_state, -> { reorder(nil).group(:state_id).count }
+
+  # Only remove after 2019-12-22 and with %12.7
+  self.ignored_columns += %i[state]
 
   after_commit :expire_etag_cache
   after_save :ensure_metrics, unless: :imported?

app/models/merge_request.rb

@@ -228,6 +228,9 @@ class MergeRequest < ApplicationRecord
     with_state(:opened).where(auto_merge_enabled: true)
   end
 
+  # Only remove after 2019-12-22 and with %12.7
+  self.ignored_columns += %i[state]
+
   after_save :keep_around_commit
 
   alias_attribute :project, :target_project

app/serializers/analytics_merge_request_entity.rb

 # frozen_string_literal: true
 
 class AnalyticsMergeRequestEntity < AnalyticsIssueEntity
-  expose :state
+  expose :state do |object|
+    MergeRequest.available_states.key(object[:state_id])
+  end
 
   expose :url do |object|
     url_to(:namespace_project_merge_request, object)

app/services/issuable_base_service.rb

@@ -397,7 +397,7 @@ class IssuableBaseService < BaseService
   end
 
   def update_project_counter_caches?(issuable)
-    issuable.state_changed?
+    issuable.state_id_changed?
   end
 
   def parent

config/pseudonymizer.yml

@@ -93,7 +93,7 @@ tables:
     - updated_at
     - description
     - milestone_id
-    - state
+    - state_id
     - updated_by_id
     - weight
     - due_date
@@ -174,7 +174,7 @@ tables:
     - created_at
     - updated_at
     - milestone_id
-    - state
+    - state_id
     - merge_status
     - target_project_id
     - updated_by_id

ee/app/services/epics/descendant_count_service.rb

@@ -20,11 +20,15 @@ module Epics
     end
 
     def opened_issues
-      issues_count.fetch('opened', 0)
+      opened_state_id = Issue.available_states[:opened]
+
+      issues_count.fetch(opened_state_id, 0)
     end
 
     def closed_issues
-      issues_count.fetch('closed', 0)
+      closed_state_id = Issue.available_states[:closed]
+
+      issues_count.fetch(closed_state_id, 0)
     end
 
     private

ee/lib/gitlab/background_migration/migrate_approver_to_approval_rules.rb

@@ -108,7 +108,7 @@ module Gitlab
         end
 
         def sync_code_owners_with_approvers
-          return if state_id == merged_state_id || state == closed_state_id
+          return if state_id == merged_state_id || state_id == closed_state_id
 
           Gitlab::GitalyClient.allow_n_plus_1_calls do
             gl_merge_request = ::MergeRequest.find(id)

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

@@ -79,6 +79,7 @@ describe MergeRequest, :elastic do
       'target_project_id',
       'author_id'
     ).merge({
+      'state' => merge_request.state,
       'type' => merge_request.es_type,
       'join_field' => {
         'name' => merge_request.es_type,

lib/gitlab/analytics/cycle_analytics/records_fetcher.rb

@@ -21,7 +21,7 @@ module Gitlab
             finder_class: MergeRequestsFinder,
             serializer_class: AnalyticsMergeRequestSerializer,
             includes_for_query: { target_project: [:namespace], author: [] },
-            columns_for_select: %I[title iid id created_at author_id state target_project_id]
+            columns_for_select: %I[title iid id created_at author_id state_id target_project_id]
           }
         }.freeze
 

lib/gitlab/bitbucket_server_import/importer.rb

@@ -200,7 +200,6 @@ module Gitlab
           target_project_id: project.id,
           target_branch: Gitlab::Git.ref_name(pull_request.target_branch_name),
           target_branch_sha: pull_request.target_branch_sha,
-          state: pull_request.state,
           state_id: MergeRequest.available_states[pull_request.state],
           author_id: author_id,
           assignee_id: nil,

lib/gitlab/cycle_analytics/code_event_fetcher.rb

@@ -10,7 +10,7 @@ module Gitlab
                         mr_table[:iid],
                         mr_table[:id],
                         mr_table[:created_at],
-                        mr_table[:state],
+                        mr_table[:state_id],
                         mr_table[:author_id]]
         @order = mr_table[:created_at]
 

lib/gitlab/cycle_analytics/review_event_fetcher.rb

@@ -10,7 +10,7 @@ module Gitlab
                         mr_table[:iid],
                         mr_table[:id],
                         mr_table[:created_at],
-                        mr_table[:state],
+                        mr_table[:state_id],
                         mr_table[:author_id]]
 
         super(*args)

lib/gitlab/github_import/importer/issue_importer.rb

@@ -52,7 +52,6 @@ module Gitlab
             project_id: project.id,
             description: description,
             milestone_id: milestone_finder.id_for(issue),
-            state: issue.state,
             state_id: ::Issue.available_states[issue.state],
             created_at: issue.created_at,
             updated_at: issue.updated_at

lib/gitlab/github_import/importer/pull_request_importer.rb

@@ -54,7 +54,6 @@ module Gitlab
             target_project_id: project.id,
             source_branch: pull_request.formatted_source_branch,
             target_branch: pull_request.target_branch,
-            state: pull_request.state,
             state_id: ::MergeRequest.available_states[pull_request.state],
             milestone_id: milestone_finder.id_for(pull_request),
             author_id: author_id,

lib/gitlab/hook_data/merge_request_builder.rb

@@ -23,7 +23,7 @@ module Gitlab
           milestone_id
           source_branch
           source_project_id
-          state
+          state_id
           target_branch
           target_project_id
           time_estimate
@@ -53,7 +53,8 @@ module Gitlab
           human_total_time_spent: merge_request.human_total_time_spent,
           human_time_estimate: merge_request.human_time_estimate,
           assignee_ids: merge_request.assignee_ids,
-          assignee_id: merge_request.assignee_ids.first # This key is deprecated
+          assignee_id: merge_request.assignee_ids.first, # This key is deprecated
+          state: merge_request.state # This key is deprecated
         }
 
         merge_request.attributes.with_indifferent_access.slice(*self.class.safe_hook_attributes)

spec/lib/gitlab/github_import/importer/issue_importer_spec.rb

@@ -99,7 +99,6 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach
               project_id: project.id,
               description: 'This is my issue',
               milestone_id: milestone.id,
-              state: :opened,
               state_id: 1,
               created_at: created_at,
               updated_at: updated_at
@@ -129,7 +128,6 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach
               project_id: project.id,
               description: "*Created by: alice*\n\nThis is my issue",
               milestone_id: milestone.id,
-              state: :opened,
               state_id: 1,
               created_at: created_at,
               updated_at: updated_at

spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb

@@ -94,7 +94,6 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
               target_project_id: project.id,
               source_branch: 'github/fork/alice/feature',
               target_branch: 'master',
-              state: :merged,
               state_id: 3,
               milestone_id: milestone.id,
               author_id: user.id,
@@ -140,7 +139,6 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
               target_project_id: project.id,
               source_branch: 'github/fork/alice/feature',
               target_branch: 'master',
-              state: :merged,
               state_id: 3,
               milestone_id: milestone.id,
               author_id: project.creator_id,
@@ -187,7 +185,6 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
               target_project_id: project.id,
               source_branch: 'master-42',
               target_branch: 'master',
-              state: :merged,
               state_id: 3,
               milestone_id: milestone.id,
               author_id: user.id,

spec/lib/gitlab/import/merge_request_helpers_spec.rb

@@ -18,7 +18,7 @@ describe Gitlab::Import::MergeRequestHelpers, type: :helper do
         target_project_id: project.id,
         source_branch: 'master-42',
         target_branch: 'master',
-        state: :merged,
+        state_id: 3,
         author_id: user.id,
         assignee_id: user.id
       }

spec/lib/gitlab/markdown_cache/active_record/extension_spec.rb

@@ -73,11 +73,11 @@ describe Gitlab::MarkdownCache::ActiveRecord::Extension do
     let(:thing) { klass.new(title: markdown, title_html: html, cached_markdown_version: cache_version) }
 
     before do
-      thing.state = 'closed'
+      thing.state_id = 2
       thing.save
     end
 
-    it { expect(thing.state).to eq('closed') }
+    it { expect(thing.state_id).to eq(2) }
     it { expect(thing.title).to eq(markdown) }
     it { expect(thing.title_html).to eq(html) }
     it { expect(thing.cached_markdown_version).to eq(cache_version) }

spec/models/concerns/issuable_states_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-# This spec checks if state_id column of issues and merge requests
-# are being synced on every save.
-# It can be removed in the next release. Check https://gitlab.com/gitlab-org/gitlab-foss/issues/51789  for more information.
-describe IssuableStates do
-  [Issue, MergeRequest].each do |klass|
-    it "syncs state_id column when #{klass.model_name.human} gets created" do
-      klass.available_states.each do |state, state_id|
-        issuable = build(klass.model_name.param_key, state: state.to_s)
-
-        issuable.save!
-
-        expect(issuable.state_id).to eq(state_id)
-      end
-    end
-
-    it "syncs state_id column when #{klass.model_name.human} gets updated" do
-      klass.available_states.each do |state, state_id|
-        issuable = create(klass.model_name.param_key, state: state.to_s)
-
-        issuable.update(state: state)
-
-        expect(issuable.state_id).to eq(state_id)
-      end
-    end
-  end
-end