From f2059bf15fc66ec726befdff4cce6f16d06915d1 Mon Sep 17 00:00:00 2001 From: Keegan George Date: Tue, 10 Sep 2024 08:11:44 -0700 Subject: [PATCH] FIX: Form template limit validation (#28791) --- app/models/form_template.rb | 8 ++-- ...33304_remove_limits_from_form_templates.rb | 8 ++++ spec/models/form_template_spec.rb | 42 +++++++++++++++++++ .../admin/form_templates_controller_spec.rb | 13 ++---- 4 files changed, 57 insertions(+), 14 deletions(-) create mode 100644 db/migrate/20240906233304_remove_limits_from_form_templates.rb diff --git a/app/models/form_template.rb b/app/models/form_template.rb index 9aaaaba9998..21e19c85cea 100644 --- a/app/models/form_template.rb +++ b/app/models/form_template.rb @@ -5,14 +5,14 @@ class FormTemplate < ActiveRecord::Base presence: true, uniqueness: true, length: { - maximum: SiteSetting.max_form_template_title_length, + maximum: -> { SiteSetting.max_form_template_title_length }, } validates :template, presence: true, length: { - maximum: SiteSetting.max_form_template_content_length, + maximum: -> { SiteSetting.max_form_template_content_length }, } - validates_with FormTemplateYamlValidator + validates_with FormTemplateYamlValidator, if: ->(ft) { ft.template } has_many :category_form_templates, dependent: :destroy has_many :categories, through: :category_form_templates @@ -26,7 +26,7 @@ end # Table name: form_templates # # id :bigint not null, primary key -# name :string(100) not null +# name :string not null # template :text not null # created_at :datetime not null # updated_at :datetime not null diff --git a/db/migrate/20240906233304_remove_limits_from_form_templates.rb b/db/migrate/20240906233304_remove_limits_from_form_templates.rb new file mode 100644 index 00000000000..cc28f43a0cf --- /dev/null +++ b/db/migrate/20240906233304_remove_limits_from_form_templates.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class RemoveLimitsFromFormTemplates < ActiveRecord::Migration[7.1] + def change + change_column :form_templates, :name, :string, limit: nil + change_column :form_templates, :template, :text, limit: nil + end +end diff --git a/spec/models/form_template_spec.rb b/spec/models/form_template_spec.rb index ab1fc601aeb..85ae830857c 100644 --- a/spec/models/form_template_spec.rb +++ b/spec/models/form_template_spec.rb @@ -11,6 +11,48 @@ RSpec.describe FormTemplate, type: :model do expect(described_class.count).to eq(1) end + context "when length validations set in SiteSetting" do + before do + SiteSetting.max_form_template_content_length = 50 + SiteSetting.max_form_template_title_length = 5 + end + + it "can't exceed max title length" do + t = Fabricate.build(:form_template, name: "Bug Report", template: "- type: input\n id: name") + expect(t.save).to eq(false) + expect(t.errors.full_messages.first).to include( + "Name #{I18n.t("errors.messages.too_long", count: SiteSetting.max_form_template_title_length)}", + ) + end + + it "can't exceed max content length" do + t = + Fabricate.build( + :form_template, + name: "Bug", + template: "- type: input\n id: name-that-is-really-long-to-make-the-template-longer", + ) + expect(t.save).to eq(false) + expect(t.errors.full_messages.first).to include( + "Template #{I18n.t("errors.messages.too_long", count: SiteSetting.max_form_template_content_length)}", + ) + end + + it "should update validation limits when the site setting has been changed" do + SiteSetting.max_form_template_content_length = 100 + SiteSetting.max_form_template_title_length = 100 + + t = + Fabricate.build( + :form_template, + name: "Bug Report", + template: "- type: input\n id: name-that-is-really-long-to-make-the-template-longer", + ) + + expect(t.save).to eq(true) + end + end + it "can't have an invalid yaml template" do template = "- type: checkbox\nattributes; bad" t = Fabricate.build(:form_template, name: "Feature Request", template: template) diff --git a/spec/requests/admin/form_templates_controller_spec.rb b/spec/requests/admin/form_templates_controller_spec.rb index 6dab22ca978..f4e59f33944 100644 --- a/spec/requests/admin/form_templates_controller_spec.rb +++ b/spec/requests/admin/form_templates_controller_spec.rb @@ -3,12 +3,11 @@ RSpec.describe Admin::FormTemplatesController do fab!(:admin) fab!(:user) + fab!(:form_template) before { SiteSetting.experimental_form_templates = true } describe "#index" do - fab!(:form_template) - context "when logged in as an admin" do before { sign_in(admin) } @@ -46,8 +45,6 @@ RSpec.describe Admin::FormTemplatesController do end describe "#show" do - fab!(:form_template) - context "when logged in as an admin" do before { sign_in(admin) } @@ -102,8 +99,6 @@ RSpec.describe Admin::FormTemplatesController do end describe "#update" do - fab!(:form_template) - context "when logged in as an admin" do before { sign_in(admin) } @@ -111,13 +106,13 @@ RSpec.describe Admin::FormTemplatesController do put "/admin/customize/form-templates/#{form_template.id}.json", params: { id: form_template.id, - name: "Updated Template", + name: "Bugs", template: "- type: checkbox\n id: checkbox", } expect(response.status).to eq(200) form_template.reload - expect(form_template.name).to eq("Updated Template") + expect(form_template.name).to eq("Bugs") expect(form_template.template).to eq("- type: checkbox\n id: checkbox") end end @@ -145,8 +140,6 @@ RSpec.describe Admin::FormTemplatesController do end describe "#destroy" do - fab!(:form_template) - context "when logged in as an admin" do before { sign_in(admin) }