diff --git a/app/models/comment.rb b/app/models/comment.rb index 233736824..493dbeb1c 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -1,4 +1,5 @@ class Comment < ActiveRecord::Base + acts_as_paranoid column: :hidden_at include ActsAsParanoidAliases acts_as_votable @@ -10,6 +11,8 @@ class Comment < ActiveRecord::Base validates :user, presence: true validates_inclusion_of :commentable_type, in: ["Debate"] + validate :validate_body_length + belongs_to :commentable, -> { with_hidden }, polymorphic: true, counter_cache: true belongs_to :user, -> { with_hidden } @@ -93,4 +96,17 @@ class Comment < ActiveRecord::Base self.commentable.try(:after_commented) end + def self.body_max_length + 1000 + end + + private + + def validate_body_length + validator = ActiveModel::Validations::LengthValidator.new( + attributes: :body, + maximum: Comment.body_max_length) + validator.validate(self) + end + end diff --git a/app/models/debate.rb b/app/models/debate.rb index 4dfb7b51f..fdf9cb6b9 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -1,7 +1,6 @@ require 'numeric' class Debate < ActiveRecord::Base apply_simple_captcha - TITLE_LENGTH = Debate.columns.find { |c| c.name == 'title' }.limit acts_as_votable acts_as_taggable @@ -16,6 +15,9 @@ class Debate < ActiveRecord::Base validates :description, presence: true validates :author, presence: true + validate :validate_title_length + validate :validate_description_length + validates :terms_of_service, acceptance: { allow_nil: false }, on: :create before_validation :sanitize_description @@ -144,13 +146,40 @@ class Debate < ActiveRecord::Base cached_votes_up/flags_count.to_f < 5 end + def self.title_max_length + @@title_max_length ||= self.columns.find { |c| c.name == 'title' }.limit + end + + def self.description_max_length + 6000 + end + protected - def sanitize_description - self.description = WYSIWYGSanitizer.new.sanitize(description) - end + def sanitize_description + self.description = WYSIWYGSanitizer.new.sanitize(description) + end + + def sanitize_tag_list + self.tag_list = TagSanitizer.new.sanitize_tag_list(self.tag_list) + end + + private + + def validate_description_length + validator = ActiveModel::Validations::LengthValidator.new( + attributes: :description, + minimum: 10, + maximum: Debate.description_max_length) + validator.validate(self) + end + + def validate_title_length + validator = ActiveModel::Validations::LengthValidator.new( + attributes: :title, + minimum: 4, + maximum: Debate.title_max_length) + validator.validate(self) + end - def sanitize_tag_list - self.tag_list = TagSanitizer.new.sanitize_tag_list(self.tag_list) - end end diff --git a/app/models/organization.rb b/app/models/organization.rb index 2962724a6..dc340b08c 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -2,6 +2,8 @@ class Organization < ActiveRecord::Base belongs_to :user, touch: true validates :name, presence: true + validates :name, uniqueness: true + validate :validate_name_length delegate :email, :phone_number, to: :user @@ -31,4 +33,17 @@ class Organization < ActiveRecord::Base text.present? ? joins(:user).where("users.email = ? OR users.phone_number = ? OR organizations.name ILIKE ?", text, text, "%#{text}%") : none end + def self.name_max_length + @@name_max_length ||= self.columns.find { |c| c.name == 'name' }.limit + end + + private + + def validate_name_length + validator = ActiveModel::Validations::LengthValidator.new( + attributes: :name, + maximum: Organization.name_max_length) + validator.validate(self) + end + end diff --git a/app/models/user.rb b/app/models/user.rb index f0cf9d6fb..1099bc6c4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,9 +1,9 @@ class User < ActiveRecord::Base - include Verification - OMNIAUTH_EMAIL_PREFIX = 'omniauth@participacion' OMNIAUTH_EMAIL_REGEX = /\A#{OMNIAUTH_EMAIL_PREFIX}/ + include Verification + apply_simple_captcha devise :database_authenticatable, :registerable, :confirmable, :recoverable, :rememberable, :trackable, :validatable, :omniauthable, :async @@ -23,6 +23,8 @@ class User < ActiveRecord::Base validates :username, presence: true, unless: :organization? validates :username, uniqueness: true, unless: :organization? + validate :validate_username_length + validates :official_level, inclusion: {in: 0..5} validates_format_of :email, without: OMNIAUTH_EMAIL_REGEX, on: :update validates :terms_of_service, acceptance: { allow_nil: false }, on: :create @@ -126,17 +128,30 @@ class User < ActiveRecord::Base Comment.hide_all comments_ids end + def email_provided? + !!(email && email !~ OMNIAUTH_EMAIL_REGEX) || + !!(unconfirmed_email && unconfirmed_email !~ OMNIAUTH_EMAIL_REGEX) + end + def self.search(term) term.present? ? where("email = ? OR username ILIKE ?", term, "%#{term}%") : none end - def email_provided? - !!(email && email !~ OMNIAUTH_EMAIL_REGEX) || - !!(unconfirmed_email && unconfirmed_email !~ OMNIAUTH_EMAIL_REGEX) + def self.username_max_length + @@username_max_length ||= self.columns.find { |c| c.name == 'username' }.limit end def show_welcome_screen? sign_in_count == 1 && unverified? && !organization end + private + + def validate_username_length + validator = ActiveModel::Validations::LengthValidator.new( + attributes: :username, + maximum: User.username_max_length) + validator.validate(self) + end + end diff --git a/app/views/account/show.html.erb b/app/views/account/show.html.erb index 5fe455a6f..413a0e892 100644 --- a/app/views/account/show.html.erb +++ b/app/views/account/show.html.erb @@ -32,12 +32,12 @@
<% if @account.organization? %> <%= f.fields_for :organization do |fo| %> - <%= fo.text_field :name, autofocus: true, placeholder: t("account.show.organization_name_label") %> + <%= fo.text_field :name, autofocus: true, maxlength: Organization.name_max_length, placeholder: t("account.show.organization_name_label") %> <% end %> <%= f.text_field :phone_number, placeholder: t("account.show.phone_number_label") %> <% else %> - <%= f.text_field :username, placeholder: t("account.show.username_label") %> + <%= f.text_field :username, maxlength: User.username_max_length, placeholder: t("account.show.username_label") %> <% end %>
diff --git a/app/views/comments/_form.html.erb b/app/views/comments/_form.html.erb index 3704b2fa1..d14f582e3 100644 --- a/app/views/comments/_form.html.erb +++ b/app/views/comments/_form.html.erb @@ -3,7 +3,7 @@
> <%= form_for [commentable, Comment.new], remote: true do |f| %> <%= label_tag "comment-body-#{css_id}", t("comments.form.leave_comment") %> - <%= f.text_area :body, id: "comment-body-#{css_id}", label: false %> + <%= f.text_area :body, id: "comment-body-#{css_id}", maxlength: Comment.body_max_length, label: false %> <%= f.hidden_field :commentable_type, value: commentable.class.name %> <%= f.hidden_field :commentable_id, value: commentable.id %> <%= f.hidden_field :parent_id, value: parent_id %> diff --git a/app/views/debates/_form.html.erb b/app/views/debates/_form.html.erb index e9d750c8a..870f5081d 100644 --- a/app/views/debates/_form.html.erb +++ b/app/views/debates/_form.html.erb @@ -4,12 +4,12 @@
<%= f.label :title, t("debates.form.debate_title") %> - <%= f.text_field :title, maxlength: Debate::TITLE_LENGTH, placeholder: t("debates.form.debate_title"), label: false %> + <%= f.text_field :title, maxlength: Debate.title_max_length, placeholder: t("debates.form.debate_title"), label: false %>
<%= f.label :description, t("debates.form.debate_text") %> - <%= f.cktext_area :description, ckeditor: { language: I18n.locale }, label: false %> + <%= f.cktext_area :description, maxlength: Debate.description_max_length, ckeditor: { language: I18n.locale }, label: false %>
diff --git a/app/views/organizations/registrations/new.html.erb b/app/views/organizations/registrations/new.html.erb index ce7f5a0d2..1b95a2d5d 100644 --- a/app/views/organizations/registrations/new.html.erb +++ b/app/views/organizations/registrations/new.html.erb @@ -6,7 +6,7 @@
<%= f.fields_for :organization do |fo| %> - <%= fo.text_field :name, autofocus: true, placeholder: t("devise_views.organizations.registrations.new.organization_name_label") %> + <%= fo.text_field :name, autofocus: true, maxlength: Organization.name_max_length, placeholder: t("devise_views.organizations.registrations.new.organization_name_label") %> <% end %> <%= f.email_field :email, placeholder: t("devise_views.organizations.registrations.new.email_label") %> diff --git a/app/views/users/registrations/new.html.erb b/app/views/users/registrations/new.html.erb index cf3d82c6a..19d4dc5d7 100644 --- a/app/views/users/registrations/new.html.erb +++ b/app/views/users/registrations/new.html.erb @@ -15,7 +15,7 @@ <%= f.label t("devise_views.users.registrations.new.username_label") %> <%= t("devise_views.users.registrations.new.username_note") %> - <%= f.text_field :username, placeholder: t("devise_views.users.registrations.new.username_label"), label: false %> + <%= f.text_field :username, maxlength: User.username_max_length, placeholder: t("devise_views.users.registrations.new.username_label"), label: false %> <%= f.email_field :email, placeholder: t("devise_views.users.registrations.new.email_label") %> diff --git a/config/initializers/foundation_rails_helper.rb b/config/initializers/foundation_rails_helper.rb new file mode 100644 index 000000000..ff5ae4618 --- /dev/null +++ b/config/initializers/foundation_rails_helper.rb @@ -0,0 +1,9 @@ +module FoundationRailsHelper + class FormBuilder < ActionView::Helpers::FormBuilder + def cktext_area(attribute, options) + field(attribute, options) do |opts| + super(attribute, opts) + end + end + end +end diff --git a/db/migrate/20150909131133_set_username_limit.rb b/db/migrate/20150909131133_set_username_limit.rb new file mode 100644 index 000000000..d7d17b4c2 --- /dev/null +++ b/db/migrate/20150909131133_set_username_limit.rb @@ -0,0 +1,5 @@ +class SetUsernameLimit < ActiveRecord::Migration + def change + change_column :users, :username, :string, limit: 60 + end +end diff --git a/db/migrate/20150909135032_remove_comment_title.rb b/db/migrate/20150909135032_remove_comment_title.rb new file mode 100644 index 000000000..632a86a4f --- /dev/null +++ b/db/migrate/20150909135032_remove_comment_title.rb @@ -0,0 +1,5 @@ +class RemoveCommentTitle < ActiveRecord::Migration + def change + remove_column :comments, :title + end +end diff --git a/db/migrate/20150909142534_set_organization_name_limit.rb b/db/migrate/20150909142534_set_organization_name_limit.rb new file mode 100644 index 000000000..b7a8f3a63 --- /dev/null +++ b/db/migrate/20150909142534_set_organization_name_limit.rb @@ -0,0 +1,5 @@ +class SetOrganizationNameLimit < ActiveRecord::Migration + def change + change_column :organizations, :name, :string, limit: 60 + end +end diff --git a/db/migrate/20150909153455_set_tag_name_limit.rb b/db/migrate/20150909153455_set_tag_name_limit.rb new file mode 100644 index 000000000..d4e43dcb8 --- /dev/null +++ b/db/migrate/20150909153455_set_tag_name_limit.rb @@ -0,0 +1,5 @@ +class SetTagNameLimit < ActiveRecord::Migration + def change + change_column :tags, :name, :string, limit: 40 + end +end diff --git a/db/schema.rb b/db/schema.rb index ef952af3b..a9ca5d4b2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -58,7 +58,6 @@ ActiveRecord::Schema.define(version: 20150910092713) do create_table "comments", force: :cascade do |t| t.integer "commentable_id" t.string "commentable_type" - t.string "title" t.text "body" t.string "subject" t.integer "user_id", null: false @@ -159,7 +158,7 @@ ActiveRecord::Schema.define(version: 20150910092713) do create_table "organizations", force: :cascade do |t| t.integer "user_id" - t.string "name", limit: 80 + t.string "name", limit: 60 t.datetime "verified_at" t.datetime "rejected_at" end @@ -194,9 +193,9 @@ ActiveRecord::Schema.define(version: 20150910092713) do add_index "taggings", ["taggable_id", "taggable_type", "context"], name: "index_taggings_on_taggable_id_and_taggable_type_and_context", using: :btree create_table "tags", force: :cascade do |t| - t.string "name" - t.integer "taggings_count", default: 0 - t.boolean "featured", default: false + t.string "name", limit: 40 + t.integer "taggings_count", default: 0 + t.boolean "featured", default: false end add_index "tags", ["name"], name: "index_tags_on_name", unique: true, using: :btree @@ -225,7 +224,7 @@ ActiveRecord::Schema.define(version: 20150910092713) do t.integer "official_level", default: 0 t.datetime "hidden_at" t.string "sms_confirmation_code" - t.string "username" + t.string "username", limit: 60 t.string "document_number" t.string "document_type" t.datetime "residence_verified_at" diff --git a/lib/tag_sanitizer.rb b/lib/tag_sanitizer.rb index f44b06adf..643bca19c 100644 --- a/lib/tag_sanitizer.rb +++ b/lib/tag_sanitizer.rb @@ -1,5 +1,4 @@ class TagSanitizer - DISALLOWED_STRINGS = %w(? < > = /) def sanitize_tag(tag) @@ -7,11 +6,15 @@ class TagSanitizer DISALLOWED_STRINGS.each do |s| tag.gsub!(s, '') end - tag + tag.truncate(TagSanitizer.tag_max_length) end def sanitize_tag_list(tag_list) tag_list.map { |tag| sanitize_tag(tag) } end + def self.tag_max_length + 40 + end + end diff --git a/spec/controllers/debates_controller_spec.rb b/spec/controllers/debates_controller_spec.rb index f87635a6b..b5d46abac 100644 --- a/spec/controllers/debates_controller_spec.rb +++ b/spec/controllers/debates_controller_spec.rb @@ -16,7 +16,7 @@ describe DebatesController do sign_in create(:user) - post :create, debate: { title: 'foo', description: 'foo bar', terms_of_service: 1 } + post :create, debate: { title: 'A sample debate', description: 'this is a sample debate', terms_of_service: 1 } expect(Ahoy::Event.where(name: :debate_created).count).to eq 1 expect(Ahoy::Event.last.properties['debate_id']).to eq Debate.last.id end diff --git a/spec/features/debates_spec.rb b/spec/features/debates_spec.rb index 277db20b0..edb5c2725 100644 --- a/spec/features/debates_spec.rb +++ b/spec/features/debates_spec.rb @@ -117,7 +117,7 @@ feature 'Debates' do click_button "Start a debate" expect(page).to_not have_content "Debate was successfully created." - expect(page).to have_content "1 error" + expect(page).to have_content "error" within(".tags") do expect(page).to have_content featured_tag.name expect(page).to_not have_content tag.name @@ -138,7 +138,7 @@ feature 'Debates' do login_as(author) visit new_debate_path - fill_in 'debate_title', with: 'A test' + fill_in 'debate_title', with: 'Testing an attack' fill_in 'debate_description', with: '

This is

' fill_in 'debate_captcha', with: correct_captcha_text check 'debate_terms_of_service' @@ -146,7 +146,7 @@ feature 'Debates' do click_button 'Start a debate' expect(page).to have_content 'Debate was successfully created.' - expect(page).to have_content 'A test' + expect(page).to have_content 'Testing an attack' expect(page.html).to include '

This is alert("an attack");

' expect(page.html).to_not include '' expect(page.html).to_not include '<p>This is' @@ -186,8 +186,8 @@ feature 'Debates' do scenario 'using dangerous strings' do visit new_debate_path - fill_in 'debate_title', with: 'A test' - fill_in 'debate_description', with: 'A test' + fill_in 'debate_title', with: 'A test of dangerous strings' + fill_in 'debate_description', with: 'A description suitable for this test' fill_in 'debate_captcha', with: correct_captcha_text check 'debate_terms_of_service' @@ -233,14 +233,14 @@ feature 'Debates' do expect(current_path).to eq(edit_debate_path(debate)) fill_in 'debate_title', with: "End child poverty" - fill_in 'debate_description', with: "Let's..." + fill_in 'debate_description', with: "Let's do something to end child poverty" fill_in 'debate_captcha', with: correct_captcha_text click_button "Save changes" expect(page).to have_content "Debate was successfully updated." expect(page).to have_content "End child poverty" - expect(page).to have_content "Let's..." + expect(page).to have_content "Let's do something to end child poverty" end scenario 'Errors on update' do @@ -266,7 +266,7 @@ feature 'Debates' do click_button "Save changes" expect(page).to_not have_content "Debate was successfully updated." - expect(page).to have_content "1 error" + expect(page).to have_content "error" fill_in 'debate_captcha', with: correct_captcha_text click_button "Save changes" @@ -288,7 +288,7 @@ feature 'Debates' do click_button "Save changes" expect(page).to_not have_content "Debate was successfully updated." - expect(page).to have_content "1 error" + expect(page).to have_content "error" within(".tags") do expect(page).to have_content featured_tag.name expect(page).to_not have_content tag.name @@ -438,7 +438,7 @@ feature 'Debates' do scenario 'Debate index search' do debate1 = create(:debate, title: "Show me what you got") debate2 = create(:debate, title: "Get Schwifty") - debate3 = create(:debate, description: "Unity") + debate3 = create(:debate) debate4 = create(:debate, description: "Schwifty in here") visit debates_path diff --git a/spec/lib/tag_sanitizer_spec.rb b/spec/lib/tag_sanitizer_spec.rb index e1fd6499b..f33b771b8 100644 --- a/spec/lib/tag_sanitizer_spec.rb +++ b/spec/lib/tag_sanitizer_spec.rb @@ -12,6 +12,12 @@ describe TagSanitizer do it 'filters out dangerous strings' do expect(subject.sanitize_tag('user_id=1')).to eq('user_id1') end + + it 'sets up a max length for each tag' do + long_tag = '1' * (TagSanitizer.tag_max_length + 100) + + expect(subject.sanitize_tag(long_tag).size).to eq(TagSanitizer.tag_max_length) + end end describe '#sanitize_tag_list' do