diff --git a/app/controllers/slugs_controller.rb b/app/controllers/slugs_controller.rb new file mode 100644 index 00000000000..531f8ff4fd8 --- /dev/null +++ b/app/controllers/slugs_controller.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class SlugsController < ApplicationController + requires_login + + MAX_SLUG_GENERATIONS_PER_MINUTE = 20 + + def generate + params.require(:name) + + raise Discourse::InvalidAccess if !current_user.has_trust_level_or_staff?(TrustLevel[4]) + + RateLimiter.new( + current_user, + "max-slug-generations-per-minute", + MAX_SLUG_GENERATIONS_PER_MINUTE, + 1.minute, + ).performed! + + render json: success_json.merge(slug: Slug.for(params[:name], "")) + end +end diff --git a/config/routes.rb b/config/routes.rb index 606fffd6d50..860ab8bd18f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1064,6 +1064,8 @@ Discourse::Application.routes.draw do resources :associated_groups, only: %i[index], constraints: AdminConstraint.new + get "slugs/generate", to: "slugs#generate" + # aliases so old API code works delete "admin/groups/:id/members" => "groups#remove_member", :constraints => AdminConstraint.new put "admin/groups/:id/members" => "groups#add_members", :constraints => AdminConstraint.new diff --git a/plugins/chat/app/controllers/api/chat_channels_controller.rb b/plugins/chat/app/controllers/api/chat_channels_controller.rb index f61107037ab..5d007000217 100644 --- a/plugins/chat/app/controllers/api/chat_channels_controller.rb +++ b/plugins/chat/app/controllers/api/chat_channels_controller.rb @@ -57,7 +57,7 @@ class Chat::Api::ChatChannelsController < Chat::Api def create channel_params = - params.require(:channel).permit(:chatable_id, :name, :description, :auto_join_users) + params.require(:channel).permit(:chatable_id, :name, :slug, :description, :auto_join_users) guardian.ensure_can_create_chat_channel! if channel_params[:name].length > SiteSetting.max_topic_title_length @@ -81,6 +81,7 @@ class Chat::Api::ChatChannelsController < Chat::Api channel = chatable.create_chat_channel!( name: channel_params[:name], + slug: channel_params[:slug], description: channel_params[:description], user_count: 1, auto_join_users: auto_join_users, diff --git a/plugins/chat/assets/javascripts/discourse/controllers/create-channel.js b/plugins/chat/assets/javascripts/discourse/controllers/create-channel.js index 9d024369470..062a3c76648 100644 --- a/plugins/chat/assets/javascripts/discourse/controllers/create-channel.js +++ b/plugins/chat/assets/javascripts/discourse/controllers/create-channel.js @@ -1,4 +1,7 @@ import { escapeExpression } from "discourse/lib/utilities"; +import { ajax } from "discourse/lib/ajax"; +import { cancel } from "@ember/runloop"; +import discourseDebounce from "discourse-common/lib/debounce"; import Controller from "@ember/controller"; import I18n from "I18n"; import ModalFunctionality from "discourse/mixins/modal-functionality"; @@ -26,6 +29,8 @@ export default class CreateChannelController extends Controller.extend( category = null; categoryId = null; name = ""; + slug = ""; + autoGeneratedSlug = ""; description = ""; categoryPermissionsHint = null; autoJoinUsers = null; @@ -51,11 +56,14 @@ export default class CreateChannelController extends Controller.extend( } onClose() { + cancel(this.generateSlugHandler); this.setProperties({ categoryId: null, category: null, name: "", description: "", + slug: "", + autoGeneratedSlug: "", categoryPermissionsHint: DEFAULT_HINT, autoJoinWarning: "", }); @@ -65,6 +73,7 @@ export default class CreateChannelController extends Controller.extend( const data = { chatable_id: this.categoryId, name: this.name, + slug: this.slug || this.autoGeneratedSlug, description: this.description, auto_join_users: this.autoJoinUsers, }; @@ -147,17 +156,53 @@ export default class CreateChannelController extends Controller.extend( } } + // intentionally not showing AJAX error for this, we will autogenerate + // the slug server-side if they leave it blank + _generateSlug(name) { + ajax("/slugs/generate.json", { type: "GET", data: { name } }).then( + (response) => { + this.set("autoGeneratedSlug", response.slug); + } + ); + } + + _debouncedGenerateSlug(name) { + cancel(this.generateSlugHandler); + this._clearAutoGeneratedSlug(); + if (!name) { + return; + } + this.generateSlugHandler = discourseDebounce( + this, + this._generateSlug, + name, + 300 + ); + } + + _clearAutoGeneratedSlug() { + this.set("autoGeneratedSlug", ""); + } + @action onCategoryChange(categoryId) { let category = categoryId ? this.site.categories.findBy("id", categoryId) : null; this._updatePermissionsHint(category); + + const name = this.name || category?.name || ""; this.setProperties({ categoryId, category, - name: this.name || category?.name || "", + name, }); + this._debouncedGenerateSlug(name); + } + + @action + onNameChange(name) { + this._debouncedGenerateSlug(name); } @action diff --git a/plugins/chat/assets/javascripts/discourse/templates/modal/create-channel.hbs b/plugins/chat/assets/javascripts/discourse/templates/modal/create-channel.hbs index 2ed1417f4d7..c25dc4e63b6 100644 --- a/plugins/chat/assets/javascripts/discourse/templates/modal/create-channel.hbs +++ b/plugins/chat/assets/javascripts/discourse/templates/modal/create-channel.hbs @@ -8,6 +8,20 @@ class="create-channel-name-input" @type="text" @value={{this.name}} + {{on "input" (action "onNameChange" value="target.value")}} + /> + + +
+ +
diff --git a/plugins/chat/assets/stylesheets/common/create-channel-modal.scss b/plugins/chat/assets/stylesheets/common/create-channel-modal.scss index e2706683986..bfd8a7735ec 100644 --- a/plugins/chat/assets/stylesheets/common/create-channel-modal.scss +++ b/plugins/chat/assets/stylesheets/common/create-channel-modal.scss @@ -10,6 +10,7 @@ .select-kit.combo-box, .create-channel-name-input, + .create-channel-slug-input, .create-channel-description-input, #choose-topic-title { width: 100%; diff --git a/plugins/chat/config/locales/client.en.yml b/plugins/chat/config/locales/client.en.yml index 6c27e8346cd..bda7d953a38 100644 --- a/plugins/chat/config/locales/client.en.yml +++ b/plugins/chat/config/locales/client.en.yml @@ -289,6 +289,7 @@ en: create: "Create channel" description: "Description (optional)" name: "Channel name" + slug: "Channel slug (optional)" title: "New channel" type: "Type" types: diff --git a/plugins/chat/spec/requests/api/chat_channels_controller_spec.rb b/plugins/chat/spec/requests/api/chat_channels_controller_spec.rb index fd1a3375ea0..eda1712df27 100644 --- a/plugins/chat/spec/requests/api/chat_channels_controller_spec.rb +++ b/plugins/chat/spec/requests/api/chat_channels_controller_spec.rb @@ -263,11 +263,22 @@ RSpec.describe Chat::Api::ChatChannelsController do new_channel = ChatChannel.last expect(new_channel.name).to eq(params[:channel][:name]) + expect(new_channel.slug).to eq("channel-name") expect(new_channel.description).to eq(params[:channel][:description]) expect(new_channel.chatable_type).to eq(category.class.name) expect(new_channel.chatable_id).to eq(category.id) end + it "creates a channel using the user-provided slug" do + new_params = params.dup + new_params[:channel][:slug] = "wow-so-cool" + post "/chat/api/channels", params: new_params + + new_channel = ChatChannel.last + + expect(new_channel.slug).to eq("wow-so-cool") + end + it "creates a channel sets auto_join_users to false by default" do post "/chat/api/channels", params: params diff --git a/plugins/chat/spec/system/create_channel_spec.rb b/plugins/chat/spec/system/create_channel_spec.rb index a525f09371c..8cf0d19d999 100644 --- a/plugins/chat/spec/system/create_channel_spec.rb +++ b/plugins/chat/spec/system/create_channel_spec.rb @@ -1,35 +1,43 @@ # frozen_string_literal: true RSpec.describe "Create channel", type: :system, js: true do - fab!(:current_admin_user) { Fabricate(:admin) } fab!(:category_1) { Fabricate(:category) } let(:chat_page) { PageObjects::Pages::Chat.new } + let(:channel_modal) { PageObjects::Modals::ChatChannelCreate.new } - before do - chat_system_bootstrap - sign_in(current_admin_user) + before { chat_system_bootstrap } + + context "when user cannot create channel" do + fab!(:current_user) { Fabricate(:user) } + before { sign_in(current_user) } + + it "does not show the create channel button" do + chat_page.visit_browse + expect(chat_page).not_to have_new_channel_button + end end context "when can create channel" do + fab!(:current_admin_user) { Fabricate(:admin) } + before { sign_in(current_admin_user) } + context "when selecting a category" do it "shows access hint" do - visit("/chat") - find(".new-channel-btn").click - find(".category-chooser").click - find(".category-row[data-value=\"#{category_1.id}\"]").click + chat_page.visit_browse + chat_page.new_channel_button.click + channel_modal.select_category(category_1) - expect(find(".create-channel-hint")).to have_content(Group[:everyone].name) + expect(channel_modal).to have_create_hint(Group[:everyone].name) end it "does not override channel name if that was already specified" do - visit("/chat") - find(".new-channel-btn").click - fill_in("channel-name", with: "My Cool Channel") - find(".category-chooser").click - find(".category-row[data-value=\"#{category_1.id}\"]").click + chat_page.visit_browse + chat_page.new_channel_button.click + channel_modal.fill_name("My Cool Channel") + channel_modal.select_category(category_1) - expect(page).to have_field("channel-name", with: "My Cool Channel") + expect(channel_modal).to have_name_prefilled("My Cool Channel") end context "when category is private" do @@ -37,12 +45,11 @@ RSpec.describe "Create channel", type: :system, js: true do fab!(:private_category_1) { Fabricate(:private_category, group: group_1) } it "shows access hint when selecting the category" do - visit("/chat") - find(".new-channel-btn").click - find(".category-chooser").click - find(".category-row[data-value=\"#{private_category_1.id}\"]").click + chat_page.visit_browse + chat_page.new_channel_button.click + channel_modal.select_category(private_category_1) - expect(find(".create-channel-hint")).to have_content(group_1.name) + expect(channel_modal).to have_create_hint(group_1.name) end context "when category is a child" do @@ -52,12 +59,11 @@ RSpec.describe "Create channel", type: :system, js: true do end it "shows access hint when selecting the category" do - visit("/chat") - find(".new-channel-btn").click - find(".category-chooser").click - find(".category-row[data-value=\"#{child_category.id}\"]").click + chat_page.visit_browse + chat_page.new_channel_button.click + channel_modal.select_category(child_category) - expect(find(".create-channel-hint")).to have_content(group_2.name) + expect(channel_modal).to have_create_hint(group_2.name) end end end @@ -72,45 +78,91 @@ RSpec.describe "Create channel", type: :system, js: true do fab!(:private_category_1) { Fabricate(:private_category, group: group_1) } it "escapes the group name" do - visit("/chat") - find(".new-channel-btn").click - find(".category-chooser").click - find(".category-row[data-value=\"#{private_category_1.id}\"]").click + chat_page.visit_browse + chat_page.new_channel_button.click + channel_modal.select_category(private_category_1) - expect(find(".create-channel-hint")["innerHTML"].strip).to include( + expect(channel_modal.create_channel_hint["innerHTML"].strip).to include( "<script>e</script>", ) end end + it "autogenerates slug from name and changes slug placeholder" do + chat_page.visit_browse + chat_page.new_channel_button.click + name = "Cats & Dogs" + channel_modal.select_category(category_1) + channel_modal.fill_name(name) + channel_modal.fill_description("All kind of cute cats") + + wait_for_attribute(channel_modal.slug_input, :placeholder, "cats-dogs") + + channel_modal.click_primary_button + + expect(page).to have_content(name) + created_channel = ChatChannel.find_by(chatable_id: category_1.id) + expect(created_channel.slug).to eq("cats-dogs") + expect(page).to have_current_path(chat.channel_path(created_channel.id, created_channel.slug)) + end + + it "allows the user to set a slug independently of name" do + chat_page.visit_browse + chat_page.new_channel_button.click + name = "Cats & Dogs" + channel_modal.select_category(category_1) + channel_modal.fill_name(name) + channel_modal.fill_description("All kind of cute cats") + channel_modal.fill_slug("pets-everywhere") + channel_modal.click_primary_button + + expect(page).to have_content(name) + created_channel = ChatChannel.find_by(chatable_id: category_1.id) + expect(created_channel.slug).to eq("pets-everywhere") + expect(page).to have_current_path(chat.channel_path(created_channel.id, created_channel.slug)) + end + context "when saving" do context "when error" do it "displays the error" do existing_channel = Fabricate(:chat_channel) - visit("/chat") - find(".new-channel-btn").click - find(".category-chooser").click - find(".category-row[data-value=\"#{existing_channel.chatable_id}\"]").click - fill_in("channel-name", with: existing_channel.name) - find(".create-channel-modal .create").click + chat_page.visit_browse + chat_page.new_channel_button.click + channel_modal.select_category(existing_channel.chatable) + channel_modal.fill_name(existing_channel.name) + channel_modal.click_primary_button expect(page).to have_content(I18n.t("chat.errors.channel_exists_for_category")) end end + context "when slug is already being used" do + it "displays the error" do + Fabricate(:chat_channel, slug: "pets-everywhere") + chat_page.visit_browse + chat_page.new_channel_button.click + channel_modal.select_category(category_1) + channel_modal.fill_name("Testing") + channel_modal.fill_slug("pets-everywhere") + channel_modal.click_primary_button + + expect(page).to have_content( + "Slug " + I18n.t("chat.category_channel.errors.is_already_in_use"), + ) + end + end + context "when successful" do it "redirects to created channel" do - visit("/chat") - find(".new-channel-btn").click - name = "Cats" - find(".category-chooser").click - find(".category-row[data-value=\"#{category_1.id}\"]").click - expect(page).to have_field("channel-name", with: category_1.name) - fill_in("channel-name", with: name) - fill_in("channel-description", with: "All kind of cute cats") - find(".create-channel-modal .create").click + chat_page.visit_browse + chat_page.new_channel_button.click + channel_modal.select_category(category_1) + expect(channel_modal).to have_name_prefilled(category_1.name) - expect(page).to have_content(name) + channel_modal.fill_description("All kind of cute cats") + channel_modal.click_primary_button + + expect(page).to have_content(category_1.name) created_channel = ChatChannel.find_by(chatable_id: category_1.id) expect(page).to have_current_path( chat.channel_path(created_channel.id, created_channel.slug), diff --git a/plugins/chat/spec/system/page_objects/chat/chat.rb b/plugins/chat/spec/system/page_objects/chat/chat.rb index 38e341e46d2..0ceceb7373f 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat.rb @@ -51,6 +51,14 @@ module PageObjects container.has_content?(message.message) container.has_content?(message.user.username) end + + def new_channel_button + find(".new-channel-btn") + end + + def has_new_channel_button? + has_css?(".new-channel-btn") + end end end end diff --git a/plugins/chat/spec/system/page_objects/modals/chat_channel_create.rb b/plugins/chat/spec/system/page_objects/modals/chat_channel_create.rb new file mode 100644 index 00000000000..41788b203d7 --- /dev/null +++ b/plugins/chat/spec/system/page_objects/modals/chat_channel_create.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module PageObjects + module Modals + class ChatChannelCreate < PageObjects::Modals::Base + def select_category(category) + find(".category-chooser").click + find(".category-row[data-value=\"#{category.id}\"]").click + end + + def create_channel_hint + find(".create-channel-hint") + end + + def slug_input + find(".create-channel-slug-input") + end + + def has_create_hint?(content) + create_channel_hint.has_content?(content) + end + + def fill_name(name) + fill_in("channel-name", with: name) + end + + def fill_slug(slug) + fill_in("channel-slug", with: slug) + end + + def fill_description(description) + fill_in("channel-description", with: description) + end + + def has_name_prefilled?(name) + has_field?("channel-name", with: name) + end + end + end +end diff --git a/spec/requests/slugs_controller_spec.rb b/spec/requests/slugs_controller_spec.rb new file mode 100644 index 00000000000..ec56d73a479 --- /dev/null +++ b/spec/requests/slugs_controller_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +RSpec.describe SlugsController do + fab!(:current_user) { Fabricate(:user, trust_level: TrustLevel[4]) } + + describe "#generate" do + let(:name) { "Arts & Media" } + + context "when user not logged in" do + it "returns a 403 error" do + get "/slugs/generate.json?name=#{name}" + expect(response.status).to eq(403) + end + end + + context "when user is logged in" do + before { sign_in(current_user) } + + it "generates a slug from the name" do + get "/slugs/generate.json", params: { name: name } + expect(response.status).to eq(200) + expect(response.parsed_body["slug"]).to eq(Slug.for(name, "")) + end + + it "rate limits" do + RateLimiter.enable + + stub_const(SlugsController, "MAX_SLUG_GENERATIONS_PER_MINUTE", 1) do + get "/slugs/generate.json?name=#{name}" + get "/slugs/generate.json?name=#{name}" + end + + expect(response.status).to eq(429) + end + + it "requires name" do + get "/slugs/generate.json" + expect(response.status).to eq(400) + end + + context "when user is not TL4 or higher" do + before { current_user.change_trust_level!(1) } + + it "returns a 403 error" do + get "/slugs/generate.json?name=#{name}" + expect(response.status).to eq(403) + end + end + + context "when user is admin" do + fab!(:current_user) { Fabricate(:admin) } + + it "generates a slug from the name" do + get "/slugs/generate.json", params: { name: name } + expect(response.status).to eq(200) + expect(response.parsed_body["slug"]).to eq(Slug.for(name, "")) + end + end + end + end +end diff --git a/spec/support/system_helpers.rb b/spec/support/system_helpers.rb index 7813ec137e6..028483bf886 100644 --- a/spec/support/system_helpers.rb +++ b/spec/support/system_helpers.rb @@ -39,6 +39,18 @@ module SystemHelpers retry end + def wait_for_attribute( + element, + attribute, + value, + timeout: Capybara.default_max_wait_time, + frequency: 0.01 + ) + try_until_success(timeout: timeout, frequency: frequency) do + expect(element[attribute.to_sym]).to eq(value) + end + end + def resize_window(width: nil, height: nil) original_size = page.driver.browser.manage.window.size page.driver.browser.manage.window.resize_to( diff --git a/spec/system/page_objects/modals/base.rb b/spec/system/page_objects/modals/base.rb index 7bad184882c..5d8e68996fa 100644 --- a/spec/system/page_objects/modals/base.rb +++ b/spec/system/page_objects/modals/base.rb @@ -17,6 +17,10 @@ module PageObjects def click_outside find(".modal-outer-container").click(x: 0, y: 0) end + + def click_primary_button + find(".modal-footer .btn-primary").click + end end end end