Skip to content

Allow usage of Relation#pluck outside of ActiveRecord models

drew stachon requested to merge allow-ar-pluck-outside-of-models into master

What does this MR do and why?

This MR allows for usage of pluck outside of models classes. The spirit of the AR CodeReuse cop is to keep querying logic in the model, but pluck isn't a chainable method and isn't really querying logic, just an optimization on the actual loading of the records. We also allow the exact behavior if the column being plucked is the primary key, via #ids.

Currently, according to Rubocop:

# Good:
Ci::Pipeline.some_relation.map(&:id)

# Good:
Ci::Pipeline.some_relation.ids

# Bad:
Ci::Pipeline.some_relation.pluck(:id)

And #ids is actually defined as:

    def ids
      pluck primary_key
    end

It's not clear to me why we allow this to be done via some syntax sugar that restricts usage to the primary key:

# gitlab-org/gitlab: app/models/application_record.rb

def self.pluck_primary_key
  where(nil).pluck(primary_key)
end

At the point in the relation query chain where we ask for the actual values of a column, I believe we're done doing the querying?

Allowing developers to load every record and map the column but not pluck the column seems restrictive and I don't understand the benefit.

Edited by drew stachon

Merge request reports

Loading