Fixes #14013 - add a setting for default owner of new hosts

app/helpers/application_helper.rb

@@ -382,6 +382,23 @@ def cancel_path_with_index_search
     prev_controller_url
   end
 
+  # creates a data set for editable select-optgroup. each element in the 'groups' array is a hash represents a group with its children.
+  # e.g - {:name => _("Users"), :class => 'user', :scope => 'visible', :value_method => 'id_and_type', :text_method => 'login'}
+  # :name -> group's name, :scope -> scoped method (e.g 'all' or another predefined scope),
+  # :value_method -> value in params, and text_method -> the shown text in the select element.
+
+  def editable_select_optgroup(groups, options = {})
+    select = []
+    select.push(nil => options[:include_blank]) if options[:include_blank].present?
+    groups.each do |group|
+      klass = group[:class].classify.constantize
+      scope = group[:scope]
+      children = Hash[klass.send(scope).map {|obj| [obj.send(group[:value_method]), obj.send(group[:text_method])]}]
+      select.push(:text => group[:name], :children => children)
+    end
+    select
+  end
+
   private
 
   def edit_inline(object, property, options = {})

app/models/concerns/hostext/ownership.rb

@@ -0,0 +1,69 @@
+module Hostext
+  module Ownership
+    extend ActiveSupport::Concern
+
+    included do
+      OWNER_TYPES = %w(User Usergroup)
+      belongs_to :owner, :polymorphic => true
+
+      before_validation :set_default_user
+      validates :owner_type, :inclusion =>
+                               {:in => OWNER_TYPES,
+                               :allow_blank => true,
+                               :message     => (_("Owner type needs to be one of the following: %s") % OWNER_TYPES.join(', '))}
+      validate :validate_owner
+      validate :owner_taxonomies_match, :if => Proc.new { |host| host.owner.is_a?(User) }
+    end
+
+    # method to return the correct owner list for host edit owner select dropbox
+    def is_owned_by
+      owner.id_and_type if owner
+    end
+
+    # virtual attributes which sets the owner based on the user selection
+    # supports a simple user, or a usergroup
+    # selection parameter is expected to be an ActiveRecord id_and_type method (see Foreman's AR extentions).
+    def is_owned_by=(selection)
+      owner = OwnerClassifier.new(selection).user_or_usergroup
+      self.owner = owner
+    end
+
+    def owner_suggestion
+      owner_id_and_type = Setting[:host_owner]
+      owner = OwnerClassifier.new(owner_id_and_type).user_or_usergroup
+      self.owner || owner || User.current
+    end
+
+    private
+
+    def owner_taxonomies_match
+      return true if self.owner.admin?
+
+      if Organization.organizations_enabled && self.organization_id && !self.owner.my_organizations.where(:id => self.organization_id).exists?
+        errors.add :is_owned_by, _("does not belong into host's organization")
+      end
+      if Location.locations_enabled && self.location_id && !self.owner.my_locations.where(:id => self.location_id).exists?
+        errors.add :is_owned_by, _("does not belong into host's location")
+      end
+    end
+
+    def set_default_user
+      return if self.owner_type.present? && (!OWNER_TYPES.include?(self.owner_type) || self.owner.nil?)
+      self.owner = owner_suggestion
+    end
+
+    def validate_owner
+      return true if self.owner_type.nil? && self.owner.nil?
+
+      add_owner_error if self.owner_type.present? && self.owner.nil?
+    end
+
+    def add_owner_error
+      if self.owner_id.present?
+        errors.add(:owner, (_('There is no owner with id %d and type %s') % [self.owner_id, self.owner_type]))
+      else
+        errors.add(:owner, _('If owner type is specified, owner must be specified too.'))
+      end
+    end
+  end
+end

app/models/host/base.rb

@@ -5,11 +5,11 @@ class Base < ActiveRecord::Base
     include Parameterizable::ByName
     include DestroyFlag
     include InterfaceCloning
+    include Hostext::Ownership
 
     self.table_name = :hosts
     extend FriendlyId
     friendly_id :name
-    OWNER_TYPES = %w(User Usergroup)
 
     validates_lengths_from_database
     belongs_to :model, :name_accessor => 'hardware_model_name'
