Project

General

Profile

Actions

Refactor #17085

open

Make use of index scope DSL

Added by Lukas Zapletal over 7 years ago. Updated almost 6 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
Discovery plugin
Target version:
Difficulty:
trivial
Triaged:
Fixed in Releases:
Found in Releases:

Description

Shim added (#16646) the ability for plugins to modify the scope used to get hosts for #index action. This is needed if a plugin wants to add more includes or eager_load statements, so Active Record won't try to lazy load additional relations and create N+1 queries.

Discovery should take advantage of this.


Related issues 1 (0 open1 closed)

Related to Foreman - Feature #16646: Allow Foreman controllers to be extended to include fields from external pluginsClosedShimon Shtein09/21/2016Actions
Actions #1

Updated by Lukas Zapletal over 7 years ago

  • Related to Feature #16646: Allow Foreman controllers to be extended to include fields from external plugins added
Actions #2

Updated by Lukas Zapletal over 7 years ago

  • Difficulty set to trivial

Trivial difficulty for our newcomers!

Actions #3

Updated by Swapnil Abnave over 7 years ago

Lukas Zapletal wrote:

Shim added (#16646) the ability for plugins to modify the scope used to get hosts for #index action. This is needed if a plugin wants to add more includes or eager_load statements, so Active Record won't try to lazy load additional relations and create N+1 queries.

Discovery should take advantage of this.

@Lukas Zapletal

`ScopesPerAction` can be used once the pull request(https://github.com/theforeman/foreman/pull/3887) gets merged(probably once conflicts are resolved) into foreman, right ?

Actions #4

Updated by Shimon Shtein over 7 years ago

ScopesPerAction concern is needed if you want to add the ability to customize actions in your controller.
If you want to customize the scope for existing controller (HostsController is the only one that uses ScopesPerAction right now), you will need to register your scope filter in the plugin declaration:

    Foreman::Plugin.register :test_hosts_controller_action_scope do
      add_controller_action_scope HostsController, :test_action, &mock_scope
    end

Actions #5

Updated by Swapnil Abnave over 7 years ago

  • Target version set to 1.15.6
Actions #6

Updated by Swapnil Abnave over 7 years ago

Shimon Shtein wrote:

ScopesPerAction concern is needed if you want to add the ability to customize actions in your controller.

This looks more of refactoring step.

If you want to customize the scope for existing controller (HostsController is the only one that uses ScopesPerAction right now), you will need to register your scope filter in the plugin declaration:
[...]

So if we take an example from foreman_discovery plugin.
https://github.com/theforeman/foreman_discovery/blob/develop/app/controllers/discovered_hosts_controller.rb#L20-L25

The `includes` method chained in DiscoveredHostsController#index would be removed and instead the plugin scope will come into play ?

## The index action would look like:

  def index
    @hosts = resource_base.search_for(params[:search], :order => params[:order]).paginate(:page => params[:page])
    fact_array = @hosts.collect do |host|
      [host.id, Hash[host.fact_values.joins(:fact_name).where('fact_names.name' => Setting::Discovered.discovery_fact_column_array).pluck(:name, :value)]]
    end
    @host_facts = Hash[fact_array]
  end

## Some initializer:

  Foreman::Plugin.register :my_plugin do                                                                                                                                                      
    add_controller_action_scope(DiscoveredHostsController, :index) { |base_scope|                                                                                                             
      base_scope.includes([                                                                                                                                                                   
                            :location,                                                                                                                                                        
                            :organization,                                                                                                                                                    
                            :model,                                                                                                                                                           
                            :discovery_attribute_set                                                                                                                                          
                          ], {:interfaces => :subnet}) }                                                                                                                                      
  end

I don't think that's what it's meant for. Using `ScopesPerAction` sounds legitimate wrt foreman_discovery.
Actions

Also available in: Atom PDF