Merge branch 'project-topics-migration-cleanup' into 'master'

Eliminate `acts_as_taggable` gem for project topics

See merge request gitlab-org/gitlab!68925

app/controllers/dashboard/projects_controller.rb

@@ -36,7 +36,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
   # rubocop: disable CodeReuse/ActiveRecord
   def starred
     @projects = load_projects(params.merge(starred: true))
-      .includes(:forked_from_project, :topics, :topics_acts_as_taggable)
+      .includes(:forked_from_project, :topics)
 
     @groups = []
 

app/finders/projects_finder.rb

@@ -182,8 +182,8 @@ class ProjectsFinder < UnionFinder
   def by_topics(items)
     return items unless params[:topic].present?
 
-    topics = params[:topic].instance_of?(String) ? params[:topic].strip.split(/\s*,\s*/) : params[:topic]
-    topics.each do |topic|
+    topics = params[:topic].instance_of?(String) ? params[:topic].split(',') : params[:topic]
+    topics.map(&:strip).uniq.reject(&:empty?).each do |topic|
       items = items.with_topic(topic)
     end
 

app/models/project.rb

@@ -128,26 +128,9 @@ class Project < ApplicationRecord
   after_initialize :use_hashed_storage
   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 :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 :template_name
   attr_writer :pipeline_status
@@ -653,15 +636,8 @@ class Project < ApplicationRecord
 
   scope :with_topic, ->(topic_name) do
     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
-
-    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)
+    topic ? where(id: topic.project_topics.select(:project_id)) : none
   end
 
   enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 }
@@ -679,7 +655,7 @@ class Project < ApplicationRecord
   mount_uploader :bfg_object_map, AttachmentUploader
 
   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
 
   def self.with_web_entity_associations
@@ -2735,15 +2711,9 @@ class Project < ApplicationRecord
     @topic_list = @topic_list.split(',') if @topic_list.instance_of?(String)
     @topic_list = @topic_list.map(&:strip).uniq.reject(&:empty?)
 
-    if @topic_list != self.topic_list || self.topics_acts_as_taggable.any?
-      self.topics_new.delete_all
+    if @topic_list != self.topic_list
+      self.topics.delete_all
       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
 
     @topic_list = nil

app/services/search/global_service.rb

@@ -24,7 +24,7 @@ module Search
 
     # rubocop: disable CodeReuse/ActiveRecord
     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
 
     def allowed_scopes

db/migrate/20210917134321_remove_temporary_index_for_project_topics_on_taggings.rb

+# 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

db/schema_migrations/20210917134321

+a0ba9fb9e2f7f738926a2273f9ff644c43acb999f4d27adf192e5006582a2a0e
\ No newline at end of file

db/structure.sql

@@ -26916,8 +26916,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_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_components_on_distribution_id_and_name ON packages_debian_group_components USING btree (distribution_id, name);

ee/lib/gitlab/elastic/search_results.rb

@@ -30,7 +30,7 @@ module Gitlab
 
         case scope
         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'
           eager_load(issues, page, per_page, preload_method, project: [:route, :namespace], labels: [], timelogs: [], assignees: [])
         when 'merge_requests'

lib/api/entities/basic_project_details.rb

@@ -39,11 +39,11 @@ module API
       # rubocop: disable CodeReuse/ActiveRecord
       def self.preload_relation(projects_relation, options = {})
         # 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)`
         # MR describing the solution: https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/20555
         projects_relation.preload(:project_feature, :route)
-                         .preload(:import_state, :topics, :topics_acts_as_taggable)
+                         .preload(:import_state, :topics)
                          .preload(:auto_devops)
                          .preload(namespace: [:route, :owner])
       end

lib/api/entities/project.rb

@@ -132,7 +132,7 @@ module API
 
       def self.preload_relation(projects_relation, options = {})
         # 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)`
         # MR describing the solution: https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/20555
         super(projects_relation).preload(group: :namespace_settings)
@@ -144,7 +144,7 @@ module API
                                 .preload(project_group_links: { group: :route },
                                          fork_network: :root_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
       # rubocop: enable CodeReuse/ActiveRecord
 

spec/lib/gitlab/import_export/all_models.yml

@@ -355,10 +355,7 @@ container_repositories:
 - name
 project:
 - external_status_checks
-- taggings
 - base_tags
-- topic_taggings
-- topics_acts_as_taggable
 - project_topics
 - topics
 - chat_services

spec/models/project_spec.rb

@@ -7240,35 +7240,6 @@ RSpec.describe Project, factory_default: :keep do
         expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3])
       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
 
   shared_examples 'all_runners' do