Project

General

Profile

Actions

Bug #17157

closed

inconsitent mapping of host owner

Added by Martin Schurz over 7 years ago. Updated about 6 years ago.

Status:
Closed
Priority:
High
Category:
Organizations and Locations
Target version:
Difficulty:
easy
Triaged:
Fixed in Releases:
Found in Releases:

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
found error:
  • 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

Actions #1

Updated by Ohad Levy over 7 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

Actions #2

Updated by Martin Schurz over 7 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

Actions #3

Updated by Kavita Gaikwad over 7 years ago

  • Assignee set to Kavita Gaikwad
  • Target version set to 1.15.1
Actions #4

Updated by The Foreman Bot over 7 years ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/4069 added
Actions #5

Updated by Kavita Gaikwad over 7 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100
Actions #6

Updated by Dominic Cleal over 7 years ago

  • Translation missing: en.field_release set to 189
Actions

Also available in: Atom PDF