Merge branch 'project-topics-data-migration-2' into 'master'

Migrate project topics from `acts_as_taggable` gem to separate table

See merge request gitlab-org/gitlab!67574

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)
+      .includes(:forked_from_project, :topics, :topics_acts_as_taggable)
 
     @groups = []
 

app/finders/projects_finder.rb

@@ -180,7 +180,14 @@ class ProjectsFinder < UnionFinder
   # rubocop: enable CodeReuse/ActiveRecord
 
   def by_topics(items)
-    params[:topic].present? ? items.tagged_with(params[:topic]) : 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|
+      items = items.with_topic(topic)
+    end
+
+    items
   end
 
   def by_search(items)

app/graphql/types/project_type.rb

@@ -406,6 +406,10 @@ module Types
       object.topic_list
     end
 
+    def topics
+      object.topic_list
+    end
+
     private
 
     def project

app/models/project.rb

@@ -103,6 +103,8 @@ class Project < ApplicationRecord
 
   after_save :create_import_state, if: ->(project) { project.import? && project.import_state.nil? }
 
+  after_save :save_topics
+
   after_create -> { create_or_load_association(:project_feature) }
 
   after_create -> { create_or_load_association(:ci_cd_settings) }
@@ -127,12 +129,31 @@ 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
   attr_accessor :skip_disk_validation
+  attr_writer :topic_list
 
   alias_attribute :title, :name
 
@@ -623,6 +644,19 @@ class Project < ApplicationRecord
     joins(:service_desk_setting).where('service_desk_settings.project_key' => key)
   end
 
+  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)
+  end
+
   enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 }
 
   chronic_duration_attr :build_timeout_human_readable, :build_timeout,
@@ -638,7 +672,7 @@ class Project < ApplicationRecord
   mount_uploader :bfg_object_map, AttachmentUploader
 
   def self.with_api_entity_associations
-    preload(:project_feature, :route, :topics, :group, :timelogs, namespace: [:route, :owner])
+    preload(:project_feature, :route, :topics, :topics_acts_as_taggable, :group, :timelogs, namespace: [:route, :owner])
   end
 
   def self.with_web_entity_associations
@@ -2673,8 +2707,32 @@ class Project < ApplicationRecord
     ci_cd_settings.group_runners_enabled?
   end
 
+  def topic_list
+    self.topics.map(&:name)
+  end
+
   private
 
+  def save_topics
+    return if @topic_list.nil?
+
+    @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
+      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
+  end
+
   def find_integration(integrations, name)
     integrations.find { _1.to_param == name }
   end

app/models/projects/project_topic.rb

+# frozen_string_literal: true
+
+module Projects
+  class ProjectTopic < ApplicationRecord
+    belongs_to :project
+    belongs_to :topic
+  end
+end

app/models/projects/topic.rb

+# frozen_string_literal: true
+
+module Projects
+  class Topic < ApplicationRecord
+    validates :name, presence: true, uniqueness: true, length: { maximum: 255 }
+
+    has_many :project_topics, class_name: 'Projects::ProjectTopic'
+    has_many :projects, through: :project_topics
+  end
+end

db/migrate/20210729081351_create_topics.rb

+# frozen_string_literal: true
+
+class CreateTopics < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  def change
+    create_table_with_constraints :topics do |t|
+      t.text :name, null: false
+      t.text_limit :name, 255
+
+      t.index :name, unique: true
+
+      t.timestamps_with_timezone
+    end
+  end
+end

db/migrate/20210729081739_create_project_topics.rb

+# frozen_string_literal: true
+
+class CreateProjectTopics < ActiveRecord::Migration[6.1]
+  def change
+    create_table :project_topics do |t|
+      t.bigint :project_id, null: false
+      t.bigint :topic_id, null: false
+
+      t.index :project_id
+      t.index :topic_id
+      t.index [:project_id, :topic_id], unique: true
+
+      t.timestamps_with_timezone
+    end
+  end
+end

db/migrate/20210729125641_add_foreign_key_to_project_on_project_topic.rb

+# frozen_string_literal: true
+
+class AddForeignKeyToProjectOnProjectTopic < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_foreign_key :project_topics, :projects, column: :project_id, on_delete: :cascade
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :project_topics, column: :project_id
+    end
+  end
+end

db/migrate/20210729125659_add_foreign_key_to_topic_on_project_topic.rb

+# frozen_string_literal: true
+
+class AddForeignKeyToTopicOnProjectTopic < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_foreign_key :project_topics, :topics, column: :topic_id, on_delete: :cascade
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :project_topics, column: :topic_id
+    end
+  end
+end

db/post_migrate/20210730104800_schedule_extract_project_topics_into_separate_table.rb

+# frozen_string_literal: true
+
+class ScheduleExtractProjectTopicsIntoSeparateTable < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  BATCH_SIZE = 1_000
+  DELAY_INTERVAL = 2.minutes
+  MIGRATION = 'ExtractProjectTopicsIntoSeparateTable'
+  INDEX_NAME = 'tmp_index_taggings_on_id_where_taggable_type_project'
+  INDEX_CONDITION = "taggable_type = 'Project'"
+
+  disable_ddl_transaction!
+
+  class Tagging < ActiveRecord::Base
+    include ::EachBatch
+
+    self.table_name = 'taggings'
+  end
+
+  def up
+    # this index is used in 20210730104800_schedule_extract_project_topics_into_separate_table
+    add_concurrent_index :taggings, :id, where: INDEX_CONDITION, name: INDEX_NAME
+
+    queue_background_migration_jobs_by_range_at_intervals(
+      Tagging.where(taggable_type: 'Project'),
+      MIGRATION,
+      DELAY_INTERVAL,
+      batch_size: BATCH_SIZE,
+      track_jobs: true
+    )
+  end
+
+  def down
+    remove_concurrent_index_by_name :taggings, INDEX_NAME
+  end
+end

db/schema_migrations/20210729081351

+17463867a8c14981386256dc90169fb879e1921d65eccca53eae576d49fba49d
\ No newline at end of file

db/schema_migrations/20210729081739

+af7963d27bda6ef85fb5b5a06ecf1de14f21829eecdaf13e763aa9a6ffc2e83c
\ No newline at end of file

db/schema_migrations/20210729125641

+b7bc495d010e0640b1145ca55f47696047fd4360d2dfc9a3da7941ab62840132
\ No newline at end of file

db/schema_migrations/20210729125659

+5826e87b2ce13d4951e9b8e774c87c29c6e0a0954a85d60ec68155f2c5cf3ccc
\ No newline at end of file

db/schema_migrations/20210730104800

+7764c058665015707aff6e25ccbf60d4a329c67c16106b2ef523862ef82298b7
\ No newline at end of file

db/structure.sql

@@ -17379,6 +17379,23 @@ CREATE SEQUENCE project_statistics_id_seq
 
 ALTER SEQUENCE project_statistics_id_seq OWNED BY project_statistics.id;
 
+CREATE TABLE project_topics (
+    id bigint NOT NULL,
+    project_id bigint NOT NULL,
+    topic_id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL
+);
+
+CREATE SEQUENCE project_topics_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE project_topics_id_seq OWNED BY project_topics.id;
+
 CREATE TABLE project_tracing_settings (
     id bigint NOT NULL,
     created_at timestamp with time zone NOT NULL,
@@ -18835,6 +18852,23 @@ CREATE SEQUENCE token_with_ivs_id_seq
 
 ALTER SEQUENCE token_with_ivs_id_seq OWNED BY token_with_ivs.id;
 
+CREATE TABLE topics (
+    id bigint NOT NULL,
+    name text NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    CONSTRAINT check_7a90d4c757 CHECK ((char_length(name) <= 255))
+);
+
+CREATE SEQUENCE topics_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE topics_id_seq OWNED BY topics.id;
+
 CREATE TABLE trending_projects (
     id integer NOT NULL,
     project_id integer NOT NULL
@@ -20777,6 +20811,8 @@ ALTER TABLE ONLY project_security_settings ALTER COLUMN project_id SET DEFAULT n
 
 ALTER TABLE ONLY project_statistics ALTER COLUMN id SET DEFAULT nextval('project_statistics_id_seq'::regclass);
 
+ALTER TABLE ONLY project_topics ALTER COLUMN id SET DEFAULT nextval('project_topics_id_seq'::regclass);
+
 ALTER TABLE ONLY project_tracing_settings ALTER COLUMN id SET DEFAULT nextval('project_tracing_settings_id_seq'::regclass);
 
 ALTER TABLE ONLY projects ALTER COLUMN id SET DEFAULT nextval('projects_id_seq'::regclass);
@@ -20905,6 +20941,8 @@ ALTER TABLE ONLY todos ALTER COLUMN id SET DEFAULT nextval('todos_id_seq'::regcl
 
 ALTER TABLE ONLY token_with_ivs ALTER COLUMN id SET DEFAULT nextval('token_with_ivs_id_seq'::regclass);
 
+ALTER TABLE ONLY topics ALTER COLUMN id SET DEFAULT nextval('topics_id_seq'::regclass);
+
 ALTER TABLE ONLY trending_projects ALTER COLUMN id SET DEFAULT nextval('trending_projects_id_seq'::regclass);
 
 ALTER TABLE ONLY u2f_registrations ALTER COLUMN id SET DEFAULT nextval('u2f_registrations_id_seq'::regclass);
@@ -22412,6 +22450,9 @@ ALTER TABLE ONLY project_settings
 ALTER TABLE ONLY project_statistics
     ADD CONSTRAINT project_statistics_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY project_topics
+    ADD CONSTRAINT project_topics_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY project_tracing_settings
     ADD CONSTRAINT project_tracing_settings_pkey PRIMARY KEY (id);
 
@@ -22625,6 +22666,9 @@ ALTER TABLE ONLY todos
 ALTER TABLE ONLY token_with_ivs
     ADD CONSTRAINT token_with_ivs_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY topics
+    ADD CONSTRAINT topics_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY trending_projects
     ADD CONSTRAINT trending_projects_pkey PRIMARY KEY (id);
 
@@ -25023,6 +25067,12 @@ CREATE INDEX index_project_statistics_on_storage_size_and_project_id ON project_
 
 CREATE INDEX index_project_statistics_on_wiki_size_and_project_id ON project_statistics USING btree (wiki_size, project_id);
 
+CREATE INDEX index_project_topics_on_project_id ON project_topics USING btree (project_id);
+
+CREATE UNIQUE INDEX index_project_topics_on_project_id_and_topic_id ON project_topics USING btree (project_id, topic_id);
+
+CREATE INDEX index_project_topics_on_topic_id ON project_topics USING btree (topic_id);
+
 CREATE UNIQUE INDEX index_project_tracing_settings_on_project_id ON project_tracing_settings USING btree (project_id);
 
 CREATE INDEX index_projects_aimed_for_deletion ON projects USING btree (marked_for_deletion_at) WHERE ((marked_for_deletion_at IS NOT NULL) AND (pending_delete = false));
@@ -25527,6 +25577,8 @@ CREATE UNIQUE INDEX index_token_with_ivs_on_hashed_plaintext_token ON token_with
 
 CREATE UNIQUE INDEX index_token_with_ivs_on_hashed_token ON token_with_ivs USING btree (hashed_token);
 
+CREATE UNIQUE INDEX index_topics_on_name ON topics USING btree (name);
+
 CREATE UNIQUE INDEX index_trending_projects_on_project_id ON trending_projects USING btree (project_id);
 
 CREATE INDEX index_u2f_registrations_on_key_handle ON u2f_registrations USING btree (key_handle);
@@ -25861,6 +25913,8 @@ 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);
@@ -26354,6 +26408,9 @@ ALTER TABLE ONLY ci_group_variables
 ALTER TABLE ONLY namespaces
     ADD CONSTRAINT fk_3448c97865 FOREIGN KEY (push_rule_id) REFERENCES push_rules(id) ON DELETE SET NULL;
 
+ALTER TABLE ONLY project_topics
+    ADD CONSTRAINT fk_34af9ab07a FOREIGN KEY (topic_id) REFERENCES topics(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY in_product_marketing_emails
     ADD CONSTRAINT fk_35c9101b63 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE;
 
@@ -26873,6 +26930,9 @@ ALTER TABLE ONLY label_links
 ALTER TABLE ONLY project_group_links
     ADD CONSTRAINT fk_daa8cee94c FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY project_topics
+    ADD CONSTRAINT fk_db13576296 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY security_scans
     ADD CONSTRAINT fk_dbc89265b9 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 

ee/lib/gitlab/elastic/search_results.rb

@@ -28,7 +28,7 @@ module Gitlab
 
         case scope
         when 'projects'
-          eager_load(projects, page, per_page, preload_method, [:route, :namespace, :topics])
+          eager_load(projects, page, per_page, preload_method, [:route, :namespace, :topics, :topics_acts_as_taggable])
         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

@@ -43,7 +43,7 @@ module API
         # 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)
+                         .preload(:import_state, :topics, :topics_acts_as_taggable)
                          .preload(:auto_devops)
                          .preload(namespace: [:route, :owner])
       end

lib/api/entities/project.rb

@@ -140,7 +140,7 @@ module API
                                 .preload(project_group_links: { group: :route },
                                          fork_network: :root_project,
                                          fork_network_member: :forked_from_project,
-                                         forked_from_project: [:route, :topics, :group, :project_feature, namespace: [:route, :owner]])
+                                         forked_from_project: [:route, :topics, :topics_acts_as_taggable, :group, :project_feature, namespace: [:route, :owner]])
       end
       # rubocop: enable CodeReuse/ActiveRecord
 

lib/gitlab/background_migration/extract_project_topics_into_separate_table.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # The class to extract the project topics into a separate `topics` table
+    class ExtractProjectTopicsIntoSeparateTable
+      # Temporary AR table for tags
+      class Tag < ActiveRecord::Base
+        self.table_name = 'tags'
+      end
+
+      # Temporary AR table for taggings
+      class Tagging < ActiveRecord::Base
+        self.table_name = 'taggings'
+        belongs_to :tag
+      end
+
+      # Temporary AR table for topics
+      class Topic < ActiveRecord::Base
+        self.table_name = 'topics'
+      end
+
+      # Temporary AR table for project topics
+      class ProjectTopic < ActiveRecord::Base
+        self.table_name = 'project_topics'
+        belongs_to :topic
+      end
+
+      def perform(start_id, stop_id)
+        Tagging.includes(:tag).where(taggable_type: 'Project', id: start_id..stop_id).each do |tagging|
+          topic = Topic.find_or_create_by(name: tagging.tag.name)
+          project_topic = ProjectTopic.find_or_create_by(project_id: tagging.taggable_id, topic: topic)
+
+          tagging.delete if project_topic.persisted?
+        end
+
+        mark_job_as_succeeded(start_id, stop_id)
+      end
+
+      private
+
+      def mark_job_as_succeeded(*arguments)
+        Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded(
+          self.class.name.demodulize,
+          arguments
+        )
+      end
+    end
+  end
+end

spec/factories/project_topics.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :project_topic, class: 'Projects::ProjectTopic' do
+    association :project, factory: :project
+    association :topic, factory: :topic
+  end
+end

spec/factories/topics.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :topic, class: 'Projects::Topic' do
+    name { generate(:name) }
+  end
+end

spec/finders/projects_finder_spec.rb

@@ -135,6 +135,7 @@ RSpec.describe ProjectsFinder do
 
       describe 'filter by tags (deprecated)' do
         before do
+          public_project.reload
           public_project.topic_list = 'foo'
           public_project.save!
         end
@@ -146,6 +147,7 @@ RSpec.describe ProjectsFinder do
 
       describe 'filter by topics' do
         before do
+          public_project.reload
           public_project.topic_list = 'foo, bar'
           public_project.save!
         end

spec/lib/gitlab/background_migration/extract_project_topics_into_separate_table_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::ExtractProjectTopicsIntoSeparateTable, schema: 20210730104800 do
+  it 'correctly extracts project topics into separate table' do
+    namespaces = table(:namespaces)
+    projects = table(:projects)
+    taggings = table(:taggings)
+    tags = table(:tags)
+    project_topics = table(:project_topics)
+    topics = table(:topics)
+
+    namespace = namespaces.create!(name: 'foo', path: 'foo')
+    project = projects.create!(namespace_id: namespace.id)
+    tag_1 = tags.create!(name: 'Topic1')
+    tag_2 = tags.create!(name: 'Topic2')
+    tag_3 = tags.create!(name: 'Topic3')
+    topic_3 = topics.create!(name: 'Topic3')
+    tagging_1 = taggings.create!(taggable_type: 'Project', taggable_id: project.id, context: 'topics', tag_id: tag_1.id)
+    tagging_2 = taggings.create!(taggable_type: 'Project', taggable_id: project.id, context: 'topics', tag_id: tag_2.id)
+    other_tagging = taggings.create!(taggable_type: 'Other', taggable_id: project.id, context: 'topics', tag_id: tag_1.id)
+    tagging_3 = taggings.create!(taggable_type: 'Project', taggable_id: project.id, context: 'topics', tag_id: tag_3.id)
+
+    subject.perform(tagging_1.id, tagging_3.id)
+
+    # Tagging records
+    expect { tagging_1.reload }.to raise_error(ActiveRecord::RecordNotFound)
+    expect { tagging_2.reload }.to raise_error(ActiveRecord::RecordNotFound)
+    expect { other_tagging.reload }.not_to raise_error(ActiveRecord::RecordNotFound)
+    expect { tagging_3.reload }.to raise_error(ActiveRecord::RecordNotFound)
+
+    # Topic records
+    topic_1 = topics.find_by(name: 'Topic1')
+    topic_2 = topics.find_by(name: 'Topic2')
+    expect(topics.all).to contain_exactly(topic_1, topic_2, topic_3)
+
+    # ProjectTopic records
+    expect(project_topics.all.map(&:topic_id)).to contain_exactly(topic_1.id, topic_2.id, topic_3.id)
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -354,6 +354,8 @@ project:
 - taggings
 - base_tags
 - topic_taggings
+- topics_acts_as_taggable
+- project_topics
 - topics
 - chat_services
 - cluster

spec/models/project_spec.rb

@@ -7135,15 +7135,96 @@ RSpec.describe Project, factory_default: :keep do
   end
 
   describe 'topics' do
-    let_it_be(:project) { create(:project, topic_list: 'topic1, topic2, topic3') }
+    let_it_be(:project) { create(:project, name: 'topic-project', topic_list: 'topic1, topic2, topic3') }
 
     it 'topic_list returns correct string array' do
-      expect(project.topic_list).to match_array(%w[topic1 topic2 topic3])
+      expect(project.topic_list).to eq(%w[topic1 topic2 topic3])
     end
 
-    it 'topics returns correct tag records' do
-      expect(project.topics.first.class.name).to eq('ActsAsTaggableOn::Tag')
-      expect(project.topics.map(&:name)).to match_array(%w[topic1 topic2 topic3])
+    it 'topics returns correct topic records' do
+      expect(project.topics.first.class.name).to eq('Projects::Topic')
+      expect(project.topics.map(&:name)).to eq(%w[topic1 topic2 topic3])
+    end
+
+    context 'topic_list=' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:topic_list, :expected_result) do
+        ['topicA', 'topicB']              | %w[topicA topicB] # rubocop:disable Style/WordArray, Lint/BinaryOperatorWithIdenticalOperands
+        ['topicB', 'topicA']              | %w[topicB topicA] # rubocop:disable Style/WordArray, Lint/BinaryOperatorWithIdenticalOperands
+        ['   topicC  ', ' topicD    ']    | %w[topicC topicD]
+        ['topicE', 'topicF', 'topicE']    | %w[topicE topicF] # rubocop:disable Style/WordArray
+        ['topicE  ', 'topicF', ' topicE'] | %w[topicE topicF]
+        'topicA, topicB'                  | %w[topicA topicB]
+        'topicB, topicA'                  | %w[topicB topicA]
+        '   topicC  , topicD    '         | %w[topicC topicD]
+        'topicE, topicF, topicE'          | %w[topicE topicF]
+        'topicE  , topicF,  topicE'       | %w[topicE topicF]
+      end
+
+      with_them do
+        it 'set topics' do
+          project.topic_list = topic_list
+          project.save!
+
+          expect(project.topics.map(&:name)).to eq(expected_result)
+        end
+      end
+
+      it 'set topics if only the order is changed' do
+        project.topic_list = 'topicA, topicB'
+        project.save!
+
+        expect(project.reload.topics.map(&:name)).to eq(%w[topicA topicB])
+
+        project.topic_list = 'topicB, topicA'
+        project.save!
+
+        expect(project.reload.topics.map(&:name)).to eq(%w[topicB topicA])
+      end
+
+      it 'does not persist topics before project is saved' do
+        project.topic_list = 'topicA, topicB'
+
+        expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3])
+      end
+
+      it 'does not update topics if project is not valid' do
+        project.name = nil
+        project.topic_list = 'topicA, topicB'
+
+        expect(project.save).to be_falsy
+        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
 

