Bug #17157
closedinconsitent mapping of host owner
Description
A host owner can either be a user or a usergroup, this is implemented in the database with two columns.
MariaDB [foreman]> desc hosts; +----------------------+--------------+------+-----+---------+----------------+ | Field | Type | Null | Key | Default | Extra | +----------------------+--------------+------+-----+---------+----------------+ ... | owner_id | int(11) | YES | | NULL | | | owner_type | varchar(255) | YES | | NULL | | ... +----------------------+--------------+------+-----+---------+----------------+
The owner_type checking seems to be inconsistent in the ruby code (not everywhere implemented where needed). I found one case where I can produce an obvious error, while tracking this error through the code I found some other possible errors.
obvious error
steps to reproduce:- need to have organizations enabled
- create a sampe organization
- create a user with a specific user id (eg. 25)
- create a group with the same id
- set the group as owner for a host
- set the sample organzation for the host
- navigate to Administer->Users
- open the user
- open the Organizations part
- the user has the organization assigned and is is grey (Message: "this is used by a host")
I know the code error should be in app/models/concerns/taxonomix.rb:get_taxonomy_ids, but my understanding of ruby is to limited to code a solution for this. :(
possible errors
in app/models/user.rb:
has_many :direct_hosts, :class_name => 'Host', :as => :owner
I think this is missing the where condition to check for the correct type.
This should be:
has_many :direct_hosts, -> { where(:owner_type => 'User') }, :class_name => 'Host', :as => :owner
in app/models/usergroup.rb
has_many_hosts :as => :owner
Same as above.
I can generate a pull request for the simple things, but I need help to create a correct patch for taxonomix.rb
Updated by Ohad Levy about 8 years ago
first, thanks for the detalied response!
secondly, just commenting that:
has_many :direct_hosts, :class_name => 'Host', :as => :owner
is actually correct, the :as keyword defines a polymorphic relationship (how rails calls this kind of db layout) and does the correct SQL
see http://guides.rubyonrails.org/association_basics.html#polymorphic-associations
Updated by Martin Schurz about 8 years ago
Ohad Levy wrote:
is actually correct, the :as keyword defines a polymorphic relationship (how rails calls this kind of db layout) and does the correct SQL
see http://guides.rubyonrails.org/association_basics.html#polymorphic-associations
oh thanks, now I understand this.
I was looking at the relation (User, Usergroup) -> Role and this is modeled with the where condition, maybe this sould be refactored for the sake of consistency
Updated by Kavita Gaikwad almost 8 years ago
- Assignee set to Kavita Gaikwad
- Target version set to 1.15.1
Updated by The Foreman Bot almost 8 years ago
- Status changed from New to Ready For Testing
- Pull request https://github.com/theforeman/foreman/pull/4069 added
Updated by Kavita Gaikwad almost 8 years ago
- Status changed from Ready For Testing to Closed
- % Done changed from 0 to 100
Applied in changeset dd17d2709515acb9eeaf5beb5afe56b43f8bd404.
Updated by Dominic Cleal almost 8 years ago
- Translation missing: en.field_release set to 189