Commit 561a30dc authored by Felipe Artur's avatar Felipe Artur Committed by Andreas Brandl

Ignore state column for issues/merge_requests

Third step of the plan of moving state:string column
to state_id:integer.
The state_column will be removed by a migration in the next release.
See the complete plan at https://gitlab.com/gitlab-org/gitlab/issues/33814
parent df3ad023
...@@ -23,7 +23,6 @@ module Issuable ...@@ -23,7 +23,6 @@ module Issuable
include Sortable include Sortable
include CreatedAtFilterable include CreatedAtFilterable
include UpdatedAtFilterable include UpdatedAtFilterable
include IssuableStates
include ClosedAtFilterable include ClosedAtFilterable
include VersionedDescription include VersionedDescription
......
# 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
...@@ -66,7 +66,10 @@ class Issue < ApplicationRecord ...@@ -66,7 +66,10 @@ class Issue < ApplicationRecord
scope :public_only, -> { where(confidential: false) } scope :public_only, -> { where(confidential: false) }
scope :confidential_only, -> { where(confidential: true) } 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_commit :expire_etag_cache
after_save :ensure_metrics, unless: :imported? after_save :ensure_metrics, unless: :imported?
......
...@@ -228,6 +228,9 @@ class MergeRequest < ApplicationRecord ...@@ -228,6 +228,9 @@ class MergeRequest < ApplicationRecord
with_state(:opened).where(auto_merge_enabled: true) with_state(:opened).where(auto_merge_enabled: true)
end end
# Only remove after 2019-12-22 and with %12.7
self.ignored_columns += %i[state]
after_save :keep_around_commit after_save :keep_around_commit
alias_attribute :project, :target_project alias_attribute :project, :target_project
......
# frozen_string_literal: true # frozen_string_literal: true
class AnalyticsMergeRequestEntity < AnalyticsIssueEntity class AnalyticsMergeRequestEntity < AnalyticsIssueEntity
expose :state expose :state do |object|
MergeRequest.available_states.key(object[:state_id])
end
expose :url do |object| expose :url do |object|
url_to(:namespace_project_merge_request, object) url_to(:namespace_project_merge_request, object)
......
...@@ -397,7 +397,7 @@ class IssuableBaseService < BaseService ...@@ -397,7 +397,7 @@ class IssuableBaseService < BaseService
end end
def update_project_counter_caches?(issuable) def update_project_counter_caches?(issuable)
issuable.state_changed? issuable.state_id_changed?
end end
def parent def parent
......
...@@ -93,7 +93,7 @@ tables: ...@@ -93,7 +93,7 @@ tables:
- updated_at - updated_at
- description - description
- milestone_id - milestone_id
- state - state_id
- updated_by_id - updated_by_id
- weight - weight
- due_date - due_date
...@@ -174,7 +174,7 @@ tables: ...@@ -174,7 +174,7 @@ tables:
- created_at - created_at
- updated_at - updated_at
- milestone_id - milestone_id
- state - state_id
- merge_status - merge_status
- target_project_id - target_project_id
- updated_by_id - updated_by_id
......
...@@ -20,11 +20,15 @@ module Epics ...@@ -20,11 +20,15 @@ module Epics
end end
def opened_issues def opened_issues
issues_count.fetch('opened', 0) opened_state_id = Issue.available_states[:opened]
issues_count.fetch(opened_state_id, 0)
end end
def closed_issues def closed_issues
issues_count.fetch('closed', 0) closed_state_id = Issue.available_states[:closed]
issues_count.fetch(closed_state_id, 0)
end end
private private
......
...@@ -108,7 +108,7 @@ module Gitlab ...@@ -108,7 +108,7 @@ module Gitlab
end end
def sync_code_owners_with_approvers 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 Gitlab::GitalyClient.allow_n_plus_1_calls do
gl_merge_request = ::MergeRequest.find(id) gl_merge_request = ::MergeRequest.find(id)
......
...@@ -79,6 +79,7 @@ describe MergeRequest, :elastic do ...@@ -79,6 +79,7 @@ describe MergeRequest, :elastic do
'target_project_id', 'target_project_id',
'author_id' 'author_id'
).merge({ ).merge({
'state' => merge_request.state,
'type' => merge_request.es_type, 'type' => merge_request.es_type,
'join_field' => { 'join_field' => {
'name' => merge_request.es_type, 'name' => merge_request.es_type,
......
...@@ -21,7 +21,7 @@ module Gitlab ...@@ -21,7 +21,7 @@ module Gitlab
finder_class: MergeRequestsFinder, finder_class: MergeRequestsFinder,
serializer_class: AnalyticsMergeRequestSerializer, serializer_class: AnalyticsMergeRequestSerializer,
includes_for_query: { target_project: [:namespace], author: [] }, 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 }.freeze
......
...@@ -200,7 +200,6 @@ module Gitlab ...@@ -200,7 +200,6 @@ module Gitlab
target_project_id: project.id, target_project_id: project.id,
target_branch: Gitlab::Git.ref_name(pull_request.target_branch_name), target_branch: Gitlab::Git.ref_name(pull_request.target_branch_name),
target_branch_sha: pull_request.target_branch_sha, target_branch_sha: pull_request.target_branch_sha,
state: pull_request.state,
state_id: MergeRequest.available_states[pull_request.state], state_id: MergeRequest.available_states[pull_request.state],
author_id: author_id, author_id: author_id,
assignee_id: nil, assignee_id: nil,
......
...@@ -10,7 +10,7 @@ module Gitlab ...@@ -10,7 +10,7 @@ module Gitlab
mr_table[:iid], mr_table[:iid],
mr_table[:id], mr_table[:id],
mr_table[:created_at], mr_table[:created_at],
mr_table[:state], mr_table[:state_id],
mr_table[:author_id]] mr_table[:author_id]]
@order = mr_table[:created_at] @order = mr_table[:created_at]
......
...@@ -10,7 +10,7 @@ module Gitlab ...@@ -10,7 +10,7 @@ module Gitlab
mr_table[:iid], mr_table[:iid],
mr_table[:id], mr_table[:id],
mr_table[:created_at], mr_table[:created_at],
mr_table[:state], mr_table[:state_id],
mr_table[:author_id]] mr_table[:author_id]]
super(*args) super(*args)
......
...@@ -52,7 +52,6 @@ module Gitlab ...@@ -52,7 +52,6 @@ module Gitlab
project_id: project.id, project_id: project.id,
description: description, description: description,
milestone_id: milestone_finder.id_for(issue), milestone_id: milestone_finder.id_for(issue),
state: issue.state,
state_id: ::Issue.available_states[issue.state], state_id: ::Issue.available_states[issue.state],
created_at: issue.created_at, created_at: issue.created_at,
updated_at: issue.updated_at updated_at: issue.updated_at
......
...@@ -54,7 +54,6 @@ module Gitlab ...@@ -54,7 +54,6 @@ module Gitlab
target_project_id: project.id, target_project_id: project.id,
source_branch: pull_request.formatted_source_branch, source_branch: pull_request.formatted_source_branch,
target_branch: pull_request.target_branch, target_branch: pull_request.target_branch,
state: pull_request.state,
state_id: ::MergeRequest.available_states[pull_request.state], state_id: ::MergeRequest.available_states[pull_request.state],
milestone_id: milestone_finder.id_for(pull_request), milestone_id: milestone_finder.id_for(pull_request),
author_id: author_id, author_id: author_id,
......
...@@ -23,7 +23,7 @@ module Gitlab ...@@ -23,7 +23,7 @@ module Gitlab
milestone_id milestone_id
source_branch source_branch
source_project_id source_project_id
state state_id
target_branch target_branch
target_project_id target_project_id
time_estimate time_estimate
...@@ -53,7 +53,8 @@ module Gitlab ...@@ -53,7 +53,8 @@ module Gitlab
human_total_time_spent: merge_request.human_total_time_spent, human_total_time_spent: merge_request.human_total_time_spent,
human_time_estimate: merge_request.human_time_estimate, human_time_estimate: merge_request.human_time_estimate,
assignee_ids: merge_request.assignee_ids, 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) merge_request.attributes.with_indifferent_access.slice(*self.class.safe_hook_attributes)
......
...@@ -99,7 +99,6 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach ...@@ -99,7 +99,6 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach
project_id: project.id, project_id: project.id,
description: 'This is my issue', description: 'This is my issue',
milestone_id: milestone.id, milestone_id: milestone.id,
state: :opened,
state_id: 1, state_id: 1,
created_at: created_at, created_at: created_at,
updated_at: updated_at updated_at: updated_at
...@@ -129,7 +128,6 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach ...@@ -129,7 +128,6 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach
project_id: project.id, project_id: project.id,
description: "*Created by: alice*\n\nThis is my issue", description: "*Created by: alice*\n\nThis is my issue",
milestone_id: milestone.id, milestone_id: milestone.id,
state: :opened,
state_id: 1, state_id: 1,
created_at: created_at, created_at: created_at,
updated_at: updated_at updated_at: updated_at
......
...@@ -94,7 +94,6 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi ...@@ -94,7 +94,6 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
target_project_id: project.id, target_project_id: project.id,
source_branch: 'github/fork/alice/feature', source_branch: 'github/fork/alice/feature',
target_branch: 'master', target_branch: 'master',
state: :merged,
state_id: 3, state_id: 3,
milestone_id: milestone.id, milestone_id: milestone.id,
author_id: user.id, author_id: user.id,
...@@ -140,7 +139,6 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi ...@@ -140,7 +139,6 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
target_project_id: project.id, target_project_id: project.id,
source_branch: 'github/fork/alice/feature', source_branch: 'github/fork/alice/feature',
target_branch: 'master', target_branch: 'master',
state: :merged,
state_id: 3, state_id: 3,
milestone_id: milestone.id, milestone_id: milestone.id,
author_id: project.creator_id, author_id: project.creator_id,
...@@ -187,7 +185,6 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi ...@@ -187,7 +185,6 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
target_project_id: project.id, target_project_id: project.id,
source_branch: 'master-42', source_branch: 'master-42',
target_branch: 'master', target_branch: 'master',
state: :merged,
state_id: 3, state_id: 3,
milestone_id: milestone.id, milestone_id: milestone.id,
author_id: user.id, author_id: user.id,
......
...@@ -18,7 +18,7 @@ describe Gitlab::Import::MergeRequestHelpers, type: :helper do ...@@ -18,7 +18,7 @@ describe Gitlab::Import::MergeRequestHelpers, type: :helper do
target_project_id: project.id, target_project_id: project.id,
source_branch: 'master-42', source_branch: 'master-42',
target_branch: 'master', target_branch: 'master',
state: :merged, state_id: 3,
author_id: user.id, author_id: user.id,
assignee_id: user.id assignee_id: user.id
} }
......
...@@ -73,11 +73,11 @@ describe Gitlab::MarkdownCache::ActiveRecord::Extension do ...@@ -73,11 +73,11 @@ describe Gitlab::MarkdownCache::ActiveRecord::Extension do
let(:thing) { klass.new(title: markdown, title_html: html, cached_markdown_version: cache_version) } let(:thing) { klass.new(title: markdown, title_html: html, cached_markdown_version: cache_version) }
before do before do
thing.state = 'closed' thing.state_id = 2
thing.save thing.save
end 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).to eq(markdown) }
it { expect(thing.title_html).to eq(html) } it { expect(thing.title_html).to eq(html) }
it { expect(thing.cached_markdown_version).to eq(cache_version) } it { expect(thing.cached_markdown_version).to eq(cache_version) }
......
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment