diff --git a/plugins/chat/app/models/chat_channel.rb b/plugins/chat/app/models/chat_channel.rb index 1a0e47d7f3e..a8e4fdfec7f 100644 --- a/plugins/chat/app/models/chat_channel.rb +++ b/plugins/chat/app/models/chat_channel.rb @@ -144,6 +144,6 @@ end # # index_chat_channels_on_chatable_id (chatable_id) # index_chat_channels_on_chatable_id_and_chatable_type (chatable_id,chatable_type) -# index_chat_channels_on_slug (slug) +# index_chat_channels_on_slug (slug) UNIQUE # index_chat_channels_on_status (status) # diff --git a/plugins/chat/db/migrate/20221201024458_make_channel_slugs_unique_with_index.rb b/plugins/chat/db/migrate/20221201024458_make_channel_slugs_unique_with_index.rb new file mode 100644 index 00000000000..7f1c751d8cf --- /dev/null +++ b/plugins/chat/db/migrate/20221201024458_make_channel_slugs_unique_with_index.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +class MakeChannelSlugsUniqueWithIndex < ActiveRecord::Migration[7.0] + def up + DB.exec("CREATE TEMPORARY TABLE tmp_chat_channel_slugs_conflicts(id int, slug text)") + + channels_with_conflicting_slugs = DB.query(<<~SQL) + SELECT chat_channels.id, subquery.slug + FROM ( + SELECT slug, count(*) + FROM chat_channels + GROUP BY slug + HAVING count(*) > 1 + ) subquery + INNER JOIN chat_channels ON chat_channels.slug = subquery.slug + ORDER BY slug, created_at ASC + SQL + + current_slug = nil + slugs_to_update = [] + channels_with_conflicting_slugs.each do |channel| + if current_slug != channel.slug + current_slug = channel.slug + + # Continue since we want to keep the slug for the first + # matching channel and just update subsequent channels. + next + end + + # Deduplicate slugs with the channel IDs, we can always improve + # slugs later on. + slugs_to_update << [channel.id, "#{channel.slug}-#{channel.id}"] + end + + values_to_insert = + slugs_to_update.map do |channel_pair| + "(#{channel_pair[0]}, '#{PG::Connection.escape_string(channel_pair[1])}')" + end + + if values_to_insert.any? + DB.exec( + "INSERT INTO tmp_chat_channel_slugs_conflicts + VALUES #{values_to_insert.join(",\n")}", + ) + + DB.exec(<<~SQL) + UPDATE chat_channels cc + SET slug = tmp.slug + FROM tmp_chat_channel_slugs_conflicts tmp + WHERE cc.id = tmp.id + SQL + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/plugins/chat/db/migrate/20221201035918_add_slug_unique_index_for_chat_channels.rb b/plugins/chat/db/migrate/20221201035918_add_slug_unique_index_for_chat_channels.rb new file mode 100644 index 00000000000..1f738ef8de2 --- /dev/null +++ b/plugins/chat/db/migrate/20221201035918_add_slug_unique_index_for_chat_channels.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddSlugUniqueIndexForChatChannels < ActiveRecord::Migration[7.0] + def change + remove_index :chat_channels, :slug + add_index :chat_channels, :slug, unique: true + end +end diff --git a/plugins/chat/db/post_migrate/20221201032830_drop_tmp_chat_slug_tables.rb b/plugins/chat/db/post_migrate/20221201032830_drop_tmp_chat_slug_tables.rb new file mode 100644 index 00000000000..2ac238d59d9 --- /dev/null +++ b/plugins/chat/db/post_migrate/20221201032830_drop_tmp_chat_slug_tables.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class DropTmpChatSlugTables < ActiveRecord::Migration[7.0] + def up + DB.exec("DROP TABLE IF EXISTS tmp_chat_channel_slugs") + DB.exec("DROP TABLE IF EXISTS tmp_chat_channel_slugs_conflicts") + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/plugins/chat/spec/models/category_channel_spec.rb b/plugins/chat/spec/models/category_channel_spec.rb index 89c950f5f7a..17d9139f6e7 100644 --- a/plugins/chat/spec/models/category_channel_spec.rb +++ b/plugins/chat/spec/models/category_channel_spec.rb @@ -88,9 +88,7 @@ RSpec.describe CategoryChannel do subject(:channel) { Fabricate(:category_channel) } context "when slug is not provided" do - before do - channel.slug = nil - end + before { channel.slug = nil } it "uses channel name when present" do channel.name = "Some Cool Stuff" @@ -129,12 +127,20 @@ RSpec.describe CategoryChannel do end context "when there is a duplicate slug" do - before { Fabricate(:category_channel, slug: "awesome-channel") } + fab!(:awesome_channel) { Fabricate(:category_channel, slug: "awesome-channel") } it "adds a validation error" do channel.slug = "awesome-channel" channel.validate - expect(channel.errors.full_messages.first).to include(I18n.t("chat.category_channel.errors.is_already_in_use")) + expect(channel.errors.full_messages.first).to include( + I18n.t("chat.category_channel.errors.is_already_in_use"), + ) + end + + it "does not allow setting the slug conflict directly in SQL" do + expect { + DB.exec("UPDATE chat_channels SET slug = 'awesome-channel' WHERE id = #{channel.id}") + }.to raise_error(PG::UniqueViolation) end end @@ -144,7 +150,9 @@ RSpec.describe CategoryChannel do it "fails if slug contains non-ascii characters" do channel.slug = "sem-acentuação" channel.validate - expect(channel.errors.full_messages.first).to match(/#{I18n.t("chat.category_channel.errors.slug_contains_non_ascii_chars")}/) + expect(channel.errors.full_messages.first).to match( + /#{I18n.t("chat.category_channel.errors.slug_contains_non_ascii_chars")}/, + ) end end end