Using pundit in view components

[leorseligmann]

Hi All,

We're evaluating two different approaches for handling the companies option in our OrderFiltersComponent used on the orders page. Currently, the component requires companies to be passed in as an option, representing the companies a user can filter for on that page.

Here’s the current implementation:

class Ui::Shipment::OrderFiltersComponent < Ui::OrderFiltersComponent
  option :order_id
  option :filter_params
  option :companies

  def form_url
    shipments_path(order_id:)
  end

  def turbo_frame_id = "reseller_shipments_results"
end

Rendering this component:

<%= render Ui::Shipment::OrderFiltersComponent.new(order_id: @order.id, filter_params: @filter_params, companies: @companies) %>

And within the Controllers:

Reseller::OrdersController < Ui::ResellerController
    @companies = policy_scope(Company)
...
Reseller::ShipmentsController < Ui::ResellerController
    @companies = policy_scope(Company)
...

My Proposal:
It seems unnecessary to inject companies each time we use this component, as we already have access to the current_user within OrderFiltersComponent. I suggest defining companies within the component itself (based on current_user) instead of passing it in as an argument.

class Ui::Shipment::OrderFiltersComponent < Ui::OrderFiltersComponent
  option :order_id
  option :filter_params

  def companies = CompanyPolicy.new(Company.all, current_user)

  def form_url
    shipments_path(order_id:)
  end

  def turbo_frame_id = "reseller_shipments_results"
end

Would love to hear your thoughts on this! Do you see benefits to keeping companies injected externally, or do you agree with handling it directly within the component?

Thanks for the feedback!

[boardfish]

Hey @leorseligmann! Generally, current_user constitutes global state. Within the gem itself, we make a priority of either making explicit where global state is necessary (particularly demonstrated by how helpers are managed), or avoiding it entirely. But components, as with plain old Ruby objects, should also seek to provide a good interface to their consumers.

Injecting companies externally would give you all the benefits of dependency injection and avoiding global state, but if this is something you're using quite widely as a pattern, then it may also be something you might like to consider.

A halfway house could be defaulting to that:

# Might need to go in a before_render callback if current_user comes from a helper.
before_render { @companies ||= CompanyPolicy.new(Company.all, current_user) }

def initialize(companies = nil)
  @companies = companies
end

But at the end of the day it's what works in your context. Hope that's helpful to you!