Add issue explanation to the email

app/jobs/email_missing_addresses_csv_job.rb

@@ -5,7 +5,7 @@ class EmailMissingAddressesCsvJob < ApplicationJob
   EXPIRATION_TIME = 1.week.to_i
   MISSING_ADDRESSES_THRESHOLD = 5
 
-  def perform(user_ids, organisation, log_type, skip_uprn_issue_organisations)
+  def perform(user_ids, organisation, log_type, issue_types, skip_uprn_issue_organisations)
     csv_service = Csv::MissingAddressesCsvService.new(organisation, skip_uprn_issue_organisations)
     case log_type
     when "lettings"
@@ -27,7 +27,7 @@ class EmailMissingAddressesCsvJob < ApplicationJob
       user = User.find(id)
       next if user.blank?
 
-      CsvDownloadMailer.new.send(email_method, user, url, EXPIRATION_TIME)
+      CsvDownloadMailer.new.send(email_method, user, url, EXPIRATION_TIME, issue_types)
     end
   end
 end

app/mailers/csv_download_mailer.rb

@@ -11,19 +11,36 @@ class CsvDownloadMailer < NotifyMailer
     )
   end
 
-  def send_missing_lettings_addresses_csv_download_mail(user, link, duration)
+  def send_missing_lettings_addresses_csv_download_mail(user, link, duration, issue_types)
     send_email(
       user.email,
       CSV_MISSING_LETTINGS_ADDRESSES_DOWNLOAD_TEMPLATE_ID,
-      { name: user.name, link:, duration: ActiveSupport::Duration.build(duration).inspect },
+      { name: user.name, link:, issue_explanation: issue_explanation(issue_types), duration: ActiveSupport::Duration.build(duration).inspect },
     )
   end
 
-  def send_missing_sales_addresses_csv_download_mail(user, link, duration)
+  def send_missing_sales_addresses_csv_download_mail(user, link, duration, issue_types)
     send_email(
       user.email,
       CSV_MISSING_SALES_ADDRESSES_DOWNLOAD_TEMPLATE_ID,
-      { name: user.name, link:, duration: ActiveSupport::Duration.build(duration).inspect },
+      { name: user.name, link:, issue_explanation: issue_explanation(issue_types), duration: ActiveSupport::Duration.build(duration).inspect },
     )
   end
+
+private
+
+  def issue_explanation(issue_types)
+    issue_type_explanations = {
+      "missing_address" => "- Full address required: The UPRN in some logs is incorrect, so address data was not imported.",
+      "missing_town" => "- Missing town or city: The town or city in some logs is missing. This data is required in the new version of CORE.",
+      "wrong_uprn" => "- UPRN may be incorrect: The UPRN in some logs may be incorrect, so
+      wrong address data was imported. We think this is an issue because in
+      some logs the UPRN is the same as the tenant code or property reference,
+      and because your organisation has submitted logs for properties in Bristol
+      for the first time.",
+    }
+
+    "Some address data is missing or incorrect. We've detected the following issues in
+    your logs imported to the new version of CORE:\n\n" + issue_types.map { |issue_type| issue_type_explanations[issue_type] }.join("\n")
+  end
 end

lib/tasks/send_missing_addresses_csv.rake

@@ -25,9 +25,13 @@ namespace :correct_addresses do
 
       missing_addresses_threshold = EmailMissingAddressesCsvJob::MISSING_ADDRESSES_THRESHOLD
       if logs_impacted_by_missing_address >= missing_addresses_threshold || logs_impacted_by_missing_town_or_city >= missing_addresses_threshold || logs_impacted_by_uprn_issue.any?
+        issue_types = []
+        issue_types << "missing_address" if logs_impacted_by_missing_address.positive?
+        issue_types << "missing_town" if logs_impacted_by_missing_town_or_city.positive?
+        issue_types << "wrong_uprn" if logs_impacted_by_uprn_issue.any?
         data_coordinators = organisation.users.where(role: 2).filter_by_active
         users_to_contact = data_coordinators.any? ? data_coordinators : organisation.users.filter_by_active
-        EmailMissingAddressesCsvJob.perform_later(users_to_contact.map(&:id), organisation, "lettings", skip_uprn_issue_organisations)
+        EmailMissingAddressesCsvJob.perform_later(users_to_contact.map(&:id), organisation, "lettings", issue_types, skip_uprn_issue_organisations)
         Rails.logger.info("Sending missing lettings addresses CSV for #{organisation.name} to #{users_to_contact.map(&:email).join(', ')}")
       else
         Rails.logger.info("Missing addresses below threshold for #{organisation.name}")
@@ -59,9 +63,13 @@ namespace :correct_addresses do
                                     end
       missing_addresses_threshold = EmailMissingAddressesCsvJob::MISSING_ADDRESSES_THRESHOLD
       if logs_impacted_by_missing_address >= missing_addresses_threshold || logs_impacted_by_missing_town_or_city >= missing_addresses_threshold || logs_impacted_by_uprn_issue.any?
+        issue_types = []
+        issue_types << "missing_address" if logs_impacted_by_missing_address.positive?
+        issue_types << "missing_town" if logs_impacted_by_missing_town_or_city.positive?
+        issue_types << "wrong_uprn" if logs_impacted_by_uprn_issue.any?
         data_coordinators = organisation.users.where(role: 2).filter_by_active
         users_to_contact = data_coordinators.any? ? data_coordinators : organisation.users.filter_by_active
-        EmailMissingAddressesCsvJob.perform_later(users_to_contact.map(&:id), organisation, "sales", skip_uprn_issue_organisations)
+        EmailMissingAddressesCsvJob.perform_later(users_to_contact.map(&:id), organisation, "sales", issue_types, skip_uprn_issue_organisations)
         Rails.logger.info("Sending missing sales addresses CSV for #{organisation.name} to #{users_to_contact.map(&:email).join(', ')}")
       else
         Rails.logger.info("Missing addresses below threshold for #{organisation.name}")

spec/jobs/email_missing_addresses_csv_job_spec.rb

@@ -29,38 +29,38 @@ describe EmailMissingAddressesCsvJob do
   context "when sending missing lettings logs csv" do
     it "uses an appropriate filename in S3" do
       expect(storage_service).to receive(:write_file).with(/missing-lettings-logs-addresses-#{organisation.name}-.*\.csv/, anything)
-      job.perform(users.map(&:id), organisation, "lettings", [1, 2])
+      job.perform(users.map(&:id), organisation, "lettings", %w[missing_address wrong_uprn], [1, 2])
     end
 
     it "creates a MissingAddressesCsvService with the correct organisation and calls create missing lettings logs adresses csv" do
       expect(Csv::MissingAddressesCsvService).to receive(:new).with(organisation, [1, 2])
       expect(missing_addresses_csv_service).to receive(:create_missing_lettings_addresses_csv)
-      job.perform(users.map(&:id), organisation, "lettings", [1, 2])
+      job.perform(users.map(&:id), organisation, "lettings", %w[missing_address wrong_uprn], [1, 2])
     end
 
     it "sends emails to all the provided users" do
-      expect(mailer).to receive(:send_missing_lettings_addresses_csv_download_mail).with(users[0], test_url, instance_of(Integer))
-      expect(mailer).to receive(:send_missing_lettings_addresses_csv_download_mail).with(users[1], test_url, instance_of(Integer))
-      job.perform(users.map(&:id), organisation, "lettings", [1, 2])
+      expect(mailer).to receive(:send_missing_lettings_addresses_csv_download_mail).with(users[0], test_url, instance_of(Integer), %w[missing_address wrong_uprn])
+      expect(mailer).to receive(:send_missing_lettings_addresses_csv_download_mail).with(users[1], test_url, instance_of(Integer), %w[missing_address wrong_uprn])
+      job.perform(users.map(&:id), organisation, "lettings", %w[missing_address wrong_uprn], [1, 2])
     end
   end
 
   context "when sending missing sales logs csv" do
     it "uses an appropriate filename in S3" do
       expect(storage_service).to receive(:write_file).with(/missing-sales-logs-addresses-#{organisation.name}-.*\.csv/, anything)
-      job.perform(users.map(&:id), organisation, "sales", [2, 3])
+      job.perform(users.map(&:id), organisation, "sales", %w[missing_town], [2, 3])
     end
 
     it "creates a MissingAddressesCsvService with the correct organisation and calls create missing sales logs adresses csv" do
       expect(Csv::MissingAddressesCsvService).to receive(:new).with(organisation, [2, 3])
       expect(missing_addresses_csv_service).to receive(:create_missing_sales_addresses_csv)
-      job.perform(users.map(&:id), organisation, "sales", [2, 3])
+      job.perform(users.map(&:id), organisation, "sales", %w[missing_town], [2, 3])
     end
 
     it "sends emails to all the provided users" do
-      expect(mailer).to receive(:send_missing_sales_addresses_csv_download_mail).with(users[0], test_url, instance_of(Integer))
-      expect(mailer).to receive(:send_missing_sales_addresses_csv_download_mail).with(users[1], test_url, instance_of(Integer))
-      job.perform(users.map(&:id), organisation, "sales", [2, 3])
+      expect(mailer).to receive(:send_missing_sales_addresses_csv_download_mail).with(users[0], test_url, instance_of(Integer), %w[missing_town])
+      expect(mailer).to receive(:send_missing_sales_addresses_csv_download_mail).with(users[1], test_url, instance_of(Integer), %w[missing_town])
+      job.perform(users.map(&:id), organisation, "sales", %w[missing_town], [2, 3])
     end
   end
 end

spec/lib/tasks/send_missing_addresses_csv_spec.rb

@@ -59,7 +59,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "lettings", [])
+          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "lettings", %w[missing_address], [])
         end
 
         it "prints out the jobs enqueued" do
