Commit c7908c24 authored by Jonas Wälter's avatar Jonas Wälter Committed by Thong Kuah

Eliminate `acts_as_taggable` gem for project topics

parent 8154de4f
...@@ -36,7 +36,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController ...@@ -36,7 +36,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def starred def starred
@projects = load_projects(params.merge(starred: true)) @projects = load_projects(params.merge(starred: true))
.includes(:forked_from_project, :topics, :topics_acts_as_taggable) .includes(:forked_from_project, :topics)
@groups = [] @groups = []
......
...@@ -182,8 +182,8 @@ class ProjectsFinder < UnionFinder ...@@ -182,8 +182,8 @@ class ProjectsFinder < UnionFinder
def by_topics(items) def by_topics(items)
return items unless params[:topic].present? return items unless params[:topic].present?
topics = params[:topic].instance_of?(String) ? params[:topic].strip.split(/\s*,\s*/) : params[:topic] topics = params[:topic].instance_of?(String) ? params[:topic].split(',') : params[:topic]
topics.each do |topic| topics.map(&:strip).uniq.reject(&:empty?).each do |topic|
items = items.with_topic(topic) items = items.with_topic(topic)
end end
......
...@@ -128,26 +128,9 @@ class Project < ApplicationRecord ...@@ -128,26 +128,9 @@ class Project < ApplicationRecord
after_initialize :use_hashed_storage after_initialize :use_hashed_storage
after_create :check_repository_absence! after_create :check_repository_absence!
# Required during the `ActsAsTaggableOn::Tag -> Topic` migration
# TODO: remove 'acts_as_ordered_taggable_on' and ':topics_acts_as_taggable' in the further process of the migration
# https://gitlab.com/gitlab-org/gitlab/-/issues/335946
acts_as_ordered_taggable_on :topics
has_many :topics_acts_as_taggable, -> { order("#{ActsAsTaggableOn::Tagging.table_name}.id") },
class_name: 'ActsAsTaggableOn::Tag',
through: :topic_taggings,
source: :tag
has_many :project_topics, -> { order(:id) }, class_name: 'Projects::ProjectTopic' has_many :project_topics, -> { order(:id) }, class_name: 'Projects::ProjectTopic'
has_many :topics, through: :project_topics, class_name: 'Projects::Topic' has_many :topics, through: :project_topics, class_name: 'Projects::Topic'
# Required during the `ActsAsTaggableOn::Tag -> Topic` migration
# TODO: remove 'topics' in the further process of the migration
# https://gitlab.com/gitlab-org/gitlab/-/issues/335946
alias_method :topics_new, :topics
def topics
self.topics_acts_as_taggable + self.topics_new
end
attr_accessor :old_path_with_namespace attr_accessor :old_path_with_namespace
attr_accessor :template_name attr_accessor :template_name
attr_writer :pipeline_status attr_writer :pipeline_status
...@@ -652,15 +635,8 @@ class Project < ApplicationRecord ...@@ -652,15 +635,8 @@ class Project < ApplicationRecord
scope :with_topic, ->(topic_name) do scope :with_topic, ->(topic_name) do
topic = Projects::Topic.find_by_name(topic_name) topic = Projects::Topic.find_by_name(topic_name)
acts_as_taggable_on_topic = ActsAsTaggableOn::Tag.find_by_name(topic_name)
return none unless topic || acts_as_taggable_on_topic topic ? where(id: topic.project_topics.select(:project_id)) : none
relations = []
relations << where(id: topic.project_topics.select(:project_id)) if topic
relations << where(id: acts_as_taggable_on_topic.taggings.select(:taggable_id)) if acts_as_taggable_on_topic
Project.from_union(relations)
end end
enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 } enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 }
...@@ -678,7 +654,7 @@ class Project < ApplicationRecord ...@@ -678,7 +654,7 @@ class Project < ApplicationRecord
mount_uploader :bfg_object_map, AttachmentUploader mount_uploader :bfg_object_map, AttachmentUploader
def self.with_api_entity_associations def self.with_api_entity_associations
preload(:project_feature, :route, :topics, :topics_acts_as_taggable, :group, :timelogs, namespace: [:route, :owner]) preload(:project_feature, :route, :topics, :group, :timelogs, namespace: [:route, :owner])
end end
def self.with_web_entity_associations def self.with_web_entity_associations
...@@ -2734,15 +2710,9 @@ class Project < ApplicationRecord ...@@ -2734,15 +2710,9 @@ class Project < ApplicationRecord
@topic_list = @topic_list.split(',') if @topic_list.instance_of?(String) @topic_list = @topic_list.split(',') if @topic_list.instance_of?(String)
@topic_list = @topic_list.map(&:strip).uniq.reject(&:empty?) @topic_list = @topic_list.map(&:strip).uniq.reject(&:empty?)
if @topic_list != self.topic_list || self.topics_acts_as_taggable.any? if @topic_list != self.topic_list
self.topics_new.delete_all self.topics.delete_all
self.topics = @topic_list.map { |topic| Projects::Topic.find_or_create_by(name: topic) } self.topics = @topic_list.map { |topic| Projects::Topic.find_or_create_by(name: topic) }
# Remove old topics (ActsAsTaggableOn::Tag)
# Required during the `ActsAsTaggableOn::Tag -> Topic` migration
# TODO: remove in the further process of the migration
# https://gitlab.com/gitlab-org/gitlab/-/issues/335946
self.topic_taggings.clear
end end
@topic_list = nil @topic_list = nil
......
...@@ -24,7 +24,7 @@ module Search ...@@ -24,7 +24,7 @@ module Search
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def projects def projects
@projects ||= ProjectsFinder.new(params: { non_archived: true }, current_user: current_user).execute.preload(:topics, :taggings) @projects ||= ProjectsFinder.new(params: { non_archived: true }, current_user: current_user).execute.preload(:topics, :project_topics)
end end
def allowed_scopes def allowed_scopes
......
# frozen_string_literal: true
class RemoveTemporaryIndexForProjectTopicsOnTaggings < Gitlab::Database::Migration[1.0]
MIGRATION = 'ExtractProjectTopicsIntoSeparateTable'
INDEX_NAME = 'tmp_index_taggings_on_id_where_taggable_type_project'
INDEX_CONDITION = "taggable_type = 'Project'"
disable_ddl_transaction!
def up
# Ensure that no background jobs of 20210730104800_schedule_extract_project_topics_into_separate_table remain
finalize_background_migration MIGRATION
# this index was used in 20210730104800_schedule_extract_project_topics_into_separate_table
remove_concurrent_index_by_name :taggings, INDEX_NAME
end
def down
add_concurrent_index :taggings, :id, where: INDEX_CONDITION, name: INDEX_NAME # rubocop:disable Migration/PreventIndexCreation
end
end
a0ba9fb9e2f7f738926a2273f9ff644c43acb999f4d27adf192e5006582a2a0e
\ No newline at end of file
...@@ -26954,8 +26954,6 @@ CREATE INDEX tmp_index_namespaces_empty_traversal_ids_with_root_namespaces ON na ...@@ -26954,8 +26954,6 @@ CREATE INDEX tmp_index_namespaces_empty_traversal_ids_with_root_namespaces ON na
CREATE INDEX tmp_index_on_vulnerabilities_non_dismissed ON vulnerabilities USING btree (id) WHERE (state <> 2); CREATE INDEX tmp_index_on_vulnerabilities_non_dismissed ON vulnerabilities USING btree (id) WHERE (state <> 2);
CREATE INDEX tmp_index_taggings_on_id_where_taggable_type_project ON taggings USING btree (id) WHERE ((taggable_type)::text = 'Project'::text);
CREATE UNIQUE INDEX uniq_pkgs_deb_grp_architectures_on_distribution_id_and_name ON packages_debian_group_architectures USING btree (distribution_id, name); CREATE UNIQUE INDEX uniq_pkgs_deb_grp_architectures_on_distribution_id_and_name ON packages_debian_group_architectures USING btree (distribution_id, name);
CREATE UNIQUE INDEX uniq_pkgs_deb_grp_components_on_distribution_id_and_name ON packages_debian_group_components USING btree (distribution_id, name); CREATE UNIQUE INDEX uniq_pkgs_deb_grp_components_on_distribution_id_and_name ON packages_debian_group_components USING btree (distribution_id, name);
...@@ -30,7 +30,7 @@ module Gitlab ...@@ -30,7 +30,7 @@ module Gitlab
case scope case scope
when 'projects' when 'projects'
eager_load(projects, page, per_page, preload_method, [:route, :namespace, :topics, :topics_acts_as_taggable]) eager_load(projects, page, per_page, preload_method, [:route, :namespace, :topics])
when 'issues' when 'issues'
eager_load(issues, page, per_page, preload_method, project: [:route, :namespace], labels: [], timelogs: [], assignees: []) eager_load(issues, page, per_page, preload_method, project: [:route, :namespace], labels: [], timelogs: [], assignees: [])
when 'merge_requests' when 'merge_requests'
......
...@@ -39,11 +39,11 @@ module API ...@@ -39,11 +39,11 @@ module API
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def self.preload_relation(projects_relation, options = {}) def self.preload_relation(projects_relation, options = {})
# Preloading topics, should be done with using only `:topics`, # Preloading topics, should be done with using only `:topics`,
# as `:topics` are defined as: `has_many :topics, through: :taggings` # as `:topics` are defined as: `has_many :topics, through: :project_topics`
# N+1 is solved then by using `subject.topics.map(&:name)` # N+1 is solved then by using `subject.topics.map(&:name)`
# MR describing the solution: https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/20555 # MR describing the solution: https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/20555
projects_relation.preload(:project_feature, :route) projects_relation.preload(:project_feature, :route)
.preload(:import_state, :topics, :topics_acts_as_taggable) .preload(:import_state, :topics)
.preload(:auto_devops) .preload(:auto_devops)
.preload(namespace: [:route, :owner]) .preload(namespace: [:route, :owner])
end end
......
...@@ -132,7 +132,7 @@ module API ...@@ -132,7 +132,7 @@ module API
def self.preload_relation(projects_relation, options = {}) def self.preload_relation(projects_relation, options = {})
# Preloading topics, should be done with using only `:topics`, # Preloading topics, should be done with using only `:topics`,
# as `:topics` are defined as: `has_many :topics, through: :taggings` # as `:topics` are defined as: `has_many :topics, through: :project_topics`
# N+1 is solved then by using `subject.topics.map(&:name)` # N+1 is solved then by using `subject.topics.map(&:name)`
# MR describing the solution: https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/20555 # MR describing the solution: https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/20555
super(projects_relation).preload(group: :namespace_settings) super(projects_relation).preload(group: :namespace_settings)
...@@ -144,7 +144,7 @@ module API ...@@ -144,7 +144,7 @@ module API
.preload(project_group_links: { group: :route }, .preload(project_group_links: { group: :route },
fork_network: :root_project, fork_network: :root_project,
fork_network_member: :forked_from_project, fork_network_member: :forked_from_project,
forked_from_project: [:route, :topics, :topics_acts_as_taggable, :group, :project_feature, namespace: [:route, :owner]]) forked_from_project: [:route, :topics, :group, :project_feature, namespace: [:route, :owner]])
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -354,10 +354,7 @@ container_repositories: ...@@ -354,10 +354,7 @@ container_repositories:
- name - name
project: project:
- external_status_checks - external_status_checks
- taggings
- base_tags - base_tags
- topic_taggings
- topics_acts_as_taggable
- project_topics - project_topics
- topics - topics
- chat_services - chat_services
......
...@@ -7239,35 +7239,6 @@ RSpec.describe Project, factory_default: :keep do ...@@ -7239,35 +7239,6 @@ RSpec.describe Project, factory_default: :keep do
expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3]) expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3])
end end
end end
context 'during ExtractProjectTopicsIntoSeparateTable migration' do
before do
topic_a = ActsAsTaggableOn::Tag.find_or_create_by!(name: 'topicA')
topic_b = ActsAsTaggableOn::Tag.find_or_create_by!(name: 'topicB')
project.reload.topics_acts_as_taggable = [topic_a, topic_b]
project.save!
project.reload
end
it 'topic_list returns correct string array' do
expect(project.topic_list).to eq(%w[topicA topicB topic1 topic2 topic3])
end
it 'topics returns correct topic records' do
expect(project.topics.map(&:class)).to eq([ActsAsTaggableOn::Tag, ActsAsTaggableOn::Tag, Projects::Topic, Projects::Topic, Projects::Topic])
expect(project.topics.map(&:name)).to eq(%w[topicA topicB topic1 topic2 topic3])
end
it 'topic_list= sets new topics and removes old topics' do
project.topic_list = 'new-topic1, new-topic2'
project.save!
project.reload
expect(project.topics.map(&:class)).to eq([Projects::Topic, Projects::Topic])
expect(project.topics.map(&:name)).to eq(%w[new-topic1 new-topic2])
end
end
end end
shared_examples 'all_runners' do shared_examples 'all_runners' do
......
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