From 765d405df1201ce23f78f3d9782cdbecf3ee0e30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 19 Jun 2019 03:04:00 +0200
Subject: [PATCH 1/3] Use Rails 5 conventions in migrations and models

We forgot to add these changes to pull requests which were in
development before we upgraded to Rails 5.

We're also moving the rubocop rules to the basic files, so we're
notified when we inherit from `ActiveRecord::Base`.
---
 .rubocop.yml                                             | 9 ---------
 .rubocop_basic.yml                                       | 9 +++++++++
 app/models/budget/investment/change_log.rb               | 2 +-
 app/models/download_setting.rb                           | 2 +-
 db/migrate/20190312100543_create_download_settings.rb    | 2 +-
 .../20190321144328_add_config_to_download_settings.rb    | 2 +-
 db/migrate/20190408083819_add_actions_to_valuators.rb    | 2 +-
 ...0190423084112_create_budget_investment_change_logs.rb | 2 +-
 .../20190429214633_add_date_of_birth_to_signatures.rb    | 2 +-
 ...s_to_required_fields_to_verify_in_signature_sheets.rb | 2 +-
 .../20190430003908_add_postal_code_to_signatures.rb      | 2 +-
 db/schema.rb                                             | 4 ++--
 12 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/.rubocop.yml b/.rubocop.yml
index 07a45ca67..6936f6f78 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -93,18 +93,9 @@ Performance/UnfreezeString:
 Performance/UriDefaultParser:
   Enabled: true
 
-Rails/ActionFilter:
-  Enabled: true
-
 Rails/ActiveSupportAliases:
   Enabled: true
 
-Rails/ApplicationJob:
-  Enabled: true
-
-Rails/ApplicationRecord:
-  Enabled: true
-
 Rails/Blank:
   Enabled: true
 
diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml
index 8f5ba639d..41c7a619d 100644
--- a/.rubocop_basic.yml
+++ b/.rubocop_basic.yml
@@ -38,6 +38,15 @@ Lint/UselessAssignment:
 Metrics/LineLength:
   Max: 110
 
+Rails/ActionFilter:
+  Enabled: true
+
+Rails/ApplicationJob:
+  Enabled: true
+
+Rails/ApplicationRecord:
+  Enabled: true
+
 RSpec/NotToNot:
   Enabled: true
 
diff --git a/app/models/budget/investment/change_log.rb b/app/models/budget/investment/change_log.rb
index b2ae65a3e..d93843e26 100644
--- a/app/models/budget/investment/change_log.rb
+++ b/app/models/budget/investment/change_log.rb
@@ -1,4 +1,4 @@
-class Budget::Investment::ChangeLog < ActiveRecord::Base
+class Budget::Investment::ChangeLog < ApplicationRecord
   belongs_to :author, -> { with_hidden }, class_name: "User", foreign_key: "author_id", required: false
 
   validates :old_value, presence: true
diff --git a/app/models/download_setting.rb b/app/models/download_setting.rb
index 76c7f21c5..0058a2866 100644
--- a/app/models/download_setting.rb
+++ b/app/models/download_setting.rb
@@ -1,4 +1,4 @@
-class DownloadSetting < ActiveRecord::Base
+class DownloadSetting < ApplicationRecord
   validates :name_model, presence: true
   validates :name_field, presence: true
 
diff --git a/db/migrate/20190312100543_create_download_settings.rb b/db/migrate/20190312100543_create_download_settings.rb
index ab28b09df..91d81e550 100644
--- a/db/migrate/20190312100543_create_download_settings.rb
+++ b/db/migrate/20190312100543_create_download_settings.rb
@@ -1,4 +1,4 @@
-class CreateDownloadSettings < ActiveRecord::Migration
+class CreateDownloadSettings < ActiveRecord::Migration[4.2]
   def change
     create_table :download_settings do |t|
       t.string :name_model, null: false
diff --git a/db/migrate/20190321144328_add_config_to_download_settings.rb b/db/migrate/20190321144328_add_config_to_download_settings.rb
index 61fddcef3..74c650011 100644
--- a/db/migrate/20190321144328_add_config_to_download_settings.rb
+++ b/db/migrate/20190321144328_add_config_to_download_settings.rb
@@ -1,4 +1,4 @@
-class AddConfigToDownloadSettings < ActiveRecord::Migration
+class AddConfigToDownloadSettings < ActiveRecord::Migration[4.2]
   def change
     add_column :download_settings, :config, :integer, default: 0, null: false
 
diff --git a/db/migrate/20190408083819_add_actions_to_valuators.rb b/db/migrate/20190408083819_add_actions_to_valuators.rb
index 62f5cf213..47a84a481 100644
--- a/db/migrate/20190408083819_add_actions_to_valuators.rb
+++ b/db/migrate/20190408083819_add_actions_to_valuators.rb
@@ -1,4 +1,4 @@
-class AddActionsToValuators < ActiveRecord::Migration
+class AddActionsToValuators < ActiveRecord::Migration[4.2]
   def change
     add_column :valuators, :can_comment, :boolean, default: true
     add_column :valuators, :can_edit_dossier, :boolean, default: true
diff --git a/db/migrate/20190423084112_create_budget_investment_change_logs.rb b/db/migrate/20190423084112_create_budget_investment_change_logs.rb
index bd663ef49..af302f4ec 100644
--- a/db/migrate/20190423084112_create_budget_investment_change_logs.rb
+++ b/db/migrate/20190423084112_create_budget_investment_change_logs.rb
@@ -1,4 +1,4 @@
-class CreateBudgetInvestmentChangeLogs < ActiveRecord::Migration
+class CreateBudgetInvestmentChangeLogs < ActiveRecord::Migration[4.2]
   def change
     create_table :budget_investment_change_logs do |t|
       t.integer :investment_id
diff --git a/db/migrate/20190429214633_add_date_of_birth_to_signatures.rb b/db/migrate/20190429214633_add_date_of_birth_to_signatures.rb
index 51d6e02c3..47dd2eea5 100644
--- a/db/migrate/20190429214633_add_date_of_birth_to_signatures.rb
+++ b/db/migrate/20190429214633_add_date_of_birth_to_signatures.rb
@@ -1,4 +1,4 @@
-class AddDateOfBirthToSignatures < ActiveRecord::Migration
+class AddDateOfBirthToSignatures < ActiveRecord::Migration[4.2]
   def change
     add_column :signatures, :date_of_birth, :date
   end
diff --git a/db/migrate/20190429214851_rename_document_numbers_to_required_fields_to_verify_in_signature_sheets.rb b/db/migrate/20190429214851_rename_document_numbers_to_required_fields_to_verify_in_signature_sheets.rb
index 7da813a9f..7493a1dcf 100644
--- a/db/migrate/20190429214851_rename_document_numbers_to_required_fields_to_verify_in_signature_sheets.rb
+++ b/db/migrate/20190429214851_rename_document_numbers_to_required_fields_to_verify_in_signature_sheets.rb
@@ -1,4 +1,4 @@
-class RenameDocumentNumbersToRequiredFieldsToVerifyInSignatureSheets < ActiveRecord::Migration
+class RenameDocumentNumbersToRequiredFieldsToVerifyInSignatureSheets < ActiveRecord::Migration[4.2]
   def change
     rename_column :signature_sheets, :document_numbers, :required_fields_to_verify
   end
diff --git a/db/migrate/20190430003908_add_postal_code_to_signatures.rb b/db/migrate/20190430003908_add_postal_code_to_signatures.rb
index 4d71687c8..beb4f019b 100644
--- a/db/migrate/20190430003908_add_postal_code_to_signatures.rb
+++ b/db/migrate/20190430003908_add_postal_code_to_signatures.rb
@@ -1,4 +1,4 @@
-class AddPostalCodeToSignatures < ActiveRecord::Migration
+class AddPostalCodeToSignatures < ActiveRecord::Migration[4.2]
   def change
     add_column :signatures, :postal_code, :string
   end
diff --git a/db/schema.rb b/db/schema.rb
index 9d1a28d42..c6d9a6f88 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1460,10 +1460,10 @@ ActiveRecord::Schema.define(version: 20190607160900) do
   end
 
   create_table "signature_sheets", force: :cascade do |t|
-    t.integer  "signable_id"
     t.string   "signable_type"
-    t.boolean  "processed",        default: false
+    t.integer  "signable_id"
     t.text     "required_fields_to_verify"
+    t.boolean  "processed",                 default: false
     t.integer  "author_id"
     t.datetime "created_at"
     t.datetime "updated_at"

From bd85aede99076de9a3c94227ffd17db4858c850d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 19 Jun 2019 14:18:33 +0200
Subject: [PATCH 2/3] Use `describe` on new feature tests

The `type: :feature` is automatically detected by RSpec because these
tests are inside the `spec/features` folder. Using `feature` re-adds a
`type: :feature` to these files, which will result in a conflict when we
upgrade to Rails 5.1's system tests.

Because of this change, we also need to change `background` to `before`
or else these tests will fail.

We're also adding a rubocop rule so we dont' accidentally add these
keywords again.
---
 .rubocop_basic.yml                             |  6 ++++++
 spec/features/admin/change_log_spec.rb         |  4 ++--
 spec/features/admin/download_settings_spec.rb  | 12 ++++++------
 .../admin/local_census_records/imports_spec.rb |  4 ++--
 .../admin/local_census_records_spec.rb         |  6 +++---
 spec/features/legislation/resume_spec.rb       |  2 +-
 spec/features/polls/votation_type_spec.rb      | 12 ++++++------
 .../tracking/budget_investments_spec.rb        | 18 +++++++++---------
 spec/features/tracking/budgets_spec.rb         |  4 ++--
 9 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml
index 41c7a619d..cd71adb58 100644
--- a/.rubocop_basic.yml
+++ b/.rubocop_basic.yml
@@ -14,6 +14,12 @@ AllCops:
   # to ignore them, so only the ones explicitly set in this file are enabled.
   DisabledByDefault: true
 
+Capybara/FeatureMethods:
+  Enabled: true
+  EnabledMethods:
+    - scenario
+    - xscenario
+
 Layout/IndentationConsistency:
   EnforcedStyle: rails
 
diff --git a/spec/features/admin/change_log_spec.rb b/spec/features/admin/change_log_spec.rb
index 288c0d5d0..684d94038 100644
--- a/spec/features/admin/change_log_spec.rb
+++ b/spec/features/admin/change_log_spec.rb
@@ -1,6 +1,6 @@
 require "rails_helper"
 
-feature "Admin change log" do
+describe "Admin change log" do
   let(:budget) {create(:budget)}
   let(:administrator) do
     create(:administrator, user: create(:user, username: "Ana", email: "ana@admins.org"))
@@ -8,7 +8,7 @@ feature "Admin change log" do
 
   context "Investments Participatory Budgets" do
 
-    background do
+    before do
       @admin = create(:administrator)
       login_as(@admin.user)
     end
diff --git a/spec/features/admin/download_settings_spec.rb b/spec/features/admin/download_settings_spec.rb
index 7891db1ee..28cffa683 100644
--- a/spec/features/admin/download_settings_spec.rb
+++ b/spec/features/admin/download_settings_spec.rb
@@ -1,8 +1,8 @@
 require "rails_helper"
 
-feature "Admin download settings" do
+describe "Admin download settings" do
 
-  background do
+  before do
     admin = create(:administrator)
     login_as(admin.user)
   end
@@ -31,7 +31,7 @@ feature "Admin download settings" do
 
   context "Download debates" do
 
-    background do
+    before do
       create(:debate)
     end
 
@@ -84,7 +84,7 @@ feature "Admin download settings" do
 
   context "Download proposals" do
 
-    background do
+    before do
       create(:proposal)
     end
 
@@ -177,7 +177,7 @@ feature "Admin download settings" do
 
   context "Download legislation process" do
 
-    background do
+    before do
       create(:legislation_process, :open)
       create(:legislation_process, :published)
     end
@@ -286,7 +286,7 @@ feature "Admin download settings" do
                                ballot_lines_count: 600) }
     let(:budget) {create :budget}
 
-    background do
+    before do
       Budget::Result.new(budget_finished, heading).calculate_winners
     end
 
diff --git a/spec/features/admin/local_census_records/imports_spec.rb b/spec/features/admin/local_census_records/imports_spec.rb
index 76d990d8b..6b22cba4f 100644
--- a/spec/features/admin/local_census_records/imports_spec.rb
+++ b/spec/features/admin/local_census_records/imports_spec.rb
@@ -1,10 +1,10 @@
 require "rails_helper"
 
-feature "Imports", type: :feature do
+describe "Imports" do
 
   let(:base_files_path) { %w[spec fixtures files local_census_records import] }
 
-  background do
+  before do
     admin = create(:administrator)
     login_as(admin.user)
   end
diff --git a/spec/features/admin/local_census_records_spec.rb b/spec/features/admin/local_census_records_spec.rb
index b763378d2..fa5db3dee 100644
--- a/spec/features/admin/local_census_records_spec.rb
+++ b/spec/features/admin/local_census_records_spec.rb
@@ -1,8 +1,8 @@
 require "rails_helper"
 
-feature "Admin local census records" do
+describe "Admin local census records" do
 
-  background do
+  before do
     login_as(create(:administrator).user)
   end
 
@@ -50,7 +50,7 @@ feature "Admin local census records" do
     end
 
     context "Search" do
-      background do
+      before do
         create(:local_census_record, document_number: "X66777888" )
       end
 
diff --git a/spec/features/legislation/resume_spec.rb b/spec/features/legislation/resume_spec.rb
index 4ffd87a9c..c61112336 100644
--- a/spec/features/legislation/resume_spec.rb
+++ b/spec/features/legislation/resume_spec.rb
@@ -1,6 +1,6 @@
 require "rails_helper"
 
-feature "Legislation" do
+describe "Legislation" do
   context "process resume page" do
 
     scenario "resume tab not show" do
diff --git a/spec/features/polls/votation_type_spec.rb b/spec/features/polls/votation_type_spec.rb
index 255a52e5c..a7922ec57 100644
--- a/spec/features/polls/votation_type_spec.rb
+++ b/spec/features/polls/votation_type_spec.rb
@@ -1,6 +1,6 @@
 require "rails_helper"
 
-feature "Poll Votation Type" do
+describe "Poll Votation Type" do
 
   context "Unique" do
 
@@ -10,7 +10,7 @@ feature "Poll Votation Type" do
     let!(:answer1) { create(:poll_question_answer, question: unique, title: "answer_1") }
     let!(:answer2) { create(:poll_question_answer, question: unique, title: "answer_2") }
 
-    background do
+    before do
       login_as(user)
     end
 
@@ -64,7 +64,7 @@ feature "Poll Votation Type" do
     let!(:answer4) { create(:poll_question_answer, question: question, title: "answer_4") }
     let!(:answer5) { create(:poll_question_answer, question: question, title: "answer_5") }
 
-    background do
+    before do
       login_as(user)
     end
 
@@ -148,7 +148,7 @@ feature "Poll Votation Type" do
     let!(:answer4) { create(:poll_question_answer, question: question, title: "answer_4") }
     let!(:answer5) { create(:poll_question_answer, question: question, title: "answer_5") }
 
-    background do
+    before do
       login_as(user)
     end
 
@@ -210,7 +210,7 @@ feature "Poll Votation Type" do
     let!(:answer4) { create(:poll_question_answer, question: question, title: "answer_4") }
     let!(:answer5) { create(:poll_question_answer, question: question, title: "answer_5") }
 
-    background do
+    before do
       login_as(user)
     end
 
@@ -306,7 +306,7 @@ feature "Poll Votation Type" do
     let!(:answer4) { create(:poll_question_answer, question: question, title: "answer_4") }
     let!(:answer5) { create(:poll_question_answer, question: question, title: "answer_5") }
 
-    background do
+    before do
       login_as(user)
     end
 
diff --git a/spec/features/tracking/budget_investments_spec.rb b/spec/features/tracking/budget_investments_spec.rb
index eab8424e7..8dad71d2c 100644
--- a/spec/features/tracking/budget_investments_spec.rb
+++ b/spec/features/tracking/budget_investments_spec.rb
@@ -1,13 +1,13 @@
 require "rails_helper"
 
-feature "Valuation budget investments" do
+describe "Valuation budget investments" do
 
   let(:budget) { create(:budget) }
   let(:tracker) do
     create(:tracker, user: create(:user, username: "Rachel", email: "rachel@trackers.org"))
   end
 
-  background do
+  before do
     login_as(tracker.user)
   end
 
@@ -23,7 +23,7 @@ feature "Valuation budget investments" do
     expect(page).to have_link "Tracking", href: tracking_root_path
   end
 
-  feature "Index" do
+  describe "Index" do
     scenario "Index shows budget investments assigned to current tracker" do
       investment1 = create(:budget_investment, budget: budget)
       investment2 = create(:budget_investment, budget: budget)
@@ -108,7 +108,7 @@ feature "Valuation budget investments" do
     end
   end
 
-  feature "Show" do
+  describe "Show" do
     let(:administrator) do
       create(:administrator, user: create(:user, username: "Ana", email: "ana@admins.org"))
     end
@@ -119,7 +119,7 @@ feature "Valuation budget investments" do
       create(:budget_investment, budget: budget, administrator: administrator)
     end
 
-    background do
+    before do
       investment.trackers << [tracker, second_tracker]
     end
 
@@ -168,7 +168,7 @@ feature "Valuation budget investments" do
 
   end
 
-  feature "Milestones" do
+  describe "Milestones" do
     let(:admin) { create(:administrator) }
     let(:investment) do
       heading = create(:budget_heading)
@@ -176,7 +176,7 @@ feature "Valuation budget investments" do
                                  administrator: admin)
     end
 
-    background do
+    before do
       investment.trackers << tracker
     end
 
