Skip to content
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 #4524 - The logged in user should be the default one in create new... #1263

Closed
wants to merge 1 commit into from

Conversation

martin-ducar-gd
Copy link

... hosts

@domcleal
Copy link
Contributor

domcleal commented Mar 3, 2014

[test]

@domcleal
Copy link
Contributor

domcleal commented Mar 4, 2014

I don't think this is the right place to fix it, since the code added in 452 is still present, albeit in a slightly different form. It's now here:

https://github.com/theforeman/foreman/blob/develop/app/models/host/managed.rb#L806-L809

It looks like #2179 broke this as it added validation of "owner types", but when you're using the form, no owner type is set.

I'd suggest changing set_default_user to something like:

def set_default_user
  return if self.owner_type.present? && !OWNER_TYPES.include?(self.owner_type)
  self.owner_type ||= 'User'
  self.owner ||= User.current
end

What do you think @elobato?

@domcleal
Copy link
Contributor

domcleal commented Mar 4, 2014

[test]

@dLobatog
Copy link
Member

dLobatog commented Mar 4, 2014

It looks good to me with the new changes, [test]

@martin-ducar-gd
Copy link
Author

Can you please take a look at the change and possibly merge it please?

@domcleal
Copy link
Contributor

domcleal commented Mar 5, 2014

@martin-ducar-gd thanks for the update, I'll give it a test soon and merge.

@domcleal
Copy link
Contributor

domcleal commented Mar 6, 2014

Merged as 19579f8 for Foreman 1.4.2. Thanks for the contribution @martin-ducar-gd!

@domcleal domcleal closed this Mar 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants