Skip to content

Add a thin encapsulation around .pluck(:id)

Nick Thomas requested to merge (removed):add-pluck-primary-key into master

What does this MR do?

Adds a pluck_primary_key method to ApplicationRecord. This is available in every ActiveRecord::Relation, meaning that we can call, for instance, Project.scopeA.scopeB.pluck_primary_key to get a list of IDs for the project.

Typically, pluck(:id) is forbidden by the CodeReuse/ActiveRecord cop. Justifications for this revolve around the fact that it leaks knowledge about the database schema from app/models into other areas where it shoudn't exist, and that it's difficult to maintain this as the schema changes.

We often need to acquire a list of IDs legitimate reasons; I've modified a few sites in app/services to use this helper, mostly to highlight ways it's being used at present. In reviews, I'm noticing that people are just starting to sprinkle # rubocop:disable CodeReuse/ActiveRecord around instead of trying to avoid or relocate the pluck, which is very sub-optimal. Hopefully, this thin wrapper treads a nice middle ground between the two concerns.

Of course, one should (almost) never use pluck to get a list of IDs to feed back into another SQL query, and this helper should not be used as such. I don't believe it makes it more likely.

cc @yorickpeterse @lulalala @dbalexandre

What are the relevant issue numbers?

Does this MR meet the acceptance criteria?

Merge request reports