diff --git a/app/assets/javascripts/discourse/app/controllers/sidebar-section-form.js b/app/assets/javascripts/discourse/app/controllers/sidebar-section-form.js index d23ff569cb2..71467d82c89 100644 --- a/app/assets/javascripts/discourse/app/controllers/sidebar-section-form.js +++ b/app/assets/javascripts/discourse/app/controllers/sidebar-section-form.js @@ -8,6 +8,7 @@ import I18n from "I18n"; import { sanitize } from "discourse/lib/text"; import { tracked } from "@glimmer/tracking"; import { A } from "@ember/array"; +import { SIDEBAR_SECTION, SIDEBAR_URL } from "discourse/lib/constants"; const FULL_RELOAD_LINKS_REGEX = [/^\/my\/[a-z_\-\/]+$/, /^\/safe-mode$/]; @@ -29,12 +30,34 @@ class Section { } get validTitle() { - return !isEmpty(this.title) && this.title.length <= 30; + return !this.#blankTitle && !this.#tooLongTitle; + } + + get invalidTitleMessage() { + if (this.title === undefined) { + return; + } + if (this.#blankTitle) { + return I18n.t("sidebar.sections.custom.title.validation.blank"); + } + if (this.#tooLongTitle) { + return I18n.t("sidebar.sections.custom.title.validation.maximum", { + count: SIDEBAR_SECTION.max_title_length, + }); + } } get titleCssClass() { return this.title === undefined || this.validTitle ? "" : "warning"; } + + get #blankTitle() { + return isEmpty(this.title); + } + + get #tooLongTitle() { + return this.title.length > SIDEBAR_SECTION.max_title_length; + } } class SectionLink { @@ -62,21 +85,71 @@ class SectionLink { } get validIcon() { - return !isEmpty(this.icon) && this.icon.length <= 40; + return !this.#blankIcon && !this.#tooLongIcon; + } + + get validName() { + return !this.#blankName && !this.#tooLongName; + } + + get validValue() { + return !this.#blankValue && !this.#tooLongValue && !this.#invalidValue; + } + + get invalidIconMessage() { + if (this.#blankIcon) { + return I18n.t("sidebar.sections.custom.links.icon.validation.blank"); + } + if (this.#tooLongIcon) { + return I18n.t("sidebar.sections.custom.links.icon.validation.maximum", { + count: SIDEBAR_URL.max_icon_length, + }); + } + } + + get invalidNameMessage() { + if (this.name === undefined) { + return; + } + if (this.#blankName) { + return I18n.t("sidebar.sections.custom.links.name.validation.blank"); + } + if (this.#tooLongName) { + return I18n.t("sidebar.sections.custom.links.name.validation.maximum", { + count: SIDEBAR_URL.max_name_length, + }); + } + } + + get invalidValueMessage() { + if (this.value === undefined) { + return; + } + if (this.#blankValue) { + return I18n.t("sidebar.sections.custom.links.value.validation.blank"); + } + if (this.#tooLongValue) { + return I18n.t("sidebar.sections.custom.links.value.validation.maximum", { + count: SIDEBAR_URL.max_value_length, + }); + } + if (this.#invalidValue) { + return I18n.t("sidebar.sections.custom.links.value.validation.invalid"); + } } get iconCssClass() { return this.icon === undefined || this.validIcon ? "" : "warning"; } - get validName() { - return !isEmpty(this.name) && this.name.length <= 80; - } - get nameCssClass() { return this.name === undefined || this.validName ? "" : "warning"; } + get valueCssClass() { + return this.value === undefined || this.validValue ? "" : "warning"; + } + get external() { return ( this.value && @@ -88,6 +161,37 @@ class SectionLink { ); } + get #blankIcon() { + return isEmpty(this.icon); + } + + get #tooLongIcon() { + return this.icon.length > SIDEBAR_URL.max_icon_length; + } + + get #blankName() { + return isEmpty(this.name); + } + + get #tooLongName() { + return this.name.length > SIDEBAR_URL.max_name_length; + } + + get #blankValue() { + return isEmpty(this.value); + } + + get #tooLongValue() { + return this.value.length > SIDEBAR_URL.max_value_length; + } + + get #invalidValue() { + return ( + this.path && + (this.external ? !this.#validExternal() : !this.#validInternal()) + ); + } + #validExternal() { try { return new URL(this.value); @@ -102,19 +206,6 @@ class SectionLink { FULL_RELOAD_LINKS_REGEX.some((regex) => this.path.match(regex)) ); } - - get validValue() { - return ( - !isEmpty(this.value) && - this.value.length <= 200 && - this.path && - (this.external ? this.#validExternal() : this.#validInternal()) - ); - } - - get valueCssClass() { - return this.value === undefined || this.validValue ? "" : "warning"; - } } export default Controller.extend(ModalFunctionality, { diff --git a/app/assets/javascripts/discourse/app/lib/constants.js b/app/assets/javascripts/discourse/app/lib/constants.js index c2e876a2612..d17290caf30 100644 --- a/app/assets/javascripts/discourse/app/lib/constants.js +++ b/app/assets/javascripts/discourse/app/lib/constants.js @@ -11,3 +11,13 @@ export const SEARCH_PRIORITIES = { }; export const SEARCH_PHRASE_REGEXP = '"([^"]+)"'; + +export const SIDEBAR_URL = { + max_icon_length: 40, + max_name_length: 80, + max_value_length: 200, +}; + +export const SIDEBAR_SECTION = { + max_title_length: 30, +}; diff --git a/app/assets/javascripts/discourse/app/templates/modal/sidebar-section-form.hbs b/app/assets/javascripts/discourse/app/templates/modal/sidebar-section-form.hbs index 7c9962eedb7..831629cc97b 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/sidebar-section-form.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/sidebar-section-form.hbs @@ -6,7 +6,9 @@
- + + {{#if this.model.invalidTitleMessage}} +
+ {{this.model.invalidTitleMessage}} +
+ {{/if}}
{{#each this.activeLinks as |link|}}
+ {{#if link.invalidIconMessage}} +
+ {{link.invalidIconMessage}} +
+ {{/if}}
+ {{#if link.invalidNameMessage}} +
+ {{link.invalidNameMessage}} +
+ {{/if}}
+ {{#if link.invalidValueMessage}} +
+ {{link.invalidValueMessage}} +
+ {{/if}}
{ order("position") }, dependent: :destroy has_many :sidebar_urls, @@ -10,7 +12,14 @@ class SidebarSection < ActiveRecord::Base accepts_nested_attributes_for :sidebar_urls, allow_destroy: true - validates :title, presence: true, uniqueness: { scope: %i[user_id] }, length: { maximum: 30 } + validates :title, + presence: true, + uniqueness: { + scope: %i[user_id], + }, + length: { + maximum: MAX_TITLE_LENGTH, + } end # == Schema Information diff --git a/app/models/sidebar_url.rb b/app/models/sidebar_url.rb index 73731ec605a..ad94de39a02 100644 --- a/app/models/sidebar_url.rb +++ b/app/models/sidebar_url.rb @@ -2,10 +2,13 @@ class SidebarUrl < ActiveRecord::Base FULL_RELOAD_LINKS_REGEX = [%r{\A/my/[a-z_\-/]+\z}, %r{\A/safe-mode\z}] + MAX_ICON_LENGTH = 40 + MAX_NAME_LENGTH = 80 + MAX_VALUE_LENGTH = 200 - validates :icon, presence: true, length: { maximum: 40 } - validates :name, presence: true, length: { maximum: 80 } - validates :value, presence: true, length: { maximum: 200 } + validates :icon, presence: true, length: { maximum: MAX_ICON_LENGTH } + validates :name, presence: true, length: { maximum: MAX_NAME_LENGTH } + validates :value, presence: true, length: { maximum: MAX_VALUE_LENGTH } validate :path_validator diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6ebeb2322cb..b98ea367ecf 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -4389,17 +4389,34 @@ en: custom: add: "Add custom section" edit: "Edit custom section" - name: "Section title" save: "Save" delete: "Delete" delete_confirm: "Are you sure you want to delete this section?" public: "Make this section public and visible to everyone" links: - icon: "Icon" - name: "Name" - value: "Link" add: "Add another link" delete: "Delete link" + icon: + label: "Icon" + validation: + blank: "Icon cannot be blank" + maximum: "Icon must be shorter than %{count} characters" + name: + label: "Name" + validation: + blank: "Name cannot be blank" + maximum: "Name must be shorter than %{count} characters" + value: + label: "Link" + validation: + blank: "Link cannot be blank" + maximum: "Link must be shorter than %{count} characters" + invalid: "Format is invalid" + title: + label: "Section title" + validation: + blank: "Title cannot be blank" + maximum: "Title must be shorter than %{count} characters" about: header_link_text: "About" messages: diff --git a/lib/tasks/javascript.rake b/lib/tasks/javascript.rake index eb544934d67..c1bb3e74235 100644 --- a/lib/tasks/javascript.rake +++ b/lib/tasks/javascript.rake @@ -163,6 +163,16 @@ task "javascript:update_constants" => :environment do export const SEARCH_PRIORITIES = #{Searchable::PRIORITIES.to_json}; export const SEARCH_PHRASE_REGEXP = '#{Search::PHRASE_MATCH_REGEXP_PATTERN}'; + + export const SIDEBAR_URL = { + max_icon_length: #{SidebarUrl::MAX_ICON_LENGTH}, + max_name_length: #{SidebarUrl::MAX_NAME_LENGTH}, + max_value_length: #{SidebarUrl::MAX_VALUE_LENGTH} + } + + export const SIDEBAR_SECTION = { + max_title_length: #{SidebarSection::MAX_TITLE_LENGTH}, + } JS pretty_notifications = Notification.types.map { |n| " #{n[0]}: #{n[1]}," }.join("\n") diff --git a/spec/system/custom_sidebar_sections_spec.rb b/spec/system/custom_sidebar_sections_spec.rb index ce10268f17d..20bd1114eef 100644 --- a/spec/system/custom_sidebar_sections_spec.rb +++ b/spec/system/custom_sidebar_sections_spec.rb @@ -189,4 +189,24 @@ describe "Custom sidebar sections", type: :system, js: true do expect(page).not_to have_button("Edited public section") end + + it "validates custom section fields" do + visit("/latest") + sidebar.open_new_custom_section + + section_modal.fill_name("A" * (SidebarSection::MAX_TITLE_LENGTH + 1)) + section_modal.fill_link("B" * (SidebarUrl::MAX_NAME_LENGTH + 1), "/wrong-url") + + expect(page.find(".title.warning")).to have_content("Title must be shorter than 30 characters") + expect(page.find(".name.warning")).to have_content("Name must be shorter than 80 characters") + expect(page.find(".value.warning")).to have_content("Format is invalid") + + section_modal.fill_name("") + section_modal.fill_link("", "") + expect(page.find(".title.warning")).to have_content("Title cannot be blank") + expect(page.find(".name.warning")).to have_content("Name cannot be blank") + expect(page.find(".value.warning")).to have_content("Link cannot be blank") + + expect(section_modal).to have_disabled_save + end end