Don't use update_attribute

This method is ambiguous. Sometimes we use it to set invalid data in
tests (which can usually be done with `update_column`), and other times
we use it instead of `update!`.

I'm removing it because, even if sometimes it could make sense to use
it, it's too similar to `update_attributes` (which is an alias for
`update` and runs validations), making it confusing.

However, there's one case where we're still using it: in the
ActsAsParanoidAliases module, we need to invoke the callbacks, which
`update_column` skips, but tests related to translations fail if we use
`update!`. The reason for this is the tests check what happens if we
restore a record without restoring its translations. But that will make
the record invalid, since there's a validation rule checking it has at
least one translation.

I'm not blacklisting any other method which skips validations because we
know they skip validations and use them anyway (hopefully with care).

.rubocop.yml

@@ -18,9 +18,6 @@ Performance/StartWith:
 Rails/HasManyOrHasOneDependent:
   Enabled: true
 
-Rails/SkipsModelValidations:
-  Enabled: true
-
 Security/JSONLoad:
   Enabled: true
 

.rubocop_basic.yml

@@ -248,6 +248,12 @@ Rails/SaveBang:
   Enabled: true
   Severity: refactor
 
+Rails/SkipsModelValidations:
+  Enabled: true
+  Blacklist:
+    - update_attribute
+  Exclude: lib/acts_as_paranoid_aliases.rb
+
 Rails/TimeZone:
   Enabled: true
 

app/controllers/debates_controller.rb

@@ -36,12 +36,12 @@ class DebatesController < ApplicationController
   end
 
   def unmark_featured
-    @debate.update_attribute(:featured_at, nil)
+    @debate.update!(featured_at: nil)
     redirect_to request.query_parameters.merge(action: :index)
   end
 
   def mark_featured
-    @debate.update_attribute(:featured_at, Time.current)
+    @debate.update!(featured_at: Time.current)
     redirect_to request.query_parameters.merge(action: :index)
   end
 

app/models/poll/question/answer.rb

@@ -21,7 +21,7 @@ class Poll::Question::Answer < ApplicationRecord
 
   def self.order_answers(ordered_array)
     ordered_array.each_with_index do |answer_id, order|
-      find(answer_id).update_attribute(:given_order, (order + 1))
+      find(answer_id).update_column(:given_order, (order + 1))
     end
   end
 

spec/controllers/comments_controller_spec.rb

@@ -27,7 +27,7 @@ describe CommentsController do
 
     it "does not create a comment if the comments are closed" do
       sign_in user
-      legal_process.update_attribute(:debate_end_date, Date.current - 1.day)
+      legal_process.update!(debate_end_date: Date.current - 1.day)
 
       expect do
         post :create, xhr: true,

spec/controllers/legislation/annotations_controller_spec.rb

@@ -81,7 +81,7 @@ describe Legislation::AnnotationsController do
 
     it "does not create an annotation if the process allegations phase is not open" do
       sign_in user
-      legal_process.update_attribute(:allegations_end_date, Date.current - 1.day)
+      legal_process.update!(allegations_end_date: Date.current - 1.day)
 
       expect do
         post :create, xhr: true,

spec/controllers/legislation/answers_controller_spec.rb

@@ -41,7 +41,7 @@ describe Legislation::AnswersController do
 
     it "does not create an answer if the process debate phase is not open" do
       sign_in user
-      legal_process.update_attribute(:debate_end_date, Date.current - 1.day)
+      legal_process.update!(debate_end_date: Date.current - 1.day)
 
       expect do
         post :create, xhr: true,

spec/features/admin/admin_notifications_spec.rb

@@ -23,7 +23,7 @@ describe "Admin Notifications" do
 
     scenario "Notification with invalid segment recipient" do
       invalid_notification = create(:admin_notification)
-      invalid_notification.update_attribute(:segment_recipient, "invalid_segment")
+      invalid_notification.update_column(:segment_recipient, "invalid_segment")
 
       visit admin_admin_notification_path(invalid_notification)
 
@@ -56,7 +56,7 @@ describe "Admin Notifications" do
 
     scenario "Notifications with invalid segment recipient" do
       invalid_notification = create(:admin_notification)
-      invalid_notification.update_attribute(:segment_recipient, "invalid_segment")
+      invalid_notification.update_column(:segment_recipient, "invalid_segment")
 
       visit admin_admin_notifications_path
 
@@ -209,7 +209,7 @@ describe "Admin Notifications" do
 
     scenario "Admin notification with invalid segment recipient cannot be sent", :js do
       invalid_notification = create(:admin_notification)
-      invalid_notification.update_attribute(:segment_recipient, "invalid_segment")
+      invalid_notification.update_column(:segment_recipient, "invalid_segment")
       visit admin_admin_notification_path(invalid_notification)
 
       expect(page).not_to have_link("Send")

spec/features/admin/budget_investments_spec.rb

@@ -1832,7 +1832,7 @@ describe "Admin budget investments" do
     end
 
     scenario "Select an investment when some columns are not displayed", :js do
-      investment.update_attribute(:title, "Don't display me, please!")
+      investment.update!(title: "Don't display me, please!")
 
       visit admin_budget_budget_investments_path(budget)
       within("#js-columns-selector") { find("strong", text: "Columns").click }

spec/features/admin/emails/emails_download_spec.rb

@@ -19,7 +19,7 @@ describe "Admin download user emails" do
 
       admin_without_email = create(:user, newsletter: true, email: "no_email@consul.dev")
       create(:administrator, user: admin_without_email)
-      admin_without_email.update_attribute(:email, nil)
+      admin_without_email.update_column(:email, nil)
     end
 
     scenario "returns the selected users segment csv file" do

spec/features/admin/emails/newsletters_spec.rb

@@ -23,7 +23,7 @@ describe "Admin newsletter emails" do
 
     scenario "Invalid newsletter" do
       invalid_newsletter = create(:newsletter)
-      invalid_newsletter.update_attribute(:segment_recipient, "invalid_segment")
+      invalid_newsletter.update_column(:segment_recipient, "invalid_segment")
 
       visit admin_newsletter_path(invalid_newsletter)
 
@@ -50,7 +50,7 @@ describe "Admin newsletter emails" do
 
     scenario "Invalid newsletter" do
       invalid_newsletter = create(:newsletter)
-      invalid_newsletter.update_attribute(:segment_recipient, "invalid_segment")
+      invalid_newsletter.update_column(:segment_recipient, "invalid_segment")
 
       visit admin_newsletters_path
 
@@ -136,7 +136,7 @@ describe "Admin newsletter emails" do
 
     scenario "Invalid newsletter cannot be sent", :js do
       invalid_newsletter = create(:newsletter)
-      invalid_newsletter.update_attribute(:segment_recipient, "invalid_segment")
+      invalid_newsletter.update_column(:segment_recipient, "invalid_segment")
       visit admin_newsletter_path(invalid_newsletter)
 
       expect(page).not_to have_link("Send")

spec/features/legislation/questions_spec.rb

@@ -94,7 +94,7 @@ describe "Legislation" do
     end
 
     scenario "cannot answer question when phase not open" do
-      process.update_attribute(:debate_end_date, Date.current - 1.day)
+      process.update!(debate_end_date: Date.current - 1.day)
       question = process.questions.first
       create(:legislation_question_option, question: question, value: "Yes")
       create(:legislation_question_option, question: question, value: "No")

spec/lib/email_digests_spec.rb

@@ -133,7 +133,7 @@ describe EmailDigest do
 
     it "returns false if email does not exist" do
       user = create(:user)
-      user.update_attribute(:email, nil)
+      user.email = nil
 
       email_digest = EmailDigest.new(user)
       expect(email_digest.valid_email?).to be(false)

spec/models/concerns/globalizable.rb

@@ -13,11 +13,11 @@ shared_examples_for "globalizable" do |factory_name|
   let(:attribute) { required_fields.sample || fields.sample }
 
   before do
-    record.update_attribute(attribute, "In English")
+    record.update!(attribute => "In English")
 
     I18n.with_locale(:es) do
       record.update!(required_fields.map { |field| [field, "En español"] }.to_h)
-      record.update_attribute(attribute, "En español")
+      record.update!(attribute => "En español")
     end
 
     record.reload
@@ -156,7 +156,7 @@ shared_examples_for "globalizable" do |factory_name|
     before do
       I18n.with_locale(:de) do
         record.update!(required_fields.map { |field| [field, "Deutsch"] }.to_h)
-        record.update_attribute(attribute, "Deutsch")
+        record.update!(attribute => "Deutsch")
       end
     end
 

spec/models/proposal_spec.rb

@@ -253,7 +253,7 @@ describe Proposal do
         proposal = create(:proposal)
 
         tag_list = ["tag1", "tag2", "tag3", "tag4", "tag5", "tag6", "tag7"]
-        proposal.update_attribute(:tag_list, tag_list)
+        proposal.update!(tag_list: tag_list)
 
         expect(proposal.update_cached_votes).to eq(true)
       end