Move scoped_search definitions from parent STI classes to subclasses
|Assigned To:||Dominic Cleal|
|Found in release:||Pull request:||https://github.com/theforeman/foreman/pull/3917|
|Velocity based estimate||-|
Under Rails 5, the CommonParameter indexes ("Global parameters") show all types of parameters and not only CommonParameters, as
CommonParameter.search_for (with no string) returns
Tests actually fail as HostParameters are in fixtures without corresponding hosts:
ActionView::Template::Error: undefined method `id' for nil:NilClass
app/views/common_parameters/index.html.erb:18:in `block in app_views_common_parameters_index_html_erb__2292157232256250179_107176060'
(The common parameters controller should never be rendering a HostParameter.)
CommonParameter.search_for, scoped_search's scope will call Parameter.all as the search definition is registered on Parameter. Under Rails 4, it would apply the current scope (where type='CommonParameter') to all related STI classes, so even explicitly calling scopes on Parameter are filtered by type='CommonParameter' while within the scoped_definition scope called through CommonParameter.
This was reverted in Rails ticket https://github.com/rails/rails/issues/18806 in 5.0.0. Now when calling CommonParameter.search_for, scoped_search calls Parameter.all again, and it returns every Parameter subclass.
scoped_search doesn't support STI - registering a definition on a parent class and expecting it to work with subclasses - and there are known bugs with it already (https://github.com/wvanbergen/scoped_search/issues/112 + related). Moving scoped search definitions to the subclasses and not relying on inheritance would be more reliable until that changes. LookupKey already does this through defining self.inherited to work around some of these bugs.
For this issue, moving the definition to CommonParameter would cause scoped_search to call CommonParameter.all instead, so it doesn't rely on the current scope.
fixes #16798 - move scoped_search definitions to STI subclasses
scoped_search doesn't support class inheritance with STI, so registering
definitions on the subclass fixes various issues. This fixes an issue
where scoped_search on CommonParameter calls Parameter.all and is
returned a list of CommonParameters under Rails 4.2, as it relies on a
bug (#18806) for applying the type='CommonParameter' condition when
On Rails 5.0, this bug was fixed and calling Parameter.all within a
scope on CommonParameter now returns all types of parameters.
Registering all scoped_search definitions on the subclasses ensures that
scoped_search calls CommonParameter.all instead.
The taxonomies API changed as it called Taxonomix objects with scopes
such as `.locations.search_for`, which no longer existed when the
scoped_search definitions were removed from Taxonomy (.locations is a
Taxonomy association with a where clause). This now explicitly searches
via the appropriate Taxonomy subclass.