@@ -29,17 +29,11 @@ class Base < ActiveRecord::Base
     belongs_to :organization
 
     alias_attribute :hostname, :name
-    before_validation :set_default_user
 
     validates :name, :presence   => true, :uniqueness => true, :format => {:with => Net::Validations::HOST_REGEXP}
-    validates :owner_type, :inclusion => { :in          => OWNER_TYPES,
-                                           :allow_blank => true,
-                                           :message     => (_("Owner type needs to be one of the following: %s") % OWNER_TYPES.join(', ')) }
-    validate :validate_owner
     validate :host_has_required_interfaces
     validate :uniq_interfaces_identifiers
     validate :build_managed_only
-    validate :owner_taxonomies_match, :if => Proc.new { |host| host.owner.is_a?(User) }
 
     include PxeLoaderSuggestion
 
@@ -318,22 +312,6 @@ def skip_orchestration?
 
     private
 
-    def owner_taxonomies_match
-      return true if self.owner.admin?
-
-      if Organization.organizations_enabled && self.organization_id && !self.owner.my_organizations.where(:id => self.organization_id).exists?
-        errors.add :is_owned_by, _("does not belong into host's organization")
-      end
-      if Location.locations_enabled && self.location_id && !self.owner.my_locations.where(:id => self.location_id).exists?
-        errors.add :is_owned_by, _("does not belong into host's location")
-      end
-    end
-
-    def set_default_user
-      self.owner ||= User.current if self.owner_type.nil? && self.owner_id.nil?
-      true
-    end
-
     def build_values_for_primary_interface!(values_for_primary_interface, args)
       new_attrs = args.shift
       unless new_attrs.nil?
@@ -533,20 +511,6 @@ def password_base64_encrypted?
         true
       end
     end
-
-    def validate_owner
-      return true if self.owner_type.nil? && self.owner.nil?
-
-      add_owner_error if self.owner_type.present? && self.owner.nil?
-    end
-
-    def add_owner_error
-      if self.owner_id.present?
-        errors.add(:owner, (_('There is no owner with id %d and type %s') % [self.owner_id, self.owner_type]))
-      else
-        errors.add(:owner, _('If owner type is specified, owner must be specified too.'))
-      end
-    end
   end
 end
 

app/models/host/managed.rb

@@ -219,24 +219,10 @@ def <=>(other)
     self.name <=> other.name
   end
 
-  # method to return the correct owner list for host edit owner select dropbox
-  def is_owned_by
-    owner.id_and_type if owner
-  end
-
   def self.model_name
     ActiveModel::Name.new(Host)
   end
 
-  # virtual attributes which sets the owner based on the user selection
-  # supports a simple user, or a usergroup
-  # selection parameter is expected to be an ActiveRecord id_and_type method (see Foreman's AR extentions).
-  def is_owned_by=(selection)
-    oid = User.find(selection.to_i) if selection =~ (/-Users\Z/)
-    oid = Usergroup.find(selection.to_i) if selection =~ (/-Usergroups\Z/)
-    self.owner = oid
-  end
-
   def clear_reports
     # Remove any reports that may be held against this host
     Report.where("host_id = #{id}").delete_all

app/models/setting.rb

@@ -10,7 +10,7 @@ class Setting < ActiveRecord::Base
   TYPES= %w{ integer boolean hash array string }
   FROZEN_ATTRS = %w{ name category full_name }
   NONZERO_ATTRS = %w{ puppet_interval idle_timeout entries_per_page max_trend outofsync_interval }
