From 2d3015703d3b3afe358e49717539fb8471363611 Mon Sep 17 00:00:00 2001 From: kikito Date: Thu, 10 Sep 2015 13:02:21 +0200 Subject: [PATCH] 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