FIX: Automatically generate category channel slugs (#18879)

This commit automatically ensures that category channels
have slugs when they are created or updated based on the
channel name, category name, or existing slug. The behaviour
has been copied from the Category model.

We also include a backfill here with a simplified version
of Slug.for with deduplication to fill the slugs for already
created Category chat channels.

The channel slug is also now used for chat notifications,
and for the UI and navigation for chat. `slugifyChannel`
is still used, but now does the following fallback:

* Uses channel.slug if it is present
* Uses channel.escapedTitle if it is present
* Uses channel.title if it is present

In future we may want to remove this altogether
and always rely on the slug being present, but this
is currently not possible because we are not generating
slugs for DM channels at this point.

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

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."

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]

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 =

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,

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

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
 #

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

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,

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() {

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() {

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, "-")
-  ).slice(0, 100);
+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;
 }

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);
     }

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 {

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"
           >

plugins/chat/assets/javascripts/discourse/templates/components/chat-channel-card.hbs

@@ -10,7 +10,7 @@
     <div class="chat-channel-card__header">
       <LinkTo
         @route="chat.channel"
-        @models={{array this.channel.id (slugify-channel this.channel.title)}}
+        @models={{array this.channel.id (slugify-channel this.channel)}}
         class="chat-channel-card__name-container"
       >
         <span class="chat-channel-card__name">
@@ -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 @@
 
         <LinkTo
           @route="chat.channel.info.settings"
-          @models={{array this.channel.id (slugify-channel this.channel.title)}}
+          @models={{array this.channel.id (slugify-channel this.channel)}}
           class="chat-channel-card__setting"
           tabindex="-1"
         >
@@ -83,7 +83,7 @@
       {{#if (gt this.channel.membershipsCount 0)}}
         <LinkTo
           @route="chat.channel.info.members"
-          @models={{array this.channel.id (slugify-channel this.channel.title)}}
+          @models={{array this.channel.id (slugify-channel this.channel)}}
           class="chat-channel-card__members"
           tabindex="-1"
         >

plugins/chat/assets/javascripts/discourse/templates/components/chat-live-pane.hbs

@@ -10,7 +10,7 @@
         </div>
       {{/if}}
 
-      <LinkTo @route={{this.infoTabRoute}} @models={{array this.chatChannel.id (slugify-channel this.chatChannel.title)}} class="chat-full-page-header__title">
+      <LinkTo @route={{this.infoTabRoute}} @models={{array this.chatChannel.id (slugify-channel this.chatChannel)}} class="chat-full-page-header__title">
         <ChatChannelTitle @channel={{this.chatChannel}} />
       </LinkTo>
 

plugins/chat/assets/javascripts/discourse/templates/components/topic-chat-float.hbs

@@ -29,7 +29,7 @@
 
         {{#if this.chat.activeChannel}}
           {{#if this.expanded}}
-            <LinkTo @route={{this.infoTabRoute}} @models={{array this.chat.activeChannel.id (slugify-channel this.chat.activeChannel.title)}} class="topic-chat-drawer-header__title">
+            <LinkTo @route={{this.infoTabRoute}} @models={{array this.chat.activeChannel.id (slugify-channel this.chat.activeChannel)}} class="topic-chat-drawer-header__title">
               <div class="topic-chat-drawer-header__top-line">
                 <ChatChannelTitle @channel={{this.chat.activeChannel}} />
               </div>

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
+    }`;
   },
 });

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
+    }`;
   },
 };
 

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}"
 

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

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
 

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)

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,
             },
           ),

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

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

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

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!" }

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

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: {

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"
+    );
   });
 });

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}`
       );
     });
   }

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}`
       );
     });
   }