spec/models/projects/project_topic_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::ProjectTopic do
+  let_it_be(:project_topic, reload: true) { create(:project_topic) }
+
+  subject { project_topic }
+
+  it { expect(subject).to be_valid }
+
+  describe 'associations' do
+    it { is_expected.to belong_to(:project) }
+    it { is_expected.to belong_to(:topic) }
+  end
+end

spec/models/projects/topic_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::Topic do
+  let_it_be(:topic, reload: true) { create(:topic) }
+
+  subject { topic }
+
+  it { expect(subject).to be_valid }
+
+  describe 'associations' do
+    it { is_expected.to have_many(:project_topics) }
+    it { is_expected.to have_many(:projects) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:name) }
+    it { is_expected.to validate_uniqueness_of(:name) }
+    it { is_expected.to validate_length_of(:name).is_at_most(255) }
+  end
+end

spec/services/projects/create_service_spec.rb

@@ -86,7 +86,7 @@ RSpec.describe Projects::CreateService, '#execute' do
     subject(:project) { create_project(user, opts) }
 
     context "with 'topics' parameter" do
-      let(:opts) { { topics: 'topics' } }
+      let(:opts) { { name: 'topic-project', topics: 'topics' } }
 
       it 'keeps them as specified' do
         expect(project.topic_list).to eq(%w[topics])
@@ -94,7 +94,7 @@ RSpec.describe Projects::CreateService, '#execute' do
     end
 
     context "with 'topic_list' parameter" do
-      let(:opts) { { topic_list: 'topic_list' } }
+      let(:opts) { { name: 'topic-project', topic_list: 'topic_list' } }
 
       it 'keeps them as specified' do
         expect(project.topic_list).to eq(%w[topic_list])
@@ -102,7 +102,7 @@ RSpec.describe Projects::CreateService, '#execute' do
     end
 
     context "with 'tag_list' parameter (deprecated)" do
-      let(:opts) { { tag_list: 'tag_list' } }
+      let(:opts) { { name: 'topic-project', tag_list: 'tag_list' } }
 
       it 'keeps them as specified' do
         expect(project.topic_list).to eq(%w[tag_list])