diff --git a/app/models/attachment.rb b/app/models/attachment.rb index 9807af4019c..4a9785670ac 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -29,6 +29,8 @@ class Attachment < ActiveRecord::Base validates_length_of :filename, :maximum => 255 validates_length_of :disk_filename, :maximum => 255 + validate :filesize_below_allowed_maximum + before_save :copy_file_to_destination after_destroy :delete_file_on_disk @@ -62,7 +64,7 @@ class Attachment < ActiveRecord::Base cattr_accessor :storage_path @@storage_path = Redmine::Configuration['attachments_storage_path'] || Rails.root.join('files').to_s - def validate + def filesize_below_allowed_maximum if self.filesize > Setting.attachment_max_size.to_i.kilobytes errors.add(:base, :too_long, :count => Setting.attachment_max_size.to_i.kilobytes) end diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index f7c5e2a4db6..1e08ee88731 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -45,6 +45,9 @@ class CustomField < ActiveRecord::Base validates_length_of :name, :maximum => 30 validates_inclusion_of :field_format, :in => Redmine::CustomFieldFormat.available_formats + validate :validate_presence_of_possible_values + validate :validate_default_value_in_translations + def initialize(attributes = nil) super self.possible_values ||= [] @@ -58,13 +61,15 @@ class CustomField < ActiveRecord::Base true end - def validate + def validate_presence_of_possible_values if self.field_format == "list" - errors.add(:possible_values, :blank) if self.possible_values.nil? || self.possible_values.empty? + errors.add(:possible_values, :blank) if self.possible_values.blank? errors.add(:possible_values, :invalid) unless self.possible_values.is_a? Array end + end - # validate default value in every translation available + # validate default value in every translation available + def validate_default_value_in_translations required_field = self.is_required self.is_required = false self.translated_locales.each do |locale| diff --git a/app/models/custom_value.rb b/app/models/custom_value.rb index f9dabaee910..402efeb6d14 100644 --- a/app/models/custom_value.rb +++ b/app/models/custom_value.rb @@ -16,6 +16,11 @@ class CustomValue < ActiveRecord::Base belongs_to :custom_field belongs_to :customized, :polymorphic => true + validate :validate_presence_of_required_value + validate :validate_format_of_value + validate :validate_type_of_value + validate :validate_length_of_value + after_initialize :set_default_value def set_default_value @@ -46,14 +51,19 @@ class CustomValue < ActiveRecord::Base end protected - def validate - if value.blank? - errors.add(:value, :blank) if custom_field.is_required? and value.blank? - else - errors.add(:value, :invalid) unless custom_field.regexp.blank? or value =~ Regexp.new(custom_field.regexp) - errors.add(:value, :too_short, :count => custom_field.min_length) if custom_field.min_length > 0 and value.length < custom_field.min_length - errors.add(:value, :too_long, :count => custom_field.max_length) if custom_field.max_length > 0 and value.length > custom_field.max_length + def validate_presence_of_required_value + errors.add(:value, :blank) if custom_field.is_required? && value.blank? + end + + def validate_format_of_value + if value.present? + errors.add(:value, :invalid) unless custom_field.regexp.blank? or value =~ Regexp.new(custom_field.regexp) + end + end + + def validate_type_of_value + if value.present? # Format specific validations case custom_field.field_format when 'int' @@ -67,4 +77,11 @@ protected end end end + + def validate_length_of_value + if value.present? + errors.add(:value, :too_short, :count => custom_field.min_length) if custom_field.min_length > 0 and value.length < custom_field.min_length + errors.add(:value, :too_long, :count => custom_field.max_length) if custom_field.max_length > 0 and value.length > custom_field.max_length + end + end end diff --git a/app/models/issue.rb b/app/models/issue.rb index b780b2a276e..0c763de75cd 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -69,6 +69,14 @@ class Issue < ActiveRecord::Base validates_inclusion_of :done_ratio, :in => 0..100 validates_numericality_of :estimated_hours, :allow_nil => true + validate :validate_format_of_due_date + validate :validate_start_date_before_due_date + validate :validate_start_date_before_soonest_start_date + validate :validate_fixed_version_is_assignable + validate :validate_fixed_version_is_still_open + validate :validate_enabled_tracker + validate :validate_correct_parent + scope :visible, lambda {|*args| { :include => :project, :conditions => Issue.visible_condition(args.first || User.current) } } @@ -315,34 +323,44 @@ class Issue < ActiveRecord::Base Setting.issue_done_ratio == 'issue_field' end - def validate + def validate_format_of_due_date if self.due_date.nil? && @attributes['due_date'] && !@attributes['due_date'].empty? errors.add :due_date, :not_a_date end + end + def validate_start_date_before_due_date if self.due_date and self.start_date and self.due_date < self.start_date errors.add :due_date, :greater_than_start_date end + end + def validate_start_date_before_soonest_start_date if start_date && soonest_start && start_date < soonest_start errors.add :start_date, :invalid end + end + def validate_fixed_version_is_assignable if fixed_version - if !assignable_versions.include?(fixed_version) - errors.add :fixed_version_id, :inclusion - elsif reopened? && fixed_version.closed? - errors.add_to_base I18n.t(:error_can_not_reopen_issue_on_closed_version) - end + errors.add :fixed_version_id, :inclusion unless assignable_versions.include?(fixed_version) end + end + def validate_fixed_version_is_still_open + if fixed_version && assignable_versions.include?(fixed_version) + errors.add_to_base I18n.t(:error_can_not_reopen_issue_on_closed_version) if reopened? && fixed_version.closed? + end + end + + def validate_enabled_tracker # Checks that the issue can not be added/moved to a disabled tracker if project && (tracker_id_changed? || project_id_changed?) - unless project.trackers.include?(tracker) - errors.add :tracker_id, :inclusion - end + errors.add :tracker_id, :inclusion unless project.trackers.include?(tracker) end + end + def validate_correct_parent # Checks parent issue assignment if @parent_issue if !Setting.cross_project_issue_relations? && @parent_issue.project_id != self.project_id diff --git a/app/models/issue_relation.rb b/app/models/issue_relation.rb index 68686f95a82..07711c5a567 100644 --- a/app/models/issue_relation.rb +++ b/app/models/issue_relation.rb @@ -40,11 +40,13 @@ class IssueRelation < ActiveRecord::Base validates_numericality_of :delay, :allow_nil => true validates_uniqueness_of :issue_to_id, :scope => :issue_from_id + validate :validate_sanity_of_relation + before_save :update_schedule attr_protected :issue_from_id, :issue_to_id - def validate + def validate_sanity_of_relation if issue_from && issue_to errors.add :issue_to_id, :invalid if issue_from_id == issue_to_id errors.add :issue_to_id, :not_same_project unless issue_from.project_id == issue_to.project_id || Setting.cross_project_issue_relations? diff --git a/app/models/member.rb b/app/models/member.rb index fef7e2e0385..7f423f861db 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -24,6 +24,8 @@ class Member < ActiveRecord::Base validates_presence_of :principal, :project validates_uniqueness_of :user_id, :scope => :project_id + validate :validate_presence_of_role + before_destroy :remove_from_category_assignments after_destroy :unwatch_from_permission_change @@ -81,7 +83,7 @@ class Member < ActiveRecord::Base protected - def validate + def validate_presence_of_role errors.add_on_empty :role if member_roles.empty? && roles.empty? || !member_roles.empty? && member_roles.all?(&:marked_for_destruction?) end diff --git a/app/models/member_role.rb b/app/models/member_role.rb index f9d4f9b276e..4811e4c1389 100644 --- a/app/models/member_role.rb +++ b/app/models/member_role.rb @@ -22,8 +22,9 @@ class MemberRole < ActiveRecord::Base attr_protected :member_id, :role_id validates_presence_of :role + validate :validate_project_member_role - def validate + def validate_project_member_role errors.add :role_id, :invalid if role && !role.member? end diff --git a/app/models/query.rb b/app/models/query.rb index 7995f0ff0b6..ca8ac903b2c 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -25,6 +25,8 @@ class Query < ActiveRecord::Base validates_presence_of :name, :on => :save validates_length_of :name, :maximum => 255 + validate :validate_filters + after_initialize :remember_project_scope @@operators = { "=" => :label_equals, @@ -91,14 +93,16 @@ class Query < ActiveRecord::Base @is_for_all = project.nil? end - def validate + def validate_filters + return unless filters + filters.each_key do |field| errors.add label_for(field), :blank unless # filter requires one or more values - (values_for(field) and !values_for(field).first.blank?) or + (values_for(field) and values_for(field).first.present?) or # filter doesn't require any value ["o", "c", "!*", "*", "t", "w"].include? operator_for(field) - end if filters + end end def editable_by?(user) diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb index ba96f356e90..417dfbfecef 100644 --- a/app/models/time_entry.rb +++ b/app/models/time_entry.rb @@ -33,6 +33,10 @@ class TimeEntry < ActiveRecord::Base validates_numericality_of :hours, :allow_nil => true, :message => :invalid validates_length_of :comments, :maximum => 255, :allow_nil => true + validate :validate_hours_are_in_range + validate :validate_project_is_set + validate :validate_consistency_of_issue_id + scope :visible, lambda {|*args| { :include => :project, :conditions => Project.allowed_to_condition(args.first || User.current, :view_time_entries) @@ -56,12 +60,6 @@ class TimeEntry < ActiveRecord::Base self.project ||= issue.project if issue end - def validate - errors.add :hours, :invalid if hours && (hours < 0 || hours >= 1000) - errors.add :project_id, :invalid if project.nil? - errors.add :issue_id, :invalid if (issue_id && !issue) || (issue && project!=issue.project) - end - def hours=(h) write_attribute :hours, (h.is_a?(String) ? (h.to_hours || h) : h) end @@ -106,4 +104,18 @@ class TimeEntry < ActiveRecord::Base end TimeEntry.maximum(:spent_on, :include => :project, :conditions => finder_conditions.conditions) end + +private + + def validate_hours_are_in_range + errors.add :hours, :invalid if hours && (hours < 0 || hours >= 1000) + end + + def validate_project_is_set + errors.add :project_id, :invalid if project.nil? + end + + def validate_consistency_of_issue_id + errors.add :issue_id, :invalid if (issue_id && !issue) || (issue && project!=issue.project) + end end diff --git a/app/models/user.rb b/app/models/user.rb index c10b33f9e2b..87707fe4031 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -85,6 +85,8 @@ class User < Principal validates_confirmation_of :password, :allow_nil => true validates_inclusion_of :mail_notification, :in => MAIL_NOTIFICATION_OPTIONS.collect(&:first), :allow_blank => true + validate :password_not_too_short + before_save :encrypt_password before_create :sanitize_mail_notification_setting before_destroy :delete_associated_public_queries @@ -604,10 +606,11 @@ class User < Principal protected - def validate - # Password length validation based on setting - if !password.nil? && password.size < Setting.password_min_length.to_i - errors.add(:password, :too_short, :count => Setting.password_min_length.to_i) + # Password length validation based on setting + def password_not_too_short + minimum_length = Setting.password_min_length.to_i + if password.present? && password.size < minimum_length + errors.add(:password, :too_short, :count => minimum_length) end end diff --git a/app/models/watcher.rb b/app/models/watcher.rb index 32a9ab6b67b..6370090cdd3 100644 --- a/app/models/watcher.rb +++ b/app/models/watcher.rb @@ -21,6 +21,8 @@ class Watcher < ActiveRecord::Base validates_presence_of :user validates_uniqueness_of :user_id, :scope => [:watchable_type, :watchable_id] + validate :validate_active_user + # Unwatch things that users are no longer allowed to view def self.prune(options={}) if options.has_key?(:user) @@ -36,8 +38,8 @@ class Watcher < ActiveRecord::Base protected - def validate - errors.add :user_id, :invalid unless user.nil? || user.active? + def validate_active_user + errors.add :user_id, :invalid if user.present? && !user.active? end private diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 12a6175b8e5..6864a72d403 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -38,6 +38,10 @@ class WikiPage < ActiveRecord::Base validates_uniqueness_of :title, :scope => :wiki_id, :case_sensitive => false validates_associated :content + validate :validate_consistency_of_parent_title + validate :validate_non_circular_dependency + validate :validate_same_project + after_initialize :check_and_mark_as_protected before_save :update_redirects before_destroy :remove_redirects @@ -168,9 +172,15 @@ class WikiPage < ActiveRecord::Base protected - def validate - errors.add(:parent_title, :invalid) if !@parent_title.blank? && parent.nil? + def validate_consistency_of_parent_title + errors.add(:parent_title, :invalid) if @parent_title.present? && parent.nil? + end + + def validate_non_circular_dependency errors.add(:parent_title, :circular_dependency) if parent && (parent == self || parent.ancestors.include?(self)) + end + + def validate_same_project errors.add(:parent_title, :not_same_project) if parent && (parent.wiki_id != wiki_id) end end diff --git a/vendor/plugins/classic_pagination/test/fixtures/company.rb b/vendor/plugins/classic_pagination/test/fixtures/company.rb index 39b55a7b315..665b0443051 100644 --- a/vendor/plugins/classic_pagination/test/fixtures/company.rb +++ b/vendor/plugins/classic_pagination/test/fixtures/company.rb @@ -4,7 +4,10 @@ class Company < ActiveRecord::Base set_sequence_name :companies_nonstd_seq validates_presence_of :name - def validate + + validate :rating_does_not_equal_two + + def rating_does_not_equal_two errors.add('rating', 'rating should not be 2') if rating == 2 end end \ No newline at end of file