@@ -78,7 +78,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", [])
+          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", %w[missing_address], [])
         end
 
         it "prints out the jobs enqueued" do
@@ -109,7 +109,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "lettings", [])
+          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "lettings", %w[missing_town], [])
         end
 
         it "prints out the jobs enqueued" do
@@ -128,7 +128,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", [])
+          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", %w[missing_town], [])
         end
 
         it "prints out the jobs enqueued" do
@@ -159,7 +159,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "lettings", [])
+          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "lettings", %w[wrong_uprn], [])
         end
 
         it "prints out the jobs enqueued" do
@@ -178,7 +178,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", [])
+          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", %w[wrong_uprn], [])
         end
 
         it "prints out the jobs enqueued" do
@@ -198,7 +198,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", [])
+          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", %w[wrong_uprn], [])
         end
 
         it "prints out the jobs enqueued" do
@@ -218,6 +218,20 @@ RSpec.describe "correct_addresses" do
         end
       end
 
+      context "when org is included in skip_uprn_issue_organisations list but faces a different issue" do
+        let!(:data_provider) { create(:user, :data_provider, organisation:, email: "data_provider3@example.com") }
+        let!(:data_provider2) { create(:user, :data_provider, organisation:, email: "data_provider4@example.com") }
+
+        before do
+          create_list(:lettings_log, 5, :imported, startdate: Time.zone.local(2023, 9, 9), address_line1: nil, town_or_city: nil, needstype: 1, old_form_id: "form_2", owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first)
+          create_list(:lettings_log, 5, :imported, startdate: Time.zone.local(2023, 9, 9), uprn: "12", propcode: "12", needstype: 1, owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first)
+        end
+
+        it "does not enqueue the job" do
+          expect { task.invoke(organisation.id.to_s) }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", %w[missing_address], [organisation.id])
+        end
+      end
+
       context "when skip_uprn_issue_organisations list is provided" do
         let!(:data_provider) { create(:user, :data_provider, organisation:, email: "data_provider3@example.com") }
         let!(:data_provider2) { create(:user, :data_provider, organisation:, email: "data_provider4@example.com") }
@@ -227,7 +241,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "does enqueues the job with correct skip_uprn_issue_organisations" do
-          expect { task.invoke("100 400") }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", [100, 400])
+          expect { task.invoke("100 400") }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", %w[wrong_uprn], [100, 400])
         end
       end
     end
@@ -290,7 +304,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke("70 90") }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "sales", [70, 90])
+          expect { task.invoke("70 90") }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "sales", %w[missing_address], [70, 90])
         end
 
         it "prints out the jobs enqueued" do
@@ -309,7 +323,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", [])
+          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", %w[missing_address], [])
         end
 
         it "prints out the jobs enqueued" do
@@ -340,7 +354,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "sales", [])
+          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "sales", %w[missing_town], [])
         end
 
         it "prints out the jobs enqueued" do
@@ -359,7 +373,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", [])
+          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", %w[missing_town], [])
         end
 
         it "prints out the jobs enqueued" do
@@ -390,7 +404,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "sales", [])
+          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "sales", %w[wrong_uprn], [])
         end
 
         it "prints out the jobs enqueued" do
@@ -409,7 +423,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", [])
+          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", %w[wrong_uprn], [])
         end
 
         it "prints out the jobs enqueued" do
@@ -429,7 +443,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "enqueues the job with correct organisations" do
-          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", [])
+          expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", %w[wrong_uprn], [])
         end
 
         it "prints out the jobs enqueued" do
@@ -458,7 +472,7 @@ RSpec.describe "correct_addresses" do
         end
 
         it "does enqueues the job with correct skip_uprn_issue_organisations" do
-          expect { task.invoke("100 400") }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", [100, 400])
+          expect { task.invoke("100 400") }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", %w[wrong_uprn], [100, 400])
         end
       end
     end