Merge pull request #5415 from LeGorge/4672-supervisor-mailer-no-recen…

…t-sign-in-list

fix: fixes the list of "no_recent_sign_in" on supervisor weekly_digest

app/mailers/supervisor_mailer.rb

@@ -10,6 +10,7 @@ def weekly_digest(supervisor)
     @supervisor = supervisor
     @casa_organization = supervisor.casa_org
     @inactive_messages = InactiveMessagesService.new(supervisor).inactive_messages
+    @inactive_volunteers = supervisor.inactive_volunteers
     if supervisor.receive_reimbursement_email
       mileage_report_attachment = MileageReport.new(@casa_organization.id).to_csv
       attachments["mileage-report-#{Time.current.strftime("%Y-%m-%d")}.csv"] = mileage_report_attachment

app/models/supervisor.rb

@@ -37,6 +37,14 @@ def pending_volunteers
     ).where(invitation_accepted_at: nil).where.not(invitation_created_at: nil)
   end
 
+  def inactive_volunteers
+    recent_case_contact_volunteer_ids = volunteers.joins(:case_contacts).where(
+      case_contacts: {created_at: 30.days.ago..}
+    ).pluck(:id)
+
+    volunteers.no_recent_sign_in.where.not(id: recent_case_contact_volunteer_ids)
+  end
+
   def recently_unassigned_volunteers
     unassigned_supervisor_volunteers.joins(:volunteer).includes(:volunteer)
       .where(updated_at: 1.week.ago..Time.zone.now).map(&:volunteer)

app/models/user.rb

@@ -56,7 +56,7 @@ class User < ApplicationRecord
     where(casa_org_id: org.id)
   }
 
-  scope :no_recent_sign_in, -> { active.where("last_sign_in_at <= ?", 30.days.ago) }
+  scope :no_recent_sign_in, -> { active.where("last_sign_in_at <= ?", 30.days.ago).or(User.active.where(last_sign_in_at: nil)) }
 
   def casa_admin?
     is_a?(CasaAdmin)

app/views/supervisor_mailer/_no_recent_sign_in.html.erb

@@ -1,10 +1,10 @@
-<% if supervisor.volunteers.no_recent_sign_in.any? %>
+<% if inactive_volunteers.any? %>
   <tr style="font-family: Helvetica,Arial,sans-serif; box-sizing: border-box; font-size: 14px; margin: 0; text-align: left;">
     <td class="content-block" style="font-family: Helvetica,Arial,sans-serif; box-sizing: border-box; font-size: 14px; vertical-align: top; margin: 0; padding: 0 0 20px;" valign="top">
-      <b><%= "The following volunteers have not recently signed in, within 30 days" %></b>
+      <b><%= "The following volunteers have not signed in or created case contacts in the last 30 days" %></b>
       <br>
 
-      <% supervisor.volunteers.no_recent_sign_in.each do |volunteer| %>
+      <% inactive_volunteers.each do |volunteer| %>
         - <%= volunteer.display_name %>
         <br>
       <% end %>

app/views/supervisor_mailer/weekly_digest.html.erb

@@ -6,5 +6,5 @@
   <%= render("recently_unassigned_volunteers", supervisor: @supervisor) %>
   <%= render("additional_notes", inactive_messages: @inactive_messages) %>
   <%= render("pending_volunteers", supervisor: @supervisor) %>
-  <%= render("no_recent_sign_in", supervisor: @supervisor) %>
+  <%= render("no_recent_sign_in", supervisor: @supervisor, inactive_volunteers: @inactive_volunteers) %>
 </table>

spec/mailers/supervisor_mailer_spec.rb

@@ -101,9 +101,33 @@
       expect(mail.body.encoded).to_not match("There are no additional notes.")
     end
 
-    it "displays no additional notes messsage when there are no additional notes" do
+    it "displays no additional notes message when there are no additional notes" do
       expect(mail.body.encoded).to match("There are no additional notes.")
     end
+
+    it "does not display no_recent_sign_in section" do
+      expect(mail.body.encoded).not_to match("volunteers have not signed in or created case contacts in the last 30 days")
+    end
+
+    context "when supervisor has a volunteer that has not been active for the last 30 days" do
+      let!(:volunteer) do
+        create(:volunteer, casa_org: supervisor.casa_org, supervisor: supervisor, last_sign_in_at: 31.days.ago)
+      end
+
+      it "display no_recent_sign_in section" do
+        expect(mail.body.encoded).to match("volunteers have not signed in or created case contacts in the last 30 days")
+      end
+
+      context "but volunteer has a recent case_contact to its name" do
+        let!(:recent_case_contact) do
+          create(:case_contact, casa_case: casa_case, occurred_at: 29.days.ago, creator: volunteer)
+        end
+
+        it "does not display no_recent_sign_in section" do
+          expect(mail.body.encoded).not_to match("volunteers have not signed in or created case contacts in the last 30 days")
+        end
+      end
+    end
   end
 
   describe ".invitation_instructions for a supervisor" do

