Merge pull request #450 from AyuntamientoMadrid/field-limits-446

Poner tamaño máximo a varios campos

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

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

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

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

app/views/account/show.html.erb

@@ -32,12 +32,12 @@
             <div class="small-12 medium-4">
           <% 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 %>
           </div>
 

app/views/comments/_form.html.erb

@@ -3,7 +3,7 @@
   <div id="js-comment-form-<%= css_id %>" <%= "style='display:none'".html_safe if toggeable %>>
     <%= 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 %>

app/views/debates/_form.html.erb

@@ -4,12 +4,12 @@
   <div class="row">
     <div class="small-12 column">
       <%= 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 %>
     </div>
 
     <div class="ckeditor small-12 column">
       <%= 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 %>
     </div>
 
     <div class="small-12 column">

app/views/organizations/registrations/new.html.erb

@@ -6,7 +6,7 @@
      <div class="small-12 column">
 
       <%= 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") %>

app/views/users/registrations/new.html.erb

@@ -15,7 +15,7 @@
 
       <%= f.label t("devise_views.users.registrations.new.username_label") %>
       <span class="note"><%= t("devise_views.users.registrations.new.username_note") %></span>
-      <%= 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") %>
 

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

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

db/migrate/20150909135032_remove_comment_title.rb

@@ -0,0 +1,5 @@
+class RemoveCommentTitle < ActiveRecord::Migration
+  def change
+    remove_column :comments, :title
+  end
+end

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

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

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"

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

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

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: '<p>This is <script>alert("an attack");</script></p>'
     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 '<p>This is alert("an attack");</p>'
     expect(page.html).to_not include '<script>alert("an attack");</script>'
     expect(page.html).to_not include '&lt;p&gt;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

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