-
Notifications
You must be signed in to change notification settings - Fork 983
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 #3752 - move data population from migrations into seed script #1065
Conversation
Looks like I've got some tests to fix, but otherwise open for review. Probably won't get to that until next week now. |
|
||
def up | ||
CONFIG_RENAMES.each do |old,new| | ||
ConfigTemplate.find_by_name(old).try(:update_attributes, :name => new) |
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.
I assume that try
does not fail when update fails. Nice.
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.
update_attributes will simply return false if update fails
Thanks for the previous review comments, PR's updated for review. |
LGTM Nice work. |
Thanks, rebased so hopefully the tests pass. |
@@ -12,7 +12,7 @@ def self.up | |||
updated = [] | |||
email = SETTINGS[:administrator] || "root@#{Facter.domain}" | |||
owner = User.find_by_mail email | |||
owner ||= User.find_or_create_by_login(:login => "admin", :admin => true, :firstname => "Admin", :lastname => "User", :mail => email) | |||
owner ||= User.where(:admin => true).first |
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.
there is a User.admin scope
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.
nm, i see you re-defined user.
This looks more severe than it is I think.
I think this is more reliable than the DB migrations. I was beginning to implement improved admin account initialisation and the use of DB migrations was hampering me, as they have to support both upgrades and initial installations - they're quite fragile as a result.