Merge branch 'main' into full-import-optimisation

app/controllers/users_controller.rb

@@ -53,7 +53,7 @@ class UsersController < ApplicationController
       if @user == current_user
         bypass_sign_in @user
         flash[:notice] = I18n.t("devise.passwords.updated") if user_params.key?("password")
-        if user_params.key?("email") && FeatureToggle.new_email_journey?
+        if user_params.key?("email")
           flash[:notice] = I18n.t("devise.email.updated", email: @user.unconfirmed_email)
         end
 
@@ -66,7 +66,7 @@ class UsersController < ApplicationController
         elsif user_params[:active] == "true"
           @user.send_confirmation_instructions
           flash[:notice] = I18n.t("devise.activation.reactivated", user_name:)
-        elsif user_params.key?("email") && FeatureToggle.new_email_journey?
+        elsif user_params.key?("email")
           flash[:notice] = I18n.t("devise.email.updated", email: @user.unconfirmed_email)
         end
         redirect_to user_path(@user)

app/mailers/devise_notify_mailer.rb

@@ -40,13 +40,11 @@ class DeviseNotifyMailer < Devise::Mailer
 
   def confirmation_instructions(record, token, _opts = {})
     if email_changed?(record)
-      if new_email_journey?
       send_email_changed_to_old_email(record)
       send_email_changed_to_new_email(record, token)
-      else
+    elsif !record.confirmed? && record.unconfirmed_email
       send_confirmation_email(record.unconfirmed_email, record, token, record.unconfirmed_email)
-        send_confirmation_email(record.email, record, token, record.unconfirmed_email)
-      end
+      send_email_changed_to_old_email(record)
     else
       send_confirmation_email(record.email, record, token, record.email)
     end
@@ -97,16 +95,11 @@ class DeviseNotifyMailer < Devise::Mailer
       record.confirmable_template == User::CONFIRMABLE_TEMPLATE_ID && (
         record.unconfirmed_email.present? && record.unconfirmed_email != record.email)
     ) || (
-      new_email_journey? &&
       record.versions.last.changeset.key?("unconfirmed_email") &&
       record.confirmed?
     )
   end
 
-  def new_email_journey?
-    FeatureToggle.new_email_journey?
-  end
-
   def send_confirmation_email(email, record, token, username)
     url = "#{user_confirmation_url}?confirmation_token="
 

app/services/feature_toggle.rb

@@ -33,8 +33,4 @@ class FeatureToggle
   def self.deduplication_flow_enabled?
     !Rails.env.production? && !Rails.env.staging?
   end
-
-  def self.new_email_journey?
-    !Rails.env.production?
-  end
 end

lib/tasks/correct_incref_values.rake

@@ -1,6 +1,6 @@
 desc "Alter incref values for non imported lettings logs in the database"
 task correct_incref_values: :environment do
-  LettingsLog.where(old_id: nil, net_income_known: 0).update_all(incref: 0)
-  LettingsLog.where(old_id: nil, net_income_known: 1).update_all(incref: 2)
-  LettingsLog.where(old_id: nil, net_income_known: 2).update_all(incref: 1)
+  LettingsLog.where(old_id: nil, net_income_known: 0).update_all(incref: 0, updated_at: Time.zone.now)
+  LettingsLog.where(old_id: nil, net_income_known: 1).update_all(incref: 2, updated_at: Time.zone.now)
+  LettingsLog.where(old_id: nil, net_income_known: 2).update_all(incref: 1, updated_at: Time.zone.now)
 end

spec/lib/tasks/correct_incref_values_spec.rb

@@ -39,6 +39,14 @@ RSpec.describe "correct_incref_values" do
         task.invoke
         expect(lettings_log.reload.incref).to eq(1)
       end
+
+      it "updates updated_at value" do
+        lettings_log.updated_at = Time.zone.local(2021, 3, 3)
+        lettings_log.save!(validate: false)
+        expect(lettings_log.updated_at.to_date).to eq(Time.zone.local(2021, 3, 3))
+        task.invoke
+        expect(lettings_log.reload.updated_at.to_date).to eq(Time.zone.today)
+      end
     end
   end
 end

