From cb9c767dd50671f6cee44fbbaafc18c89f3576fd Mon Sep 17 00:00:00 2001 From: baarkerlounger <5101747+baarkerlounger@users.noreply.github.com> Date: Thu, 3 Feb 2022 09:01:36 +0000 Subject: [PATCH] Rspec rubocop cleanup (#266) * Devise token stubs * Case logs controller stubs * More devise token stubs * And another one bites the dust * 2 left * All cops pass --- .rubocop.yml | 4 ++-- app/controllers/users_controller.rb | 2 +- spec/features/form/page_routing_spec.rb | 3 ++- spec/features/organisation_spec.rb | 8 +++++--- spec/features/user_spec.rb | 8 +++++--- spec/requests/auth/passwords_controller_spec.rb | 6 ++++-- spec/requests/case_logs_controller_spec.rb | 4 +++- spec/requests/form_controller_spec.rb | 14 +++++++------- spec/requests/users_controller_spec.rb | 10 +++++++--- 9 files changed, 36 insertions(+), 23 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index cae659438..cf0c1e933 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,13 +1,13 @@ require: - rubocop-performance - rubocop-rails -# - rubocop-rspec + - rubocop-rspec inherit_gem: rubocop-govuk: - config/default.yml - config/rails.yml -# - config/rspec.yml + - config/rspec.yml AllCops: Exclude: diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index e60f9439f..eb1df74de 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -70,7 +70,7 @@ private end def find_resource - @user = User.find(params[:id]) + @user = User.find_by(id: params[:id]) end def authenticate_scope! diff --git a/spec/features/form/page_routing_spec.rb b/spec/features/form/page_routing_spec.rb index ad63321d4..ed0bf8aa7 100644 --- a/spec/features/form/page_routing_spec.rb +++ b/spec/features/form/page_routing_spec.rb @@ -14,10 +14,11 @@ RSpec.describe "Form Page Routing" do ) end let(:id) { case_log.id } + let(:validator) { case_log._validators[nil].first } before do RequestHelper.stub_http_requests - allow_any_instance_of(CaseLogValidator).to receive(:validate_pregnancy).and_return(true) + allow(validator).to receive(:validate_pregnancy).and_return(true) sign_in user end diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb index aeebc125b..1eb906212 100644 --- a/spec/features/organisation_spec.rb +++ b/spec/features/organisation_spec.rb @@ -8,11 +8,13 @@ RSpec.describe "User Features" do let(:set_password_template_id) { DeviseNotifyMailer::SET_PASSWORD_TEMPLATE_ID } let(:notify_client) { instance_double(Notifications::Client) } let(:reset_password_token) { "MCDH5y6Km-U7CFPgAMVS" } + let(:devise_notify_mailer) { DeviseNotifyMailer.new } before do - allow_any_instance_of(DeviseNotifyMailer).to receive(:notify_client).and_return(notify_client) - allow_any_instance_of(DeviseNotifyMailer).to receive(:host).and_return("test.com") - allow_any_instance_of(User).to receive(:set_reset_password_token).and_return(reset_password_token) + allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer) + allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client) + allow(devise_notify_mailer).to receive(:host).and_return("test.com") + allow(Devise.token_generator).to receive(:generate).and_return(reset_password_token) allow(notify_client).to receive(:send_email).and_return(true) sign_in user end diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index ed022868c..10b091d37 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -6,12 +6,14 @@ RSpec.describe "User Features" do let(:reset_password_template_id) { DeviseNotifyMailer::RESET_PASSWORD_TEMPLATE_ID } let(:notify_client) { instance_double(Notifications::Client) } let(:reset_password_token) { "MCDH5y6Km-U7CFPgAMVS" } + let(:devise_notify_mailer) { DeviseNotifyMailer.new } before do - allow_any_instance_of(DeviseNotifyMailer).to receive(:notify_client).and_return(notify_client) - allow_any_instance_of(DeviseNotifyMailer).to receive(:host).and_return("test.com") + allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer) + allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client) + allow(devise_notify_mailer).to receive(:host).and_return("test.com") allow(notify_client).to receive(:send_email).and_return(true) - allow_any_instance_of(User).to receive(:set_reset_password_token).and_return(reset_password_token) + allow(Devise.token_generator).to receive(:generate).and_return(reset_password_token) end context "when the user navigates to case logs" do diff --git a/spec/requests/auth/passwords_controller_spec.rb b/spec/requests/auth/passwords_controller_spec.rb index 23a4b23d3..ce0c8f1d8 100644 --- a/spec/requests/auth/passwords_controller_spec.rb +++ b/spec/requests/auth/passwords_controller_spec.rb @@ -5,9 +5,11 @@ RSpec.describe Auth::PasswordsController, type: :request do let(:params) { { user: { email: email } } } let(:page) { Capybara::Node::Simple.new(response.body) } let(:notify_client) { instance_double(Notifications::Client) } + let(:devise_notify_mailer) { DeviseNotifyMailer.new } before do - allow_any_instance_of(DeviseNotifyMailer).to receive(:notify_client).and_return(notify_client) + allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer) + allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client) allow(notify_client).to receive(:send_email).and_return(true) end @@ -25,7 +27,7 @@ RSpec.describe Auth::PasswordsController, type: :request do context "when a password reset is requested with an email that doesn't exist in the system" do before do - allow_any_instance_of(described_class).to receive(:is_navigational_format?).and_return(false) + allow(Devise.navigational_formats).to receive(:include?).and_return(false) end let(:email) { "madeup_email@test.com" } diff --git a/spec/requests/case_logs_controller_spec.rb b/spec/requests/case_logs_controller_spec.rb index 56867a1f9..09f14089e 100644 --- a/spec/requests/case_logs_controller_spec.rb +++ b/spec/requests/case_logs_controller_spec.rb @@ -1,4 +1,5 @@ require "rails_helper" +require_relative "../request_helper" RSpec.describe CaseLogsController, type: :request do let(:owning_organisation) { FactoryBot.create(:organisation) } @@ -426,7 +427,8 @@ RSpec.describe CaseLogsController, type: :request do context "when a case log deletion fails" do before do - allow_any_instance_of(CaseLog).to receive(:discard).and_return(false) + allow(CaseLog).to receive(:find_by).and_return(case_log) + allow(case_log).to receive(:discard).and_return(false) delete "/logs/#{id}", headers: headers end diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb index 93f3bfb14..3d77492b7 100644 --- a/spec/requests/form_controller_spec.rb +++ b/spec/requests/form_controller_spec.rb @@ -230,9 +230,10 @@ RSpec.describe FormController, type: :request do Form::Question.new("tenant_code", { "type" => "text" }, nil), ] end + let(:page) { case_log.form.get_page("accessibility_requirements") } it "updates both question fields" do - allow_any_instance_of(Form::Page).to receive(:expected_responses).and_return(questions_for_page) + allow(page).to receive(:expected_responses).and_return(questions_for_page) post "/logs/#{case_log.id}/form", params: case_log_form_params case_log.reload @@ -244,10 +245,7 @@ RSpec.describe FormController, type: :request do end context "with conditional routing" do - before do - allow_any_instance_of(CaseLogValidator).to receive(:validate_pregnancy).and_return(true) - end - + let(:validator) { case_log._validators[nil].first } let(:case_log_form_conditional_question_yes_params) do { id: case_log.id, @@ -257,7 +255,6 @@ RSpec.describe FormController, type: :request do }, } end - let(:case_log_form_conditional_question_no_params) do { id: case_log.id, @@ -267,7 +264,6 @@ RSpec.describe FormController, type: :request do }, } end - let(:case_log_form_conditional_question_wchair_yes_params) do { id: case_log.id, @@ -278,6 +274,10 @@ RSpec.describe FormController, type: :request do } end + before do + allow(validator).to receive(:validate_pregnancy).and_return(true) + end + it "routes to the appropriate conditional page based on the question answer of the current page" do post "/logs/#{case_log.id}/form", params: case_log_form_conditional_question_yes_params expect(response).to redirect_to("/logs/#{case_log.id}/conditional-question-yes-page") diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 6b4af4d55..59997f3ce 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -8,9 +8,11 @@ RSpec.describe UsersController, type: :request do let(:new_value) { "new test name" } let(:params) { { id: user.id, user: { name: new_value } } } let(:notify_client) { instance_double(Notifications::Client) } + let(:devise_notify_mailer) { DeviseNotifyMailer.new } before do - allow_any_instance_of(DeviseNotifyMailer).to receive(:notify_client).and_return(notify_client) + allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer) + allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client) allow(notify_client).to receive(:send_email).and_return(true) end @@ -84,7 +86,8 @@ RSpec.describe UsersController, type: :request do end before do - allow_any_instance_of(User).to receive(:reset_password_sent_at).and_return(4.hours.ago) + allow(User).to receive(:find_or_initialize_with_error_by).and_return(user) + allow(user).to receive(:reset_password_sent_at).and_return(4.hours.ago) put "/users/password", headers: headers, params: params end @@ -197,8 +200,9 @@ RSpec.describe UsersController, type: :request do context "when the update fails to persist" do before do - allow_any_instance_of(User).to receive(:update).and_return(false) sign_in user + allow(User).to receive(:find_by).and_return(user) + allow(user).to receive(:update).and_return(false) patch "/users/#{user.id}", headers: headers, params: params end