-  BLANK_ATTRS = %w{ trusted_puppetmaster_hosts login_delegation_logout_url authorize_login_delegation_auth_source_user_autocreate root_pass default_location default_organization websockets_ssl_key websockets_ssl_cert oauth_consumer_key oauth_consumer_secret login_text
+  BLANK_ATTRS = %w{ host_owner trusted_puppetmaster_hosts login_delegation_logout_url authorize_login_delegation_auth_source_user_autocreate root_pass default_location default_organization websockets_ssl_key websockets_ssl_cert oauth_consumer_key oauth_consumer_secret login_text
                     smtp_address smtp_domain smtp_user_name smtp_password smtp_openssl_verify_mode smtp_authentication sendmail_arguments sendmail_location }
   ARRAY_HOSTNAMES = %w{ trusted_puppetmaster_hosts }
   URI_ATTRS = %w{ foreman_url unattended_url }
@@ -42,6 +42,7 @@ def validate(record)
 
   validates :value, :url_schema => ['http', 'https'], :if => Proc.new { |s| URI_BLANK_ATTRS.include?(s.name) && s.value.present? }
 
+  validate :validate_host_owner, :if => Proc.new {|s| s.name == "host_owner" }
   validates :value, :format => { :with => Resolv::AddressRegex }, :if => Proc.new { |s| IP_ATTRS.include? s.name }
   validates :value, :regexp => true, :if => Proc.new { |s| REGEXP_ATTRS.include? s.name }
   validates :value, :array_type => true, :if => Proc.new { |s| s.settings_type == "array" }
@@ -268,7 +269,10 @@ def self.load_defaults
   def self.set(name, description, default, full_name = nil, value = nil, options = {})
     if options.has_key? :collection
       SettingsHelper.module_eval do
-        define_method("#{name}_collection".to_sym){ options[:collection].call }
+        define_method("#{name}_collection".to_sym) do
+          collection = options[:collection].call
+          collection.is_a?(Hash) ? collection : editable_select_optgroup(collection, :include_blank => options[:include_blank])
+        end
       end
     end
     options[:encrypted] ||= false
@@ -287,6 +291,13 @@ def self.column_check(opts)
 
   private
 
+  def validate_host_owner
+    owner_type_and_id = self.value
+    return if owner_type_and_id.blank?
+    owner = OwnerClassifier.new(owner_type_and_id).user_or_usergroup
+    errors.add(:value, _("Host owner is invalid")) if owner.nil?
+  end
+
   def invalid_value_error(error)
     errors.add(:value, _("is invalid: %s") % error)
   end

app/models/setting/provisioning.rb

@@ -12,8 +12,11 @@ def self.default_global_labels
   def self.default_settings
     fqdn = Facter.value(:fqdn) || SETTINGS[:fqdn]
     unattended_url = "http://#{fqdn}"
+    select = [{:name => _("Users"), :class => 'user', :scope => 'visible', :value_method => 'id_and_type', :text_method => 'login'},
+              {:name => _("Usergroup"), :class => 'usergroup', :scope => 'visible', :value_method => 'id_and_type', :text_method => 'name'}]
 
     [
+      self.set('host_owner', N_("Default owner on provisioned hosts, if empty Foreman will use current user"), nil, N_('Host owner'), nil, {:collection => Proc.new { select }, :include_blank => _("Select an owner")}),
       self.set('root_pass', N_("Default encrypted root password on provisioned hosts"), nil, N_('Root password')),
       self.set('unattended_url', N_("URL hosts will retrieve templates from during build (normally http as many installers don't support https)"), unattended_url, N_('Unattended URL')),
       self.set('safemode_render', N_("Enable safe mode config templates rendering (recommended)"), true, N_('Safemode rendering')),

app/services/owner_classifier.rb

@@ -0,0 +1,10 @@
+class OwnerClassifier
+  def initialize(id_and_type)
+    @id_and_type = id_and_type
+  end
+
+  def user_or_usergroup
+    return nil unless @id_and_type =~ /^\d+-(Users|Usergroups)$/
+    @id_and_type.include?('Users') ? User.find_by_id(@id_and_type.to_i) : Usergroup.find_by_id(@id_and_type.to_i)
+  end
+end

app/views/hosts/_form.html.erb

@@ -114,7 +114,7 @@
         <% if SETTINGS[:login] %>
           <%= selectable_f f, :is_owned_by, option_groups_from_collection_for_select(
             [User.authorized(:view_users), Usergroup.authorized(:view_usergroups)], :visible, :table_name, :id_and_type, :select_title,
-            @host.is_owned_by || User.current.id_and_type), { :include_blank => _("select an owner") }, { :label => _("Owned By") }
+            @host.owner_suggestion.id_and_type), { :include_blank => _("Select an owner") }, { :label => _("Owned By") }
           %>
         <% end %>
         <%= checkbox_f f, :enabled,

test/fixtures/settings.yml

@@ -343,3 +343,8 @@ attribute71:
   category: Setting::Provisioning
   default: "false"
   description: 'Stop updating domain from facts'
+attribute72:
+  name: host_owner
+  category: Setting::Provisioning
+  default: nil
+  description: 'Default host owner'

test/models/host_test.rb

@@ -780,44 +780,6 @@ def teardown
       end
     end
 
-    context 'owner_type validations' do
-      test "should save if owner_type is User or Usergroup" do
-        host = FactoryGirl.build(:host, :owner_type => "User", :owner => User.current)
-        assert_valid host
-      end
-
-      test "should not save if owner_type is not User or Usergroup" do
-        host = FactoryGirl.build(:host, :owner_type => "UserGr(up") # should be Usergroup
-        refute_valid host
-      end
-
-      test 'should succeed validation if owner not set' do
-        host = FactoryGirl.build(:host, :without_owner)
-        assert_valid host
-      end
-
-      test "should not save if owner_type is set without owner" do
-        host = FactoryGirl.build(:host, :owner_type => "Usergroup")
-        refute_valid host
-        assert_match(/owner must be specified/, host.errors[:owner].first)
-      end
-
-      test "should not save if owner_type is not in sync with owner" do
-        host = FactoryGirl.build(:host, :owner => User.current)
-        host.owner_type = 'Usergroup'
-        refute_valid host
-        assert_match(/Usergroup/, host.errors[:owner].first)
-      end
-    end
-
-    test "should not save if owner_type is not User or Usergroup" do
-      host = Host.new :name => "myfullhost", :mac => "aabbecddeeff", :ip => "3.3.4.03", :medium => media(:one),
-        :domain => domains(:mydomain), :operatingsystem => operatingsystems(:redhat), :subnet => subnets(:two), :puppet_proxy => smart_proxies(:puppetmaster),
-        :architecture => architectures(:x86_64), :environment => environments(:production), :managed => true,
-        :owner_type => "UserGr(up" # should be Usergroup
-      refute host.valid?
-    end
-
     test "should not save if installation media is missing" do
       host = Host.new :name => "myfullhost", :mac => "aabbecddeeff", :ip => "3.3.4.03", :ptable => FactoryGirl.create(:ptable),
         :domain => domains(:mydomain), :operatingsystem => operatingsystems(:redhat), :subnet => subnets(:two), :puppet_proxy => smart_proxies(:puppetmaster),
@@ -1928,13 +1890,6 @@ def teardown
       end
     end
 
-    test "can auto-complete owner searches by current_user" do
-      as_admin do
-        completions = Host::Managed.complete_for("owner = ")
-        assert completions.include?("owner = current_user"), "completion missing: current_user"
-      end
-    end
-
     test "should accept lookup_values_attributes" do
       h = FactoryGirl.create(:host)
       as_admin do
@@ -1959,23 +1914,6 @@ def teardown
       assert_equal results[0].owner, User.current
     end
 
-    test "can search hosts by owner" do
-      FactoryGirl.create(:host)
-      results = Host.search_for("owner = " + User.current.login)
-      assert_equal User.current.hosts.count, results.count
-      assert_equal results[0].owner, User.current
-    end
-
-    test "search by user returns only the relevant hosts" do
-      host = nil
-      as_user :one do
-        host = FactoryGirl.create(:host)
-      end
-      refute_equal User.current, host.owner
-      results = Host.search_for("owner = " + User.current.login)
-      refute results.include?(host)
-    end
-
     test "search by params returns only the relevant hosts" do
       hg = hostgroups(:common)
       host = FactoryGirl.create(:host, :hostgroup => hg)

test/models/setting_test.rb

@@ -462,6 +462,12 @@ class CustomSetting < Setting
     assert_equal /\Aa.*\Z|\Ab.*\Z/, Setting.convert_array_to_regexp(['a*', 'b*'])
   end
 
+  test "host's owner should be valid" do
+    assert_raises(ActiveRecord::RecordInvalid) do
+      Setting[:host_owner] = "xyz"
+    end
+  end
+
   private
 
   def check_parsed_value(settings_type, expected_value, string_value)