diff --git a/app/models/category.rb b/app/models/category.rb index 0efbb5f3ed8..7e7bd947b02 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -374,7 +374,7 @@ class Category < ActiveRecord::Base elsif SiteSetting.slug_generation_method == 'ascii' && !CGI.unescape(self.slug).ascii_only? errors.add(:slug, I18n.t("category.errors.slug_contains_non_ascii_chars")) elsif duplicate_slug? - errors.add(:slug, 'is already in use') + errors.add(:slug, I18n.t("category.errors.is_already_in_use")) end else # auto slug diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 73b0d3fe785..07b182b805e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -697,6 +697,7 @@ en: disallowed_topic_tags: "This topic has tags not allowed by this category: '%{tags}'" disallowed_tags_generic: "This topic has disallowed tags." slug_contains_non_ascii_chars: "contains non-ascii characters" + is_already_in_use: "is already in use" cannot_delete: uncategorized: "This category is special. It is intended as a holding area for topics that have no category; it cannot be deleted." has_subcategories: "Can't delete this category because it has sub-categories." diff --git a/plugins/chat/app/controllers/chat_controller.rb b/plugins/chat/app/controllers/chat_controller.rb index e272188e720..3f78f15b8ef 100644 --- a/plugins/chat/app/controllers/chat_controller.rb +++ b/plugins/chat/app/controllers/chat_controller.rb @@ -352,6 +352,7 @@ class Chat::ChatController < Chat::ChatBaseController message: "chat.invitation_notification", chat_channel_id: @chat_channel.id, chat_channel_title: @chat_channel.title(user), + chat_channel_slug: @chat_channel.slug, invited_by_username: current_user.username, } data[:chat_message_id] = params[:chat_message_id] if params[:chat_message_id] diff --git a/plugins/chat/app/jobs/regular/chat_notify_mentioned.rb b/plugins/chat/app/jobs/regular/chat_notify_mentioned.rb index 523d3259f2f..d6fa48e3320 100644 --- a/plugins/chat/app/jobs/regular/chat_notify_mentioned.rb +++ b/plugins/chat/app/jobs/regular/chat_notify_mentioned.rb @@ -39,9 +39,10 @@ module Jobs is_direct_message_channel: @chat_channel.direct_message_channel?, } - data[:chat_channel_title] = @chat_channel.title( - membership.user, - ) unless @is_direct_message_channel + if !@is_direct_message_channel + data[:chat_channel_title] = @chat_channel.title(membership.user) + data[:chat_channel_slug] = @chat_channel.slug + end return data if identifier_type == :direct_mentions @@ -64,8 +65,7 @@ module Jobs username: @creator.username, tag: Chat::ChatNotifier.push_notification_tag(:mention, @chat_channel.id), excerpt: @chat_message.push_notification_excerpt, - post_url: - "/chat/channel/#{@chat_channel.id}/#{@chat_channel.title(membership.user)}?messageId=#{@chat_message.id}", + post_url: "#{@chat_channel.relative_url}?messageId=#{@chat_message.id}", } translation_prefix = diff --git a/plugins/chat/app/jobs/regular/chat_notify_watching.rb b/plugins/chat/app/jobs/regular/chat_notify_watching.rb index c2c2cd8681e..e9b8805e88d 100644 --- a/plugins/chat/app/jobs/regular/chat_notify_watching.rb +++ b/plugins/chat/app/jobs/regular/chat_notify_watching.rb @@ -62,7 +62,7 @@ module Jobs payload = { username: @creator.username, notification_type: Notification.types[:chat_message], - post_url: "/chat/channel/#{@chat_channel.id}/#{@chat_channel.title(user)}", + post_url: @chat_channel.relative_url, translated_title: I18n.t(translation_key, translation_args), tag: Chat::ChatNotifier.push_notification_tag(:message, @chat_channel.id), excerpt: @chat_message.push_notification_excerpt, diff --git a/plugins/chat/app/models/category_channel.rb b/plugins/chat/app/models/category_channel.rb index 2a2a32246b4..e95c3d5cff8 100644 --- a/plugins/chat/app/models/category_channel.rb +++ b/plugins/chat/app/models/category_channel.rb @@ -21,7 +21,23 @@ class CategoryChannel < ChatChannel name.presence || category.name end - def slug - title.truncate(100).parameterize + def generate_auto_slug + return if self.slug.present? + self.slug = Slug.for(self.title.strip, "") + self.slug = "" if duplicate_slug? + end + + def ensure_slug_ok + # if we don't unescape it first we strip the % from the encoded version + slug = SiteSetting.slug_generation_method == "encoded" ? CGI.unescape(self.slug) : self.slug + self.slug = Slug.for(slug, "", method: :encoded) + + if self.slug.blank? + errors.add(:slug, :invalid) + elsif SiteSetting.slug_generation_method == "ascii" && !CGI.unescape(self.slug).ascii_only? + errors.add(:slug, I18n.t("chat.category_channel.errors.slug_contains_non_ascii_chars")) + elsif duplicate_slug? + errors.add(:slug, I18n.t("chat.category_channel.errors.is_already_in_use")) + end end end diff --git a/plugins/chat/app/models/chat_channel.rb b/plugins/chat/app/models/chat_channel.rb index bbcb53e0ae7..560ae2d322b 100644 --- a/plugins/chat/app/models/chat_channel.rb +++ b/plugins/chat/app/models/chat_channel.rb @@ -21,6 +21,8 @@ class ChatChannel < ActiveRecord::Base }, presence: true, allow_nil: true + validate :ensure_slug_ok + before_validation :generate_auto_slug scope :public_channels, -> { @@ -74,7 +76,11 @@ class ChatChannel < ActiveRecord::Base end def url - "#{Discourse.base_url}/chat/channel/#{self.id}/-" + "#{Discourse.base_url}#{relative_url}" + end + + def relative_url + "/chat/channel/#{self.id}/#{self.slug || "-"}" end def public_channel_title @@ -109,6 +115,10 @@ class ChatChannel < ActiveRecord::Base ChatPublisher.publish_channel_status(self) end + + def duplicate_slug? + ChatChannel.where(slug: self.slug).where.not(id: self.id).any? + end end # == Schema Information @@ -132,6 +142,7 @@ end # auto_join_users :boolean default(FALSE), not null # user_count_stale :boolean default(FALSE), not null # slug :string +# type :string # # Indexes # diff --git a/plugins/chat/app/models/direct_message_channel.rb b/plugins/chat/app/models/direct_message_channel.rb index 69be58827a6..9d116643d7e 100644 --- a/plugins/chat/app/models/direct_message_channel.rb +++ b/plugins/chat/app/models/direct_message_channel.rb @@ -18,4 +18,12 @@ class DirectMessageChannel < ChatChannel def title(user) direct_message.chat_channel_title_for_user(self, user) end + + def ensure_slug_ok + true + end + + def generate_auto_slug + self.slug = nil + end end diff --git a/plugins/chat/app/serializers/chat_channel_serializer.rb b/plugins/chat/app/serializers/chat_channel_serializer.rb index 8bb24577697..6a194047f98 100644 --- a/plugins/chat/app/serializers/chat_channel_serializer.rb +++ b/plugins/chat/app/serializers/chat_channel_serializer.rb @@ -9,6 +9,7 @@ class ChatChannelSerializer < ApplicationSerializer :chatable_url, :description, :title, + :slug, :last_message_sent_at, :status, :archive_failed, diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-sidebar.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-sidebar.js index a523b2cea6b..a190e99a4b9 100644 --- a/plugins/chat/assets/javascripts/discourse/initializers/chat-sidebar.js +++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-sidebar.js @@ -58,7 +58,7 @@ export default { } get name() { - return dasherize(slugifyChannel(this.title)); + return dasherize(slugifyChannel(this.channel)); } get classNames() { @@ -72,7 +72,7 @@ export default { } get models() { - return [this.channel.id, slugifyChannel(this.title)]; + return [this.channel.id, slugifyChannel(this.channel)]; } get text() { @@ -240,7 +240,7 @@ export default { } get name() { - return slugifyChannel(this.title); + return slugifyChannel(this.channel); } get classNames() { @@ -254,7 +254,7 @@ export default { } get models() { - return [this.channel.id, slugifyChannel(this.title)]; + return [this.channel.id, slugifyChannel(this.channel)]; } get title() { diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-user-menu.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-user-menu.js index 192203a506a..62758b9b0c5 100644 --- a/plugins/chat/assets/javascripts/discourse/initializers/chat-user-menu.js +++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-user-menu.js @@ -20,11 +20,15 @@ export default { (NotificationItemBase) => { return class extends NotificationItemBase { get linkHref() { - const title = this.notification.data.chat_channel_title - ? slugifyChannel(this.notification.data.chat_channel_title) - : "-"; - - return `/chat/channel/${this.notification.data.chat_channel_id}/${title}?messageId=${this.notification.data.chat_message_id}`; + const slug = slugifyChannel({ + title: this.notification.data.chat_channel_title, + slug: this.notification.data.chat_channel_slug, + }); + return `/chat/channel/${ + this.notification.data.chat_channel_id + }/${slug || "-"}?messageId=${ + this.notification.data.chat_message_id + }`; } get linkTitle() { @@ -53,11 +57,15 @@ export default { (NotificationItemBase) => { return class extends NotificationItemBase { get linkHref() { - const title = this.notification.data.chat_channel_title - ? slugifyChannel(this.notification.data.chat_channel_title) - : "-"; - - return `/chat/channel/${this.notification.data.chat_channel_id}/${title}?messageId=${this.notification.data.chat_message_id}`; + const slug = slugifyChannel({ + title: this.notification.data.chat_channel_title, + slug: this.notification.data.chat_channel_slug, + }); + return `/chat/channel/${ + this.notification.data.chat_channel_id + }/${slug || "-"}?messageId=${ + this.notification.data.chat_message_id + }`; } get linkTitle() { diff --git a/plugins/chat/assets/javascripts/discourse/lib/slugify-channel.js b/plugins/chat/assets/javascripts/discourse/lib/slugify-channel.js index 885f9d11dea..f7a2734879e 100644 --- a/plugins/chat/assets/javascripts/discourse/lib/slugify-channel.js +++ b/plugins/chat/assets/javascripts/discourse/lib/slugify-channel.js @@ -1,8 +1,19 @@ import { slugify } from "discourse/lib/utilities"; -export default function slugifyChannel(title) { - const slug = slugify(title); - return ( - slug.length ? slug : title.trim().toLowerCase().replace(/\s|_+/g, "-") +export default function slugifyChannel(channel) { + if (channel.slug) { + return channel.slug; + } + const slug = slugify(channel.escapedTitle || channel.title); + const resolvedSlug = ( + slug.length + ? slug + : channel.title.trim().toLowerCase().replace(/\s|_+/g, "-") ).slice(0, 100); + + if (!resolvedSlug) { + return "-"; + } + + return resolvedSlug; } diff --git a/plugins/chat/assets/javascripts/discourse/routes/chat-channel.js b/plugins/chat/assets/javascripts/discourse/routes/chat-channel.js index 97e42858d19..999870ac8a5 100644 --- a/plugins/chat/assets/javascripts/discourse/routes/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/routes/chat-channel.js @@ -42,7 +42,7 @@ export default class ChatChannelRoute extends DiscourseRoute { this.chat.setActiveChannel(model?.chatChannel); const queryParams = this.paramsFor(this.routeName); - const slug = slugifyChannel(model.chatChannel.title); + const slug = slugifyChannel(model.chatChannel); if (queryParams?.channelTitle !== slug) { this.replaceWith("chat.channel.index", model.chatChannel.id, slug); } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat.js b/plugins/chat/assets/javascripts/discourse/services/chat.js index af4925489b7..237e8328cf4 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat.js @@ -503,7 +503,7 @@ export default class Chat extends Service { return this.router.transitionTo( "chat.channel", response.id, - slugifyChannel(response.title), + slugifyChannel(response), { queryParams } ); }); @@ -535,7 +535,7 @@ export default class Chat extends Service { return this.router.transitionTo( "chat.channel", channel.id, - slugifyChannel(channel.title), + slugifyChannel(channel), { queryParams } ); } else { diff --git a/plugins/chat/assets/javascripts/discourse/templates/chat-channel-info.hbs b/plugins/chat/assets/javascripts/discourse/templates/chat-channel-info.hbs index 01f6cc80854..fc488396a5f 100644 --- a/plugins/chat/assets/javascripts/discourse/templates/chat-channel-info.hbs +++ b/plugins/chat/assets/javascripts/discourse/templates/chat-channel-info.hbs @@ -12,7 +12,7 @@ @route="chat.channel" @models={{array this.model.chatChannel.id - (slugify-channel this.model.chatChannel.title) + (slugify-channel this.model.chatChannel) }} class="channel-info__back-btn" > @@ -40,7 +40,7 @@ @route={{concat "chat.channel.info." tab}} @models={{array this.model.chatChannel.id - (slugify-channel this.model.chatChannel.title) + (slugify-channel this.model.chatChannel) }} class="chat-tabs-list__link" > diff --git a/plugins/chat/assets/javascripts/discourse/templates/components/chat-channel-card.hbs b/plugins/chat/assets/javascripts/discourse/templates/components/chat-channel-card.hbs index 04be19f906b..411dc94ea38 100644 --- a/plugins/chat/assets/javascripts/discourse/templates/components/chat-channel-card.hbs +++ b/plugins/chat/assets/javascripts/discourse/templates/components/chat-channel-card.hbs @@ -10,7 +10,7 @@
@@ -27,7 +27,7 @@ @route="chat.channel.info.settings" @models={{array this.channel.id - (slugify-channel this.channel.title) + (slugify-channel this.channel) }} class="chat-channel-card__tag -muted" tabindex="-1" @@ -38,7 +38,7 @@ @@ -83,7 +83,7 @@ {{#if (gt this.channel.membershipsCount 0)}} diff --git a/plugins/chat/assets/javascripts/discourse/templates/components/chat-live-pane.hbs b/plugins/chat/assets/javascripts/discourse/templates/components/chat-live-pane.hbs index f7c62611509..43ad2f6197a 100644 --- a/plugins/chat/assets/javascripts/discourse/templates/components/chat-live-pane.hbs +++ b/plugins/chat/assets/javascripts/discourse/templates/components/chat-live-pane.hbs @@ -10,7 +10,7 @@
{{/if}} - + diff --git a/plugins/chat/assets/javascripts/discourse/templates/components/topic-chat-float.hbs b/plugins/chat/assets/javascripts/discourse/templates/components/topic-chat-float.hbs index 58ec9d9a0cb..10af503050a 100644 --- a/plugins/chat/assets/javascripts/discourse/templates/components/topic-chat-float.hbs +++ b/plugins/chat/assets/javascripts/discourse/templates/components/topic-chat-float.hbs @@ -29,7 +29,7 @@ {{#if this.chat.activeChannel}} {{#if this.expanded}} - +
diff --git a/plugins/chat/assets/javascripts/discourse/widgets/chat-invitation-notification-item.js b/plugins/chat/assets/javascripts/discourse/widgets/chat-invitation-notification-item.js index 31d71168fe4..f9367adec4a 100644 --- a/plugins/chat/assets/javascripts/discourse/widgets/chat-invitation-notification-item.js +++ b/plugins/chat/assets/javascripts/discourse/widgets/chat-invitation-notification-item.js @@ -34,9 +34,12 @@ createWidgetFrom(DefaultNotificationItem, "chat-invitation-notification-item", { }, url(data) { - const title = data.chat_channel_title - ? slugifyChannel(data.chat_channel_title) - : "-"; - return `/chat/channel/${data.chat_channel_id}/${title}?messageId=${data.chat_message_id}`; + const slug = slugifyChannel({ + title: data.chat_channel_title, + slug: data.chat_channel_slug, + }); + return `/chat/channel/${data.chat_channel_id}/${slug || "-"}?messageId=${ + data.chat_message_id + }`; }, }); diff --git a/plugins/chat/assets/javascripts/discourse/widgets/chat-mention-notification-item.js b/plugins/chat/assets/javascripts/discourse/widgets/chat-mention-notification-item.js index ea8c59cfde3..7d194879dc2 100644 --- a/plugins/chat/assets/javascripts/discourse/widgets/chat-mention-notification-item.js +++ b/plugins/chat/assets/javascripts/discourse/widgets/chat-mention-notification-item.js @@ -44,10 +44,13 @@ const chatNotificationItem = { }, url(data) { - const title = data.chat_channel_title - ? slugifyChannel(data.chat_channel_title) - : "-"; - return `/chat/channel/${data.chat_channel_id}/${title}?messageId=${data.chat_message_id}`; + const slug = slugifyChannel({ + title: data.chat_channel_title, + slug: data.chat_channel_slug, + }); + return `/chat/channel/${data.chat_channel_id}/${slug || "-"}?messageId=${ + data.chat_message_id + }`; }, }; diff --git a/plugins/chat/config/locales/server.en.yml b/plugins/chat/config/locales/server.en.yml index 72d651f2ab2..ea2349f2b7f 100644 --- a/plugins/chat/config/locales/server.en.yml +++ b/plugins/chat/config/locales/server.en.yml @@ -107,6 +107,11 @@ en: multi_user: "%{users}" multi_user_truncated: "%{users} and %{leftover} others" + category_channel: + errors: + slug_contains_non_ascii_chars: "contains non-ascii characters" + is_already_in_use: "is already in use" + bookmarkable: notification_title: "message in %{channel_name}" diff --git a/plugins/chat/db/post_migrate/20221104054957_backfill_channel_slugs.rb b/plugins/chat/db/post_migrate/20221104054957_backfill_channel_slugs.rb new file mode 100644 index 00000000000..0efcdec486f --- /dev/null +++ b/plugins/chat/db/post_migrate/20221104054957_backfill_channel_slugs.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +class BackfillChannelSlugs < ActiveRecord::Migration[7.0] + def up + channels = DB.query(<<~SQL) + SELECT chat_channels.id, COALESCE(chat_channels.name, categories.name) AS title, NULL as slug + FROM chat_channels + INNER JOIN categories ON categories.id = chat_channels.chatable_id + WHERE chat_channels.chatable_type = 'Category' AND chat_channels.slug IS NULL + SQL + return if channels.count.zero? + + DB.exec("CREATE TEMPORARY TABLE tmp_chat_channel_slugs(id int, slug text)") + + taken_slugs = {} + channels.each do |channel| + # Simplified version of Slug.for generation that doesn't take into + # account different encodings to make things a little easier. + title = channel.title + if title.blank? + channel.slug = "channel-#{channel.id}" + else + channel.slug = + title + .downcase + .chomp + .tr("'", "") + .parameterize + .tr("_", "-") + .truncate(255, omission: "") + .squeeze("-") + .gsub(/\A-+|-+\z/, "") + end + + # Deduplicate slugs with the channel IDs, we can always improve + # slugs later on. + channel.slug = "#{channel.slug}-#{channel.id}" if taken_slugs.key?(channel.slug) + taken_slugs[channel.slug] = true + end + + values_to_insert = + channels.map { |channel| "(#{channel.id}, '#{PG::Connection.escape_string(channel.slug)}')" } + + DB.exec( + "INSERT INTO tmp_chat_channel_slugs + VALUES #{values_to_insert.join(",\n")}", + ) + + DB.exec(<<~SQL) + UPDATE chat_channels cc + SET slug = tmp.slug + FROM tmp_chat_channel_slugs tmp + WHERE cc.id = tmp.id AND cc.slug IS NULL + SQL + + DB.exec("DROP TABLE tmp_chat_channel_slugs(id int, slug text)") + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/plugins/chat/spec/fabricators/chat_fabricator.rb b/plugins/chat/spec/fabricators/chat_fabricator.rb index 5a6f7f231dc..ee07f45022a 100644 --- a/plugins/chat/spec/fabricators/chat_fabricator.rb +++ b/plugins/chat/spec/fabricators/chat_fabricator.rb @@ -2,9 +2,22 @@ Fabricator(:chat_channel) do name do - ["Gaming Lounge", "Music Lodge", "Random", "Politics", "Sports Center", "Kino Buffs"].sample + sequence(:name) do |n| + random_name = [ + "Gaming Lounge", + "Music Lodge", + "Random", + "Politics", + "Sports Center", + "Kino Buffs", + ].sample + "#{random_name} #{n}" + end end chatable { Fabricate(:category) } + type do |attrs| + attrs[:chatable_type] == "Category" || attrs[:chatable]&.is_a?(Category) ? "CategoryChannel" : "DirectMessageChannel" + end status { :open } end diff --git a/plugins/chat/spec/jobs/regular/chat_notify_mentioned_spec.rb b/plugins/chat/spec/jobs/regular/chat_notify_mentioned_spec.rb index 0693f32ca61..32204adc9cd 100644 --- a/plugins/chat/spec/jobs/regular/chat_notify_mentioned_spec.rb +++ b/plugins/chat/spec/jobs/regular/chat_notify_mentioned_spec.rb @@ -215,7 +215,7 @@ describe Jobs::ChatNotifyMentioned do ) expect(desktop_notification.data[:excerpt]).to eq(message.push_notification_excerpt) expect(desktop_notification.data[:post_url]).to eq( - "/chat/channel/#{public_channel.id}/#{expected_channel_title}?messageId=#{message.id}", + "/chat/channel/#{public_channel.id}/#{public_channel.slug}?messageId=#{message.id}", ) end @@ -230,7 +230,7 @@ describe Jobs::ChatNotifyMentioned do tag: Chat::ChatNotifier.push_notification_tag(:mention, public_channel.id), excerpt: message.push_notification_excerpt, post_url: - "/chat/channel/#{public_channel.id}/#{expected_channel_title}?messageId=#{message.id}", + "/chat/channel/#{public_channel.id}/#{public_channel.slug}?messageId=#{message.id}", translated_title: payload_translated_title, }, ) @@ -259,6 +259,7 @@ describe Jobs::ChatNotifyMentioned do expect(data_hash[:mentioned_by_username]).to eq(user_1.username) expect(data_hash[:is_direct_message_channel]).to eq(false) expect(data_hash[:chat_channel_title]).to eq(expected_channel_title) + expect(data_hash[:chat_channel_slug]).to eq(public_channel.slug) chat_mention = ChatMention.where(notification: created_notification, user: user_2, chat_message: message) diff --git a/plugins/chat/spec/jobs/regular/chat_notify_watching_spec.rb b/plugins/chat/spec/jobs/regular/chat_notify_watching_spec.rb index b62a7a0573c..1690c3b0184 100644 --- a/plugins/chat/spec/jobs/regular/chat_notify_watching_spec.rb +++ b/plugins/chat/spec/jobs/regular/chat_notify_watching_spec.rb @@ -50,7 +50,13 @@ RSpec.describe Jobs::ChatNotifyWatching do { username: user1.username, notification_type: Notification.types[:chat_message], - post_url: "/chat/channel/#{channel.id}/#{channel.title(user2)}", + post_url: channel.relative_url, + translated_title: + I18n.t( + "discourse_push_notifications.popup.new_chat_message", + { username: user1.username, channel: channel.title(user2) }, + ), + tag: Chat::ChatNotifier.push_notification_tag(:message, channel.id), excerpt: message.message, }, ) @@ -81,7 +87,13 @@ RSpec.describe Jobs::ChatNotifyWatching do { username: user1.username, notification_type: Notification.types[:chat_message], - post_url: "/chat/channel/#{channel.id}/#{channel.title(user2)}", + post_url: channel.relative_url, + translated_title: + I18n.t( + "discourse_push_notifications.popup.new_chat_message", + { username: user1.username, channel: channel.title(user2) }, + ), + tag: Chat::ChatNotifier.push_notification_tag(:message, channel.id), excerpt: message.message, }, ), @@ -176,7 +188,13 @@ RSpec.describe Jobs::ChatNotifyWatching do { username: user1.username, notification_type: Notification.types[:chat_message], - post_url: "/chat/channel/#{channel.id}/#{channel.title(user2)}", + post_url: channel.relative_url, + translated_title: + I18n.t( + "discourse_push_notifications.popup.new_direct_chat_message", + { username: user1.username, channel: channel.title(user2) }, + ), + tag: Chat::ChatNotifier.push_notification_tag(:message, channel.id), excerpt: message.message, }, ) @@ -207,7 +225,13 @@ RSpec.describe Jobs::ChatNotifyWatching do { username: user1.username, notification_type: Notification.types[:chat_message], - post_url: "/chat/channel/#{channel.id}/#{channel.title(user2)}", + post_url: channel.relative_url, + translated_title: + I18n.t( + "discourse_push_notifications.popup.new_direct_chat_message", + { username: user1.username, channel: channel.title(user2) }, + ), + tag: Chat::ChatNotifier.push_notification_tag(:message, channel.id), excerpt: message.message, }, ), diff --git a/plugins/chat/spec/models/category_channel_spec.rb b/plugins/chat/spec/models/category_channel_spec.rb index ad59b3efe91..89c950f5f7a 100644 --- a/plugins/chat/spec/models/category_channel_spec.rb +++ b/plugins/chat/spec/models/category_channel_spec.rb @@ -83,4 +83,70 @@ RSpec.describe CategoryChannel do end end end + + describe "slug generation" do + subject(:channel) { Fabricate(:category_channel) } + + context "when slug is not provided" do + before do + channel.slug = nil + end + + it "uses channel name when present" do + channel.name = "Some Cool Stuff" + channel.validate! + expect(channel.slug).to eq("some-cool-stuff") + end + + it "uses category name when present" do + channel.name = nil + channel.category.name = "some category stuff" + channel.validate! + expect(channel.slug).to eq("some-category-stuff") + end + end + + context "when slug is provided" do + context "when using encoded slug generator" do + before do + SiteSetting.slug_generation_method = "encoded" + channel.slug = "测试" + end + after { SiteSetting.slug_generation_method = "ascii" } + + it "creates a slug with the correct escaping" do + channel.validate! + expect(channel.slug).to eq("%E6%B5%8B%E8%AF%95") + end + end + + context "when slug ends up blank" do + it "adds a validation error" do + channel.slug = "-" + channel.validate + expect(channel.errors.full_messages).to include("Slug is invalid") + end + end + + context "when there is a duplicate slug" do + before { 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")) + end + end + + context "if SiteSettings.slug_generation_method = ascii" do + before { SiteSetting.slug_generation_method = "ascii" } + + 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")}/) + end + end + end + end end diff --git a/plugins/chat/spec/models/chat_channel_spec.rb b/plugins/chat/spec/models/chat_channel_spec.rb new file mode 100644 index 00000000000..475390a07a9 --- /dev/null +++ b/plugins/chat/spec/models/chat_channel_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +RSpec.describe ChatChannel do + fab!(:category_channel) { Fabricate(:category_channel) } + fab!(:dm_channel) { Fabricate(:direct_message_channel) } + + describe "#relative_url" do + context "when the slug is nil" do + it "uses a - instead" do + category_channel.slug = nil + expect(category_channel.relative_url).to eq("/chat/channel/#{category_channel.id}/-") + end + end + + context "when the slug is not nil" do + before do + category_channel.update!(slug: "some-cool-channel") + end + + it "includes the slug for the channel" do + expect(category_channel.relative_url).to eq("/chat/channel/#{category_channel.id}/some-cool-channel") + end + end + end +end diff --git a/plugins/chat/spec/models/direct_message_channel_spec.rb b/plugins/chat/spec/models/direct_message_channel_spec.rb index 85674e858fb..227a143d60d 100644 --- a/plugins/chat/spec/models/direct_message_channel_spec.rb +++ b/plugins/chat/spec/models/direct_message_channel_spec.rb @@ -60,4 +60,14 @@ RSpec.describe DirectMessageChannel do expect(title).to eq("something") end end + + describe "slug generation" do + subject(:channel) { Fabricate(:direct_message_channel) } + + it "always sets the slug to nil for direct message channels" do + channel.name = "Cool Channel" + channel.validate! + expect(channel.slug).to eq(nil) + end + end end diff --git a/plugins/chat/spec/requests/chat_channel_controller_spec.rb b/plugins/chat/spec/requests/chat_channel_controller_spec.rb index f40a9ca0149..9e88a47f72f 100644 --- a/plugins/chat/spec/requests/chat_channel_controller_spec.rb +++ b/plugins/chat/spec/requests/chat_channel_controller_spec.rb @@ -305,7 +305,7 @@ RSpec.describe Chat::ChatChannelsController do it "errors when channel for category and same name already exists" do sign_in(admin) name = "beep boop hi" - ChatChannel.create!(chatable: category2, name: name) + category2.create_chat_channel!(name: name) put "/chat/chat_channels.json", params: { id: category2.id, name: name } expect(response.status).to eq(400) @@ -313,7 +313,7 @@ RSpec.describe Chat::ChatChannelsController do it "creates a channel for category and if name is unique" do sign_in(admin) - ChatChannel.create!(chatable: category2, name: "this is a name") + category2.create_chat_channel!(name: "this is a name") expect { put "/chat/chat_channels.json", params: { id: category2.id, name: "Different name!" } diff --git a/plugins/chat/spec/requests/chat_controller_spec.rb b/plugins/chat/spec/requests/chat_controller_spec.rb index 78dc47157f8..ad42097db28 100644 --- a/plugins/chat/spec/requests/chat_controller_spec.rb +++ b/plugins/chat/spec/requests/chat_controller_spec.rb @@ -809,6 +809,7 @@ RSpec.describe Chat::ChatController do chat_message_id: msg.id, chat_channel_id: msg.chat_channel_id, chat_channel_title: msg.chat_channel.title(user), + chat_channel_slug: msg.chat_channel.slug, mentioned_by_username: sender.username, }.to_json, ) @@ -1029,6 +1030,10 @@ RSpec.describe Chat::ChatController do }.to change { user.notifications.where(notification_type: Notification.types[:chat_invitation]).count }.by(1) + notification = user.notifications.where(notification_type: Notification.types[:chat_invitation]).last + parsed_data = JSON.parse(notification[:data]) + expect(parsed_data["chat_channel_title"]).to eq(chat_channel.title(user)) + expect(parsed_data["chat_channel_slug"]).to eq(chat_channel.slug) end it "creates multiple invitations" do diff --git a/plugins/chat/test/javascripts/chat-fixtures.js b/plugins/chat/test/javascripts/chat-fixtures.js index 1877116d9f2..c8ab6151605 100644 --- a/plugins/chat/test/javascripts/chat-fixtures.js +++ b/plugins/chat/test/javascripts/chat-fixtures.js @@ -78,7 +78,7 @@ const chatables = { 8: { id: 8, name: "Public category", - slug: "public_category", + slug: "public-category", posts_count: 1, }, 12: { diff --git a/plugins/chat/test/javascripts/unit/lib/slugify-channel-test.js b/plugins/chat/test/javascripts/unit/lib/slugify-channel-test.js index 52bd96830ef..9ce84b57ae3 100644 --- a/plugins/chat/test/javascripts/unit/lib/slugify-channel-test.js +++ b/plugins/chat/test/javascripts/unit/lib/slugify-channel-test.js @@ -2,20 +2,38 @@ import { module, test } from "qunit"; import slugifyChannel from "discourse/plugins/chat/discourse/lib/slugify-channel"; module("Discourse Chat | Unit | slugify-channel", function () { - test("defaults", function (assert) { - assert.equal(slugifyChannel("Foo bar"), "foo-bar"); + test("defaults for title", function (assert) { + assert.equal(slugifyChannel({ title: "Foo bar" }), "foo-bar"); }); - test("a very long name", function (assert) { + test("a very long name for the title", function (assert) { const string = "xAq8l5ca2CtEToeMLe2pEr2VUGQBx3HPlxbkDExKrJHp4f7jCVw9id1EQv1N1lYMRdAIiZNnn94Kr0uU0iiEeVO4XkBVmpW8Mknmd"; - assert.equal(slugifyChannel(string), string.toLowerCase().slice(0, -1)); + assert.equal( + slugifyChannel({ title: string }), + string.toLowerCase().slice(0, -1) + ); }); - test("a cyrillic name", function (assert) { + test("a cyrillic name for the title", function (assert) { const string = "Русская литература и фольклор"; - assert.equal(slugifyChannel(string), "русская-литература-и-фольклор"); + assert.equal( + slugifyChannel({ title: string }), + "русская-литература-и-фольклор" + ); + }); + + test("channel has escapedTitle", function (assert) { + assert.equal(slugifyChannel({ escapedTitle: "Foo bar" }), "foo-bar"); + }); + + test("channel has slug and title", function (assert) { + assert.equal( + slugifyChannel({ title: "Foo bar", slug: "some-other-thing" }), + "some-other-thing", + "slug takes priority" + ); }); }); diff --git a/plugins/chat/test/javascripts/widgets/chat-invitation-notification-item-test.js b/plugins/chat/test/javascripts/widgets/chat-invitation-notification-item-test.js index a5f561b2a0f..8e850d9f8ff 100644 --- a/plugins/chat/test/javascripts/widgets/chat-invitation-notification-item-test.js +++ b/plugins/chat/test/javascripts/widgets/chat-invitation-notification-item-test.js @@ -43,9 +43,9 @@ module( const data = this.args.data; assert.strictEqual( query(".chat-invitation a").getAttribute("href"), - `/chat/channel/${data.chat_channel_id}/${slugifyChannel( - data.chat_channel_title - )}?messageId=${data.chat_message_id}` + `/chat/channel/${data.chat_channel_id}/${slugifyChannel({ + title: data.chat_channel_title, + })}?messageId=${data.chat_message_id}` ); }); } diff --git a/plugins/chat/test/javascripts/widgets/chat-mention-notification-item-test.js b/plugins/chat/test/javascripts/widgets/chat-mention-notification-item-test.js index d341e4ac8ce..65532c33b10 100644 --- a/plugins/chat/test/javascripts/widgets/chat-mention-notification-item-test.js +++ b/plugins/chat/test/javascripts/widgets/chat-mention-notification-item-test.js @@ -52,9 +52,9 @@ module( assert.strictEqual( query(".chat-invitation a").getAttribute("href"), - `/chat/channel/${data.chat_channel_id}/${slugifyChannel( - data.chat_channel_title - )}?messageId=${data.chat_message_id}` + `/chat/channel/${data.chat_channel_id}/${slugifyChannel({ + title: data.chat_channel_title, + })}?messageId=${data.chat_message_id}` ); }); } @@ -91,9 +91,9 @@ module( assert.strictEqual( query(".chat-invitation a").getAttribute("href"), - `/chat/channel/${data.chat_channel_id}/${slugifyChannel( - data.chat_channel_title - )}?messageId=${data.chat_message_id}` + `/chat/channel/${data.chat_channel_id}/${slugifyChannel({ + title: data.chat_channel_title, + })}?messageId=${data.chat_message_id}` ); }); } @@ -130,9 +130,9 @@ module( assert.strictEqual( query(".chat-invitation a").getAttribute("href"), - `/chat/channel/${data.chat_channel_id}/${slugifyChannel( - data.chat_channel_title - )}?messageId=${data.chat_message_id}` + `/chat/channel/${data.chat_channel_id}/${slugifyChannel({ + title: data.chat_channel_title, + })}?messageId=${data.chat_message_id}` ); }); }