Skip to content

RuboCop: Consider disabling cop rule `Rails/SkipsModelValidations` permanently

Problem

There has been discussions about the use of update_columns vs update! in production code and specs.

The former skips validations, callbacks is therefor faster because it also potentially avoids SQL queries.

We've enabled a 👮 rule Rails/SkipsModelValidations which suggests the opposite:

This cop checks for the use of methods which skip validations which are listed in https://guides.rubyonrails.org/active_record_validations.html#skipping-validations

However, this 👮 rule is currently disabled due to too many offenses (see Split up .rubocop_todo.yml into .rubocop_todo/*... (#354328 - closed)) but is likely to be enabled soon.

Proposed solution

🅰 Disable rule permanently - Decide case by case

Do not suggest to use update! by default anymore via 👮 Rails/SkipsModelValidations.

We'd need to mention update! vs update_columns in our development guidelines and help developers to decide when to use the former and when the latter. Case by case.

Note: When it comes to performance update_columns is clearly faster in mirco benchmarks because it skips validations, callbacks and potentially extra SQL queries. See !81767 (comment 863638026) and also → The mirco benchmark.

🅱 Re-Enable rule - Focus on record integrity

When it comes to record integrity update! is a safer approach because it executes validations, callbacks etc. making sure that stored record remains integer (not every validation can be expressed as a database constraint). See !81767 (comment 862594907).

In this case we should enable 👮 Rails/SkipsModelValidations and suggest the use of update! and save! by default. In cases where performance really matters this 👮 can be disabled. For example: https://docs.gitlab.com/ee/development/merge_request_performance_guidelines.html#query-counts

Out of scope

  • Updating development guidelines to discuss when update! and when update_columns is preferred.
  • Creating a 🆕 👮, say, Performance/RunsModelValidations to suggest update_columns (and friends) over update!

Refs

Edited by Peter Leitzen