spec/requests/users_controller_spec.rb

@@ -1584,27 +1584,6 @@ RSpec.describe UsersController, type: :request do
 
           before do
             user.legacy_users.destroy_all
-
-            allow(FeatureToggle).to receive(:new_email_journey?).and_return(false)
-          end
-
-          it "allows changing email and dpo" do
-            request
-            user.reload
-            expect(user.unconfirmed_email).to eq(new_email)
-            expect(user.is_data_protection_officer?).to be true
-            expect(user.is_key_contact?).to be true
-          end
-
-          it "sends a confirmation email to both emails" do
-            expect(notify_client).to receive(:send_email).with(email_address: new_email, template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation:).once
-            expect(notify_client).to receive(:send_email).with(email_address: user.email, template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation:).once
-            request
-          end
-
-          context "with new email journy enabled" do
-            before do
-              allow(FeatureToggle).to receive(:new_email_journey?).and_return(true)
           end
 
           it "shows flash notice" do
@@ -1637,6 +1616,47 @@ RSpec.describe UsersController, type: :request do
 
             patch "/users/#{other_user.id}", headers:, params:
           end
+
+          context "when user has never confirmed email address" do
+            let(:old_email) { "old@test.com" }
+            let(:new_email) { "new@test.com" }
+            let(:other_user) { create(:user, organisation: user.organisation, email: old_email, confirmed_at: nil) }
+
+            before do
+              other_user.legacy_users.destroy_all
+            end
+
+            it "shows flash notice" do
+              patch("/users/#{other_user.id}", headers:, params:)
+
+              expect(flash[:notice]).to eq("An email has been sent to #{new_email} to confirm this change.")
+            end
+
+            it "sends new flow emails" do
+              expect(notify_client).to receive(:send_email).with(
+                email_address: new_email,
+                template_id: User::RECONFIRMABLE_TEMPLATE_ID,
+                personalisation: {
+                  name: new_name,
+                  email: new_email,
+                  organisation: other_user.organisation.name,
+                  link: include("/account/confirmation?confirmation_token="),
+                },
+              ).once
+
+              expect(notify_client).to receive(:send_email).with(
+                email_address: old_email,
+                template_id: User::FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
+                personalisation: {
+                  new_email:,
+                  old_email:,
+                },
+              ).once
+
+              expect(notify_client).not_to receive(:send_email)
+
+              patch "/users/#{other_user.id}", headers:, params:
+            end
           end
         end
 
@@ -1769,37 +1789,6 @@ RSpec.describe UsersController, type: :request do
 
               before do
                 other_user.legacy_users.destroy_all
-
-                allow(FeatureToggle).to receive(:new_email_journey?).and_return(false)
-              end
-
-              it "does not update the password" do
-                expect { patch "/users/#{other_user.id}", headers:, params: }
-                  .not_to change(other_user, :encrypted_password)
-              end
-
-              it "does update other values" do
-                expect { patch "/users/#{other_user.id}", headers:, params: }
-                  .to change { other_user.reload.name }.from("Danny Rojas").to("new name")
-              end
-
-              it "allows changing email" do
-                expect { patch "/users/#{other_user.id}", headers:, params: }
-                  .to change { other_user.reload.unconfirmed_email }.from(nil).to(new_email)
-              end
-
-              it "sends a confirmation email to both emails" do
-                expect(notify_client).to receive(:send_email).with(email_address: other_user.email, template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation:).once
-                expect(notify_client).to receive(:send_email).with(email_address: new_email, template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation:).once
-
-                expect(notify_client).not_to receive(:send_email)
-
-                patch "/users/#{other_user.id}", headers:, params:
-              end
-
-              context "with new user email flow enabled" do
-                before do
-                  allow(FeatureToggle).to receive(:new_email_journey?).and_return(true)
               end
 
               it "shows flash notice" do
@@ -1836,7 +1825,6 @@ RSpec.describe UsersController, type: :request do
           end
         end
       end
-      end
 
       context "when the update fails to persist" do
         before do