spec/models/supervisor_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Supervisor, type: :model do
+  include Devise::Test::IntegrationHelpers
+
   subject(:supervisor) { create :supervisor }
 
   describe "#role" do
@@ -43,6 +45,56 @@
     end
   end
 
+  describe "not_signed_in_nor_have_case_contacts_volunteers" do
+    let(:supervisor) { create(:supervisor) }
+    let(:volunteer) { create(:volunteer, last_sign_in_at: 31.days.ago) }
+    let(:other_volunteer) { create(:volunteer, last_sign_in_at: 29.days.ago) }
+
+    subject { supervisor.inactive_volunteers }
+
+    before do
+      create(:supervisor_volunteer, supervisor: supervisor, volunteer: volunteer)
+      create(:supervisor_volunteer, supervisor: supervisor, volunteer: other_volunteer)
+    end
+
+    context "when volunteer has logged in in the last 30 days" do
+      let(:volunteer) { create(:volunteer, last_sign_in_at: 29.days.ago) }
+
+      it { is_expected.to be_empty }
+
+      context "when volunteer then logged out" do
+        it "is empty" do
+          sign_out volunteer
+
+          is_expected.to be_empty
+        end
+      end
+    end
+
+    context "when volunteer hasn't logged in in the last 30 days" do
+      it { is_expected.to contain_exactly(volunteer) }
+
+      context "when volunteer is inactive" do
+        let(:volunteer) { create(:volunteer, last_sign_in_at: 31.days.ago, active: false) }
+
+        it { is_expected.to be_empty }
+      end
+
+      context "when volunteer has never logged in" do
+        let(:volunteer) { create(:volunteer, last_sign_in_at: nil) }
+
+        it { is_expected.to contain_exactly(volunteer) }
+      end
+
+      context "when volunteer has a recent case_contact" do
+        let(:casa_case) { create(:casa_case, casa_org: supervisor.casa_org) }
+        let!(:case_contact) { create(:case_contact, casa_case: casa_case, occurred_at: 31.days.ago, creator: volunteer, created_at: 29.days.ago) }
+
+        it { is_expected.to be_empty }
+      end
+    end
+  end
+
   describe "change to admin" do
     it "returns true if the change was successful" do
       expect(subject.change_to_admin!).to be_truthy

spec/views/supervisor_mailer/weekly_digest.html.erb_spec.rb

@@ -17,6 +17,7 @@
       create_list :case_contact, 2, creator: volunteer, casa_case: casa_case, contact_made: false, occurred_at: Time.current - 6.days
       @case_contact = create :case_contact, creator: volunteer, casa_case: casa_case, contact_made: true, occurred_at: Time.current - 6.days
       assign :supervisor, supervisor
+      assign :inactive_volunteers, []
       sign_in supervisor
       @inactive_messages = InactiveMessagesService.new(supervisor).inactive_messages
       render template: "supervisor_mailer/weekly_digest"
@@ -40,6 +41,7 @@
     before(:each) do
       sign_in supervisor
       assign :supervisor, supervisor
+      assign :inactive_volunteers, []
       @inactive_messages = InactiveMessagesService.new(supervisor).inactive_messages
       render template: "supervisor_mailer/weekly_digest"
     end
@@ -55,6 +57,7 @@
       volunteer.casa_cases << casa_case
       sign_in supervisor
       assign :supervisor, supervisor
+      assign :inactive_volunteers, []
       @inactive_messages = InactiveMessagesService.new(supervisor).inactive_messages
       render template: "supervisor_mailer/weekly_digest"
     end
@@ -74,6 +77,7 @@
 
       sign_in supervisor
       assign :supervisor, supervisor
+      assign :inactive_volunteers, []
       render template: "supervisor_mailer/weekly_digest"
     end
 
@@ -91,7 +95,7 @@
       new_supervisor.volunteers << volunteer
       volunteer.supervisor_volunteer.update(is_active: true)
       assign :supervisor, supervisor
-      @inactive_messages = []
+      assign :inactive_volunteers, []
 
       render template: "supervisor_mailer/weekly_digest"
     end
@@ -107,6 +111,7 @@
       supervisor.volunteers << volunteer
       sign_in supervisor
       assign :supervisor, supervisor
+      assign :inactive_volunteers, []
       volunteer.supervisor_volunteer.update(is_active: false)
       @inactive_messages = []
       render template: "supervisor_mailer/weekly_digest"
@@ -123,10 +128,11 @@
       supervisor.volunteers << volunteer
       sign_in supervisor
       assign :supervisor, supervisor
+      assign :inactive_volunteers, supervisor.inactive_volunteers
       render template: "supervisor_mailer/weekly_digest"
     end
 
-    it { expect(rendered).to have_text("The following volunteers have not recently signed in, within 30 days") }
+    it { expect(rendered).to have_text("The following volunteers have not signed in or created case contacts in the last 30 days") }
     it { expect(rendered).to have_text("- #{volunteer.display_name}") }
   end
 end