@@ -250,7 +250,7 @@ feature "Valuation budget investments" do
 
   end
 
-  feature "Progress Bars" do
+  describe "Progress Bars" do
 
     let(:admin) { create(:administrator) }
     let(:investment) do
@@ -259,7 +259,7 @@ feature "Valuation budget investments" do
              administrator: admin)
     end
 
-    background do
+    before do
       investment.trackers << tracker
     end
 
diff --git a/spec/features/tracking/budgets_spec.rb b/spec/features/tracking/budgets_spec.rb
index ab24069dd..b6fb44890 100644
--- a/spec/features/tracking/budgets_spec.rb
+++ b/spec/features/tracking/budgets_spec.rb
@@ -1,8 +1,8 @@
 require "rails_helper"
 
-feature "Tracking budgets" do
+describe "Tracking budgets" do
 
-  background do
+  before do
     @tracker = create(:tracker, user: create(:user, username: "Rachel",
                                              email: "rachel@trackers.org"))
     login_as(@tracker.user)

From ae98dbf683d3d24f4156f49bdc7639e013813aeb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 7 Aug 2019 13:43:57 +0200
Subject: [PATCH 3/3] Use Rails 5.1 conventions in tests params

---
 .rubocop.yml                                           | 3 ---
 .rubocop_basic.yml                                     | 3 +++
 spec/controllers/debates_controller_spec.rb            | 2 +-
 spec/controllers/remote_translation_controller_spec.rb | 8 ++++----
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/.rubocop.yml b/.rubocop.yml
index 6936f6f78..5854c404b 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -138,9 +138,6 @@ Rails/HasAndBelongsToMany:
 Rails/HasManyOrHasOneDependent:
   Enabled: true
 
-Rails/HttpPositionalArguments:
-  Enabled: true
-
 Rails/InverseOf:
   Enabled: true
 
diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml
index cd71adb58..1d9d41b45 100644
--- a/.rubocop_basic.yml
+++ b/.rubocop_basic.yml
@@ -53,6 +53,9 @@ Rails/ApplicationJob:
 Rails/ApplicationRecord:
   Enabled: true
 
+Rails/HttpPositionalArguments:
+  Enabled: true
+
 RSpec/NotToNot:
   Enabled: true
 
diff --git a/spec/controllers/debates_controller_spec.rb b/spec/controllers/debates_controller_spec.rb
index 2e8b111d2..67f98936e 100644
--- a/spec/controllers/debates_controller_spec.rb
+++ b/spec/controllers/debates_controller_spec.rb
@@ -24,7 +24,7 @@ describe DebatesController do
       }
       sign_in create(:user)
 
-      post :create, debate: debate_attributes
+      post :create, params: { debate: debate_attributes }
       expect(Ahoy::Event.where(name: :debate_created).count).to eq 1
       expect(Ahoy::Event.last.properties["debate_id"]).to eq Debate.last.id
     end
diff --git a/spec/controllers/remote_translation_controller_spec.rb b/spec/controllers/remote_translation_controller_spec.rb
index f38a4aedf..fec828b55 100644
--- a/spec/controllers/remote_translation_controller_spec.rb
+++ b/spec/controllers/remote_translation_controller_spec.rb
@@ -18,7 +18,7 @@ describe RemoteTranslationsController do
     end
 
     it "create correctly remote translation" do
-      post :create, remote_translations: @remote_translations_params
+      post :create, params: { remote_translations: @remote_translations_params }
 
       expect(RemoteTranslation.count).to eq(1)
     end
@@ -26,7 +26,7 @@ describe RemoteTranslationsController do
     it "create remote translation when same remote translation with error_message is enqueued" do
       create(:remote_translation, remote_translatable: debate, locale: :es, error_message: "Has errors")
 
-      post :create, remote_translations: @remote_translations_params
+      post :create, params: { remote_translations: @remote_translations_params }
 
       expect(RemoteTranslation.count).to eq(2)
     end
@@ -34,13 +34,13 @@ describe RemoteTranslationsController do
     it "not create remote translation when same remote translation is enqueued" do
       create(:remote_translation, remote_translatable: debate, locale: :es)
 
-      post :create, remote_translations: @remote_translations_params
+      post :create, params: { remote_translations: @remote_translations_params }
 
       expect(RemoteTranslation.count).to eq(1)
     end
 
     it "redirect_to request referer after create" do
-      post :create, remote_translations: @remote_translations_params
+      post :create, params: { remote_translations: @remote_translations_params }
 
       expect(subject).to redirect_to("any_path")
     end