From a1ede8f9b6a4adc8350c2a336d9154e92b6a749a Mon Sep 17 00:00:00 2001 From: kikito Date: Wed, 9 Sep 2015 15:52:43 +0200 Subject: [PATCH 01/12] Migrates adding limit to username and removing Comment.title --- .../20150909131133_set_username_limit.rb | 5 ++++ .../20150909135032_remove_comment_title.rb | 5 ++++ db/schema.rb | 25 +++++++++---------- 3 files changed, 22 insertions(+), 13 deletions(-) create mode 100644 db/migrate/20150909131133_set_username_limit.rb create mode 100644 db/migrate/20150909135032_remove_comment_title.rb diff --git a/db/migrate/20150909131133_set_username_limit.rb b/db/migrate/20150909131133_set_username_limit.rb new file mode 100644 index 000000000..4358a523b --- /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: 200 + 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/schema.rb b/db/schema.rb index f1b4dfe76..37e69cf39 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150908102936) do +ActiveRecord::Schema.define(version: 20150909135032) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -58,7 +58,6 @@ ActiveRecord::Schema.define(version: 20150908102936) 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 @@ -202,37 +201,37 @@ ActiveRecord::Schema.define(version: 20150908102936) do add_index "tags", ["name"], name: "index_tags_on_name", unique: true, using: :btree create_table "users", force: :cascade do |t| - t.string "email", default: "", null: false - t.string "encrypted_password", default: "", null: false + t.string "email", default: "", null: false + t.string "encrypted_password", default: "", null: false t.string "reset_password_token" t.datetime "reset_password_sent_at" t.datetime "remember_created_at" - t.integer "sign_in_count", default: 0, null: false + t.integer "sign_in_count", default: 0, null: false t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" t.string "current_sign_in_ip" t.string "last_sign_in_ip" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false t.string "confirmation_token" t.datetime "confirmed_at" t.datetime "confirmation_sent_at" t.string "unconfirmed_email" - t.boolean "email_on_debate_comment", default: false - t.boolean "email_on_comment_reply", default: false + t.boolean "email_on_debate_comment", default: false + t.boolean "email_on_comment_reply", default: false t.string "phone_number", limit: 30 t.string "official_position" - t.integer "official_level", default: 0 + t.integer "official_level", default: 0 t.datetime "hidden_at" t.string "sms_confirmation_code" - t.string "username" + t.string "username", limit: 200 t.string "document_number" t.string "document_type" t.datetime "residence_verified_at" t.datetime "letter_sent_at" t.string "email_verification_token" - t.integer "sms_confirmation_tries", default: 0 - t.integer "residence_verification_tries", default: 0 + t.integer "sms_confirmation_tries", default: 0 + t.integer "residence_verification_tries", default: 0 t.datetime "verified_at" t.string "unconfirmed_phone" t.string "confirmed_phone" From 4be475990c3fc21320289efbc81b1a51de43feb1 Mon Sep 17 00:00:00 2001 From: kikito Date: Wed, 9 Sep 2015 16:14:19 +0200 Subject: [PATCH 02/12] limits username length to 60 --- db/migrate/20150909131133_set_username_limit.rb | 2 +- db/schema.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/db/migrate/20150909131133_set_username_limit.rb b/db/migrate/20150909131133_set_username_limit.rb index 4358a523b..d7d17b4c2 100644 --- a/db/migrate/20150909131133_set_username_limit.rb +++ b/db/migrate/20150909131133_set_username_limit.rb @@ -1,5 +1,5 @@ class SetUsernameLimit < ActiveRecord::Migration def change - change_column :users, :username, :string, limit: 200 + change_column :users, :username, :string, limit: 60 end end diff --git a/db/schema.rb b/db/schema.rb index 37e69cf39..bb2fafddd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -224,7 +224,7 @@ ActiveRecord::Schema.define(version: 20150909135032) do t.integer "official_level", default: 0 t.datetime "hidden_at" t.string "sms_confirmation_code" - t.string "username", limit: 200 + t.string "username", limit: 60 t.string "document_number" t.string "document_type" t.datetime "residence_verified_at" From 7157d056afc0cd85b6eececeb15ce3c4469dade4 Mon Sep 17 00:00:00 2001 From: kikito Date: Wed, 9 Sep 2015 16:33:15 +0200 Subject: [PATCH 03/12] lowers organization.name limit to 60 in order to be coherent with username --- ...50909142534_set_organization_name_limit.rb | 5 ++++ db/schema.rb | 24 +++++++++---------- 2 files changed, 17 insertions(+), 12 deletions(-) create mode 100644 db/migrate/20150909142534_set_organization_name_limit.rb 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/schema.rb b/db/schema.rb index bb2fafddd..dd20a6347 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150909135032) do +ActiveRecord::Schema.define(version: 20150909142534) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -158,7 +158,7 @@ ActiveRecord::Schema.define(version: 20150909135032) 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 @@ -201,27 +201,27 @@ ActiveRecord::Schema.define(version: 20150909135032) do add_index "tags", ["name"], name: "index_tags_on_name", unique: true, using: :btree create_table "users", force: :cascade do |t| - t.string "email", default: "", null: false - t.string "encrypted_password", default: "", null: false + t.string "email", default: "", null: false + t.string "encrypted_password", default: "", null: false t.string "reset_password_token" t.datetime "reset_password_sent_at" t.datetime "remember_created_at" - t.integer "sign_in_count", default: 0, null: false + t.integer "sign_in_count", default: 0, null: false t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" t.string "current_sign_in_ip" t.string "last_sign_in_ip" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false t.string "confirmation_token" t.datetime "confirmed_at" t.datetime "confirmation_sent_at" t.string "unconfirmed_email" - t.boolean "email_on_debate_comment", default: false - t.boolean "email_on_comment_reply", default: false + t.boolean "email_on_debate_comment", default: false + t.boolean "email_on_comment_reply", default: false t.string "phone_number", limit: 30 t.string "official_position" - t.integer "official_level", default: 0 + t.integer "official_level", default: 0 t.datetime "hidden_at" t.string "sms_confirmation_code" t.string "username", limit: 60 @@ -230,8 +230,8 @@ ActiveRecord::Schema.define(version: 20150909135032) do t.datetime "residence_verified_at" t.datetime "letter_sent_at" t.string "email_verification_token" - t.integer "sms_confirmation_tries", default: 0 - t.integer "residence_verification_tries", default: 0 + t.integer "sms_confirmation_tries", default: 0 + t.integer "residence_verification_tries", default: 0 t.datetime "verified_at" t.string "unconfirmed_phone" t.string "confirmed_phone" From 0e96c82faa847bfe67a4b1880e9f883ec0cdc1e1 Mon Sep 17 00:00:00 2001 From: kikito Date: Wed, 9 Sep 2015 17:36:21 +0200 Subject: [PATCH 04/12] Sets the max length of tags to 40 chars in the db --- db/migrate/20150909153455_set_tag_name_limit.rb | 5 +++++ db/schema.rb | 8 ++++---- 2 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20150909153455_set_tag_name_limit.rb 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 dd20a6347..b23ad3c05 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150909142534) do +ActiveRecord::Schema.define(version: 20150909153455) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -193,9 +193,9 @@ ActiveRecord::Schema.define(version: 20150909142534) 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 From c750765bcaa958ff2cfaeb8d5dd83428ff6cb50d Mon Sep 17 00:00:00 2001 From: kikito Date: Wed, 9 Sep 2015 18:30:27 +0200 Subject: [PATCH 05/12] Modifies the tag sanitizer to truncate tags longer than 40 I could not make a nice ActAsTaggable error message, this is way faster --- lib/tag_sanitizer.rb | 3 ++- spec/lib/tag_sanitizer_spec.rb | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/tag_sanitizer.rb b/lib/tag_sanitizer.rb index f44b06adf..4d15a510b 100644 --- a/lib/tag_sanitizer.rb +++ b/lib/tag_sanitizer.rb @@ -1,4 +1,5 @@ class TagSanitizer + TAG_MAX_LENGTH = 40 DISALLOWED_STRINGS = %w(? < > = /) @@ -7,7 +8,7 @@ class TagSanitizer DISALLOWED_STRINGS.each do |s| tag.gsub!(s, '') end - tag + tag.truncate(TAG_MAX_LENGTH) end def sanitize_tag_list(tag_list) diff --git a/spec/lib/tag_sanitizer_spec.rb b/spec/lib/tag_sanitizer_spec.rb index e1fd6499b..dde5aa483 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 From 5e7a5b57de0f46b6e2655ca6f802b3ed616d0b17 Mon Sep 17 00:00:00 2001 From: kikito Date: Wed, 9 Sep 2015 18:32:36 +0200 Subject: [PATCH 06/12] Adds max lengths and validations in models --- app/models/comment.rb | 4 ++++ app/models/debate.rb | 9 +++++++-- app/models/organization.rb | 4 ++++ app/models/user.rb | 7 +++++-- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index efa9ec7a0..657994d8e 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -1,4 +1,6 @@ class Comment < ActiveRecord::Base + BODY_LENGTH = {minimum: 10, maximum: 2000} + acts_as_paranoid column: :hidden_at include ActsAsParanoidAliases acts_as_votable @@ -10,6 +12,8 @@ class Comment < ActiveRecord::Base validates :user, presence: true validates_inclusion_of :commentable_type, in: ["Debate"] + validates :body, length: BODY_LENGTH + belongs_to :commentable, -> { with_hidden }, polymorphic: true, counter_cache: true belongs_to :user, -> { with_hidden } diff --git a/app/models/debate.rb b/app/models/debate.rb index f3443dc2e..e976bcef9 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -1,7 +1,9 @@ require 'numeric' class Debate < ActiveRecord::Base + TITLE_LENGTH = {minimum: 10, maximum: Debate.columns.find { |c| c.name == 'title' }.limit || 80 } + DESCRIPTION_LENGTH = {minimum: 10, maximum: 6000} + apply_simple_captcha - TITLE_LENGTH = Debate.columns.find { |c| c.name == 'title' }.limit acts_as_votable acts_as_taggable @@ -16,6 +18,9 @@ class Debate < ActiveRecord::Base validates :description, presence: true validates :author, presence: true + validates :title, length: TITLE_LENGTH + validates :description, length: DESCRIPTION_LENGTH + validates :terms_of_service, acceptance: { allow_nil: false }, on: :create before_validation :sanitize_description @@ -148,6 +153,6 @@ class Debate < ActiveRecord::Base end def sanitize_tag_list - self.tag_list = TagSanitizer.new.sanitize_tag_list(self.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..4c068cc4a 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -1,7 +1,11 @@ class Organization < ActiveRecord::Base + NAME_LENGTH = {maximum: Organization.columns.find { |c| c.name == 'name' }.limit || 60} + belongs_to :user, touch: true validates :name, presence: true + validates :name, length: NAME_LENGTH + validates :name, uniqueness: true delegate :email, :phone_number, to: :user diff --git a/app/models/user.rb b/app/models/user.rb index a806de7c3..68274caa5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,9 +1,11 @@ class User < ActiveRecord::Base - include Verification - OMNIAUTH_EMAIL_PREFIX = 'omniauth@participacion' OMNIAUTH_EMAIL_REGEX = /\A#{OMNIAUTH_EMAIL_PREFIX}/ + USERNAME_LENGTH = {maximum: User.columns.find { |c| c.name == 'username' }.limit || 60} + + include Verification + apply_simple_captcha devise :database_authenticatable, :registerable, :confirmable, :recoverable, :rememberable, :trackable, :validatable, :omniauthable, :async @@ -23,6 +25,7 @@ class User < ActiveRecord::Base validates :username, presence: true, unless: :organization? validates :username, uniqueness: true, unless: :organization? + validates :username, length: 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 From ada03c8474dbd5eb78b51391442ae1cb4f24d221 Mon Sep 17 00:00:00 2001 From: kikito Date: Wed, 9 Sep 2015 18:34:26 +0200 Subject: [PATCH 07/12] Adds max lengths in views --- app/views/account/show.html.erb | 4 ++-- app/views/comments/_form.html.erb | 2 +- app/views/debates/_form.html.erb | 4 ++-- app/views/organizations/registrations/new.html.erb | 2 +- app/views/users/registrations/new.html.erb | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/views/account/show.html.erb b/app/views/account/show.html.erb index 5fe455a6f..3e42cc588 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_LENGTH[:maximum], 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_LENGTH[:maximum], placeholder: t("account.show.username_label") %> <% end %>
diff --git a/app/views/comments/_form.html.erb b/app/views/comments/_form.html.erb index 5c9f4eab2..7efe50110 100644 --- a/app/views/comments/_form.html.erb +++ b/app/views/comments/_form.html.erb @@ -2,7 +2,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_LENGTH[:maximum], 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..c320b33ca 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_LENGTH[:maximum], 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_LENGTH[:maximum], 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..7aa23187d 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_LENGTH[:maximum], 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..913ae4e01 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_LENGTH[:maximum], placeholder: t("devise_views.users.registrations.new.username_label"), label: false %> <%= f.email_field :email, placeholder: t("devise_views.users.registrations.new.email_label") %> From 86e1974a58ad19478645830d8ae4d625661253f5 Mon Sep 17 00:00:00 2001 From: kikito Date: Wed, 9 Sep 2015 18:36:03 +0200 Subject: [PATCH 08/12] monkeypatches foundation_rails_helper so it shows an error div after ckeditor --- config/initializers/foundation_rails_helper.rb | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 config/initializers/foundation_rails_helper.rb 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 From f93173ef4ae1772af345273be8e24e2917e3ae3e Mon Sep 17 00:00:00 2001 From: kikito Date: Wed, 9 Sep 2015 19:37:52 +0200 Subject: [PATCH 09/12] fixes typo --- app/models/debate.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/debate.rb b/app/models/debate.rb index 645d6146b..32467b409 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -156,6 +156,6 @@ class Debate < ActiveRecord::Base end def sanitize_tag_list - #self.tag_list = TagSanitizer.new.sanitize_tag_list(self.tag_list) + self.tag_list = TagSanitizer.new.sanitize_tag_list(self.tag_list) end end From 2d3015703d3b3afe358e49717539fb8471363611 Mon Sep 17 00:00:00 2001 From: kikito Date: Thu, 10 Sep 2015 13:02:21 +0200 Subject: [PATCH 10/12] replaces constants by class methods + private functions. Fixes broken tests --- app/models/comment.rb | 16 ++++++- app/models/debate.rb | 46 ++++++++++++++++----- app/models/organization.rb | 15 ++++++- app/models/user.rb | 26 ++++++++---- app/views/account/show.html.erb | 2 +- app/views/comments/_form.html.erb | 2 +- app/views/debates/_form.html.erb | 4 +- app/views/users/registrations/new.html.erb | 2 +- spec/controllers/debates_controller_spec.rb | 2 +- spec/features/debates_spec.rb | 20 ++++----- 10 files changed, 98 insertions(+), 37 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index 5997276d0..493dbeb1c 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -1,5 +1,4 @@ class Comment < ActiveRecord::Base - BODY_LENGTH = {minimum: 10, maximum: 2000} acts_as_paranoid column: :hidden_at include ActsAsParanoidAliases @@ -12,7 +11,7 @@ class Comment < ActiveRecord::Base validates :user, presence: true validates_inclusion_of :commentable_type, in: ["Debate"] - validates :body, length: BODY_LENGTH + validate :validate_body_length belongs_to :commentable, -> { with_hidden }, polymorphic: true, counter_cache: true belongs_to :user, -> { with_hidden } @@ -97,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 32467b409..fdf9cb6b9 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -1,8 +1,5 @@ require 'numeric' class Debate < ActiveRecord::Base - TITLE_LENGTH = {minimum: 10, maximum: Debate.columns.find { |c| c.name == 'title' }.limit || 80 } - DESCRIPTION_LENGTH = {minimum: 10, maximum: 6000} - apply_simple_captcha acts_as_votable @@ -18,8 +15,8 @@ class Debate < ActiveRecord::Base validates :description, presence: true validates :author, presence: true - validates :title, length: TITLE_LENGTH - validates :description, length: DESCRIPTION_LENGTH + validate :validate_title_length + validate :validate_description_length validates :terms_of_service, acceptance: { allow_nil: false }, on: :create @@ -149,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 4c068cc4a..e52ce0ae3 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -4,8 +4,8 @@ class Organization < ActiveRecord::Base belongs_to :user, touch: true validates :name, presence: true - validates :name, length: NAME_LENGTH validates :name, uniqueness: true + validate :validate_name_length delegate :email, :phone_number, to: :user @@ -35,4 +35,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 68274caa5..d674e4aeb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2,8 +2,6 @@ class User < ActiveRecord::Base OMNIAUTH_EMAIL_PREFIX = 'omniauth@participacion' OMNIAUTH_EMAIL_REGEX = /\A#{OMNIAUTH_EMAIL_PREFIX}/ - USERNAME_LENGTH = {maximum: User.columns.find { |c| c.name == 'username' }.limit || 60} - include Verification apply_simple_captcha @@ -25,7 +23,8 @@ class User < ActiveRecord::Base validates :username, presence: true, unless: :organization? validates :username, uniqueness: true, unless: :organization? - validates :username, length: USERNAME_LENGTH + 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 @@ -129,13 +128,26 @@ class User < ActiveRecord::Base Comment.hide_all comments_ids 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) end + def self.search(term) + term.present? ? where("email = ? OR username ILIKE ?", term, "%#{term}%") : none + end + + def self.username_max_length + @@username_max_length ||= self.columns.find { |c| c.name == 'username' }.limit + 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 3e42cc588..1b2219ecb 100644 --- a/app/views/account/show.html.erb +++ b/app/views/account/show.html.erb @@ -37,7 +37,7 @@ <%= f.text_field :phone_number, placeholder: t("account.show.phone_number_label") %> <% else %> - <%= f.text_field :username, maxlength: User::USERNAME_LENGTH[:maximum], 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 cde2bf824..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}", maxlength: Comment::BODY_LENGTH[:maximum], 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 c320b33ca..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[:maximum], 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, maxlength: Debate::DESCRIPTION_LENGTH[:maximum], 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/users/registrations/new.html.erb b/app/views/users/registrations/new.html.erb index 913ae4e01..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, maxlength: User::USERNAME_LENGTH[:maximum], 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/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 330508e88..800a624d6 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 From 95fc7ea9490fccf45a7871cb68b517cff16d301c Mon Sep 17 00:00:00 2001 From: kikito Date: Thu, 10 Sep 2015 18:07:30 +0200 Subject: [PATCH 11/12] remove constants / replace by class methods --- app/models/organization.rb | 2 -- lib/tag_sanitizer.rb | 8 +++++--- spec/lib/tag_sanitizer_spec.rb | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/organization.rb b/app/models/organization.rb index e52ce0ae3..dc340b08c 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -1,6 +1,4 @@ class Organization < ActiveRecord::Base - NAME_LENGTH = {maximum: Organization.columns.find { |c| c.name == 'name' }.limit || 60} - belongs_to :user, touch: true validates :name, presence: true diff --git a/lib/tag_sanitizer.rb b/lib/tag_sanitizer.rb index 4d15a510b..643bca19c 100644 --- a/lib/tag_sanitizer.rb +++ b/lib/tag_sanitizer.rb @@ -1,6 +1,4 @@ class TagSanitizer - TAG_MAX_LENGTH = 40 - DISALLOWED_STRINGS = %w(? < > = /) def sanitize_tag(tag) @@ -8,11 +6,15 @@ class TagSanitizer DISALLOWED_STRINGS.each do |s| tag.gsub!(s, '') end - tag.truncate(TAG_MAX_LENGTH) + 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/lib/tag_sanitizer_spec.rb b/spec/lib/tag_sanitizer_spec.rb index dde5aa483..f33b771b8 100644 --- a/spec/lib/tag_sanitizer_spec.rb +++ b/spec/lib/tag_sanitizer_spec.rb @@ -14,9 +14,9 @@ describe TagSanitizer do end it 'sets up a max length for each tag' do - long_tag = '1' * (TagSanitizer::TAG_MAX_LENGTH + 100) + long_tag = '1' * (TagSanitizer.tag_max_length + 100) - expect(subject.sanitize_tag(long_tag).size).to eq(TagSanitizer::TAG_MAX_LENGTH) + expect(subject.sanitize_tag(long_tag).size).to eq(TagSanitizer.tag_max_length) end end From be2a0987069aba57f7f15261df49f7cd0712c710 Mon Sep 17 00:00:00 2001 From: kikito Date: Thu, 10 Sep 2015 18:14:59 +0200 Subject: [PATCH 12/12] removes missing constant in views, replacing by class method --- app/views/account/show.html.erb | 2 +- app/views/organizations/registrations/new.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/account/show.html.erb b/app/views/account/show.html.erb index 1b2219ecb..413a0e892 100644 --- a/app/views/account/show.html.erb +++ b/app/views/account/show.html.erb @@ -32,7 +32,7 @@
<% if @account.organization? %> <%= f.fields_for :organization do |fo| %> - <%= fo.text_field :name, autofocus: true, maxlength: Organization::NAME_LENGTH[:maximum], 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") %> diff --git a/app/views/organizations/registrations/new.html.erb b/app/views/organizations/registrations/new.html.erb index 7aa23187d..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, maxlength: Organization::NAME_LENGTH[:maximum], 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") %>