-
Notifications
You must be signed in to change notification settings - Fork 987
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #7743 - ensure name is unique in scope of major and minor #1885
Conversation
assert_equal(operatingsystems(:centos5_3).name, operatingsystem.name) | ||
assert_equal(operatingsystems(:centos5_3).major, operatingsystem.major) | ||
assert_equal(operatingsystems(:centos5_3).minor, operatingsystem.minor) | ||
refute operatingsystem.valid? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe use FactoryGirl to create two operating systems that are tests?
If it's planned to remove all or most of the fixtures it would be easier if new tests avoided them
tested, works for me 👍 |
@@ -29,10 +29,11 @@ class Operatingsystem < ActiveRecord::Base | |||
has_many :trends, :as => :trendable, :class_name => "ForemanTrend" | |||
attr_name :to_label | |||
validates :minor, :numericality => {:greater_than_or_equal_to => 0}, :allow_nil => true, :allow_blank => true | |||
validates :name, :presence => true, :format => {:with => /\A(\S+)\Z/, :message => N_("can't contain white spaces.")} | |||
validates :name, :presence => true, :format => {:with => /\A(\S+)\Z/, :message => N_("can't contain white spaces.")}, | |||
:uniqueness => { :scope => [:major, :minor], :message => N_("Operating system version exists")} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: "Operating system version already exists" is better, IMHO
Same concern as expressed in #1962 : Need a migration to handle existing duplicate names. |
@shlomizadok please rebase |
Merged as f320bbc, thanks @shlomizadok! |
No description provided.