Merge pull request code-dot-org#4467 from code-dot-org/fix-workshop-surveys

fix workshop surveys

Author: laurelfan

dashboard/app/mailers/ops_mailer.rb

@@ -22,9 +22,9 @@ def script_assigned(params)
   end
 
   def unexpected_teacher_added(user, added_teachers, workshop)
+    @user = user
     @added_teachers = added_teachers
     @workshop = workshop
-    @user = user
 
     subject = "[ops notification] #{user.email} has added unexpected teachers to #{workshop.name}"
     mail content_type: 'text/html', subject: subject
@@ -34,11 +34,10 @@ def workshop_reminder(workshop, recipient)
     @workshop = workshop
     @recipient = recipient
 
-    subject = "Important: Your #{@workshop.phase_info[:long_name]} workshop is coming up in
+    subject = "Important: Your #{@workshop.phase_long_name} workshop is coming up in
        #{(@workshop.segments.first.start.to_date - Date.today).to_i} days"
-    if @workshop.phase_info[:prerequisite_phase]
-      @prerequisite_phase = ActivityConstants::PHASES[@workshop.phase_info[:prerequisite_phase]]
-      subject += ". Complete #{@prerequisite_phase[:long_name]}"
+    if @workshop.prerequisite_phase
+      subject += ". Complete #{@workshop.prerequisite_phase[:long_name]}"
     end
 
     mail content_type: 'text/html', subject: subject, to: @recipient.email, from: 'pd@code.org'

dashboard/app/models/workshop.rb

@@ -17,6 +17,8 @@
 #  index_workshops_on_program_type  (program_type)
 #
 
+require 'cdo/activity_constants'
+
 class Workshop < ActiveRecord::Base
   PROGRAM_TYPES = %w(1 2 3 4 5 6)
 
@@ -46,49 +48,67 @@ class Workshop < ActiveRecord::Base
                           association_foreign_key: 'facilitator_id',
                           join_table: 'facilitators_workshops'
 
-  def self.workshops_ending_today
+  def self.ending_today
     Workshop.joins(:segments).group(:workshop_id).having("(DATE(MAX(start)) = ?)", Date.today)
   end
 
-  def self.workshops_in_2_weeks
+  def self.in_2_weeks
     Workshop.joins(:segments).group(:workshop_id).having("(DATE(MIN(start)) = DATE_ADD(?, INTERVAL 2 WEEK))", Date.today)
   end
 
-  def self.workshops_in_3_days
+  def self.in_3_days
     Workshop.joins(:segments).group(:workshop_id).having("DATE(MIN(start)) = (DATE_ADD(?, INTERVAL 3 DAY))", Date.today)
   end
 
   def phase_info
     ActivityConstants::PHASES[self.phase]
   end
 
-  def program_type_info
-    ActivityConstants::PHASES[self.program_type.to_i]
+  def phase_long_name
+    return nil unless phase_info
+    phase_info[:long_name]
   end
 
-  def self.send_automated_emails
-    [Workshop.workshops_in_2_weeks, Workshop.workshops_in_3_days, Workshop.workshops_ending_today].each do |workshop_list|
-      workshop_list.each do |workshop|
-        teachers = workshop.teachers
-        drop_ins = workshop.unexpected_teachers
-        facilitators = workshop.facilitators
-        district_contacts = workshop.district_contacts
-        [teachers, drop_ins, facilitators, district_contacts].each do |recipient_list|
-          recipient_list.each do |recipient|
-            if EmailValidator::email_address?(recipient.email)
-              if workshop.segments.first.start.to_date == Date.today
-                logger.debug("Sending exit survey info to #{recipient.email}")
-                OpsMailer.exit_survey_information(workshop, recipient).deliver_now
-              else
-                logger.debug("Sending email reminder to #{recipient.email}")
-                OpsMailer.workshop_reminder(workshop, recipient).deliver_now
-              end
-            else
-              logger.debug("Cannot send email to #{recipient.email} because it is not a valid email address")
-            end
-          end
-        end
-      end
+  def prerequisite_phase
+    return nil unless phase_info
+    ActivityConstants::PHASES[phase_info[:prerequisite_phase]]
+  end
+
+  def automated_email_recipients
+    (teachers + unexpected_teachers + facilitators + district_contacts).uniq
+  end
+
+  def send_reminders
+    automated_email_recipients.each do |recipient|
+      next unless EmailValidator::email_address?(recipient.email)
+      logger.debug("Sending email reminder to #{recipient.email}")
+      OpsMailer.workshop_reminder(self, recipient).deliver_now
+    end
+  end
+
+  def self.send_2_week_reminders
+    in_2_weeks.each(&:send_reminders)
+  end
+
+  def self.send_3_day_reminders
+    in_3_days.each(&:send_reminders)
+  end
+
+  def send_exit_surveys
+    automated_email_recipients.each do |recipient|
+      next unless EmailValidator::email_address?(recipient.email)
+      logger.debug("Sending exit survey info to #{recipient.email}")
+      OpsMailer.exit_survey_information(self, recipient).deliver_now
     end
   end
+
+  def self.send_exit_surveys
+    ending_today.each(&:send_exit_surveys)
+  end
+
+  def self.send_automated_emails
+    send_2_week_reminders
+    send_3_day_reminders
+    send_exit_surveys
+  end
 end

dashboard/app/views/ops_mailer/exit_survey_information.html.haml

@@ -1,7 +1,7 @@
 %p
   Dear #{@recipient.ops_first_name},
 %p
-  Thanks for attending your #{@workshop.phase_info[:long_name]} in-person workshop.
+  Thanks for attending your #{@workshop.phase_long_name} in-person workshop.
   = render partial: 'exit_survey_url', locals: {workshop: @workshop, link_text: "short survey"}
 - if @workshop.phase == 2
   %p
@@ -21,4 +21,4 @@
       Check out this PDF
     for the many ways you can stay in touch with this community.
 %p
-  The Code.org PD team
+  The Code.org PD team

dashboard/app/views/ops_mailer/workshop_reminder.html.haml

@@ -1,7 +1,7 @@
 %p
   Dear #{@recipient.ops_first_name},
 %p
-  We’re looking forward to seeing you at your #{@workshop.phase_info[:long_name]} workshop on #{@workshop.segments.first.start.strftime("%A, %B %d")}! Together with your peers and facilitators, you’ll gain the support and resources you need to make a difference at your school.
+  We’re looking forward to seeing you at your #{@workshop.phase_long_name} workshop on #{@workshop.segments.first.start.strftime("%A, %B %d")}! Together with your peers and facilitators, you’ll gain the support and resources you need to make a difference at your school.
 %p
   As part of your commitment to this program, you are expected to participate in this workshop. If you have any conflicts with these dates, please email
   %a{href:'mailto:pd@code.org'}
@@ -34,24 +34,23 @@
 %h3
   What to bring
 - if @workshop.phase == 2
-  -if @workshop.program_type.to_i == 3
+  - if @workshop.program_type.to_i == 3
     %p
       Laptop (optional), water bottle, and snacks
-  -else
+  - else
     %p
       Laptop and charger, water bottle, and snacks
-- elsif @workshop.phase >= 4 && @workshop.phase <= 8
-  -if @workshop.program_type.to_i == 3
+- elsif @workshop.phase && (@workshop.phase >= 4 && @workshop.phase <= 8)
+  - if @workshop.program_type.to_i == 3
     %p
       Curriculum guide, green journal, laptop and charger, water bottle, and snacks
-  -else
+  - else
     %p
       Curriculum guide, laptop and charger, water bottle, and snacks
--if @workshop.instructions.to_s != ''
-  %h3
-    Notes
-  %p
-    #{@workshop.instructions}
+
+- if @workshop.instructions.to_s != ''
+  %h3 Notes
+  %p= @workshop.instructions
 %p
   See you soon,
   %br

dashboard/test/factories.rb

@@ -250,7 +250,7 @@
   end
 
   factory :workshop do
-    name 'My Workshop'
+    sequence(:name) { |n| "My Workshop #{n}" }
     program_type '1'
     location 'Somewhere, USA'
     instructions 'Test workshop instructions.'

dashboard/test/mailers/ops_mailer_test.rb

@@ -15,6 +15,8 @@ class OpsMailerTest < ActionMailer::TestCase
     assert_match "added 1 teachers", mail.body.encoded
   end
 
+  # see also tests in WorkshopTest
+
   test "workshop_in_2_weeks_reminder" do
     # Create a workshop that starts 14 days from now. Include other segments for authenticity
     @workshop_1 = create_workshop(['abc@code.org', 'xyz@code.org'], 'facilitator_1@code.org', 'district_contact_1@code.org', Date.today + 14.day)
@@ -44,7 +46,7 @@ class OpsMailerTest < ActionMailer::TestCase
     incorrect_recipients.each do |email|
       assert_equal false, correct_recipients.include?(email)
     end
-    assert_equal [@workshop_1], Workshop.workshops_in_2_weeks
+    assert_equal [@workshop_1], Workshop.in_2_weeks
   end
 
   test "workshop_in_3_days_reminder" do
@@ -76,7 +78,7 @@ class OpsMailerTest < ActionMailer::TestCase
     incorrect_recipients.each do |email|
       assert_equal false, correct_recipients.include?(email)
     end
-    assert_equal [@workshop_1], Workshop.workshops_in_3_days
+    assert_equal [@workshop_1], Workshop.in_3_days
   end
 
   test "exit_survey_information" do
@@ -107,7 +109,7 @@ class OpsMailerTest < ActionMailer::TestCase
     incorrect_recipients.each do |email|
       assert_equal false, correct_recipients.include?(email)
     end
-    assert_equal [@workshop_1], Workshop.workshops_ending_today
+    assert_equal [@workshop_1], Workshop.ending_today
   end
 
   # Only supports two teachers as an array and one facilitator as a string.

dashboard/test/models/workshop_test.rb

@@ -1,7 +1,152 @@
 require 'test_helper'
 
 class WorkshopTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  # see also tests in OpsMailerTest
+
+  def create_workshop(segment_starts_and_ends)
+    create(:workshop).tap do |workshop|
+      next if segment_starts_and_ends.blank?
+
+      workshop.segments = []
+      segment_starts_and_ends.each do |segment_start_and_end|
+        workshop.segments.build(start: segment_start_and_end.first,
+                                end: segment_start_and_end.last)
+      end
+      workshop.save!
+
+    end
+  end
+
+  setup do
+    @old_workshop = create_workshop [[Time.now.utc - 10.days, Time.now.utc - 9.days]]
+    @tomorrow_workshop = create_workshop [[Time.now.utc + 1.days, Time.now.utc + 1.days + 1.hour]]
+
+    now = Time.zone.today
+    today_start = Time.new(now.year, now.month, now.mday, 9, 0, 0, 0)
+    today_end = Time.new(now.year, now.month, now.mday, 17, 0, 0, 0)
+
+    @today_workshop = create_workshop [[today_start, today_end]]
+    # give this one a phase
+    @today_workshop.phase = 1
+    @today_workshop.save!
+
+    @today_ending_workshop = create_workshop [[today_start - 5.days, today_end - 5.days], [today_start + 5.minutes, today_end + 5.minutes]]
+    @today_starting_workshop = create_workshop [[today_start + 5.days, today_end + 5.days], [today_start + 5.minutes, today_end + 5.minutes]]
+
+    @workshop_in_2_weeks = create_workshop [[today_start + 2.weeks, today_end + 2.weeks]]
+
+    @workshop_in_3_days = create_workshop [[today_start + 3.days, today_end + 3.days]]
+  end
+
+  test "workshops ending today" do
+    assert_equal [@today_workshop, @today_ending_workshop], Workshop.ending_today
+  end
+
+  test "workshops in 2 weeks" do
+    assert_equal [@workshop_in_2_weeks], Workshop.in_2_weeks
+  end
+
+  test "workshops in 3 days" do
+    assert_equal [@workshop_in_3_days], Workshop.in_3_days
+  end
+
+  test "automated_email_recipients" do
+    expected_recipients = Set.new
+
+    # just a workshop with a facilitator
+    workshop = create :workshop
+    expected_recipients << workshop.facilitators.first
+
+    assert_equal expected_recipients, Set.new(workshop.automated_email_recipients)
+
+    # add a district contact
+    create :district_contact # not this one
+    dc = create :district_contact
+    district = create :district, contact: dc
+    cohort = workshop.cohorts.first
+    cohort.districts << district
+    cohort.save!
+    expected_recipients << dc
+
+    workshop = workshop.reload
+    assert_equal expected_recipients, Set.new(workshop.automated_email_recipients)
+
+    # add some teachers
+    create :teacher
+    create :teacher
+    t1 = create :teacher
+    t2 = create :teacher
+
+    cohort = workshop.cohorts.first
+    cohort.teachers << t1
+    cohort.teachers << t2
+    cohort.save!
+
+    expected_recipients << t1
+    expected_recipients << t2
+
+    workshop = workshop.reload
+    assert_equal expected_recipients, Set.new(workshop.automated_email_recipients)
+
+    # add another cohort
+    cohort = create :cohort
+    cohort.teachers << create(:teacher)
+    expected_recipients << cohort.teachers.first
+    cohort.save!
+    workshop.cohorts << cohort
+    workshop.save!
+
+    workshop = workshop.reload
+    assert_equal expected_recipients, Set.new(workshop.automated_email_recipients)
+
+    # add an unexpected teacher
+    t = create :teacher
+    workshop.unexpected_teachers << t
+    expected_recipients << t
+    workshop.save!
+
+    workshop = workshop.reload
+    assert_equal expected_recipients, Set.new(workshop.automated_email_recipients)
+  end
+
+  test "send automated emails" do
+    Workshop.send_automated_emails
+
+    assert_equal 11, ActionMailer::Base.deliveries.count
+
+    subjects = ActionMailer::Base.deliveries.map(&:subject)
+
+    assert_equal 7, subjects.grep(/confirmation instructions/).count
+    assert_equal 1, subjects.grep(/Important: Your workshop is coming up in 14 days/).count
+    assert_equal 1, subjects.grep(/Important: Your workshop is coming up in 3 days/).count
+    assert_equal 2, subjects.grep(/Feedback requested for your Code.org PD workshop/).count
+  end
+
+  test "phase long name" do
+    # from ActivityConstants
+    #  PHASES = {
+    #   1 => {id: 1, short_name: 'Phase 1', long_name: 'Phase 1: Online Introduction'},
+    #   2 => {id: 2, short_name: 'Phase 2', long_name: 'Phase 2: Blended Summer Study', prerequisite_phase: 1},
+
+    assert_equal 'Phase 1: Online Introduction', create(:workshop, phase: 1).phase_long_name
+    assert_equal 'Phase 2: Blended Summer Study', create(:workshop, phase: 2).phase_long_name
+
+    assert_equal nil, create(:workshop, phase: "????").phase_long_name
+    assert_equal nil, create(:workshop, phase: nil).phase_long_name
+  end
+
+  test "prerequisite phase" do
+    # from ActivityConstants
+    #  PHASES = {
+    #   1 => {id: 1, short_name: 'Phase 1', long_name: 'Phase 1: Online Introduction'},
+    #   2 => {id: 2, short_name: 'Phase 2', long_name: 'Phase 2: Blended Summer Study', prerequisite_phase: 1},
+
+    assert_equal nil, create(:workshop, phase: 1).prerequisite_phase
+    phase1 = {id: 1, short_name: 'Phase 1', long_name: 'Phase 1: Online Introduction'}
+    assert_equal phase1, create(:workshop, phase: 2).prerequisite_phase
+    assert_equal nil, create(:workshop, phase: 3).prerequisite_phase
+
+    assert_equal nil, create(:workshop, phase: "????").prerequisite_phase
+    assert_equal nil, create(:workshop, phase: nil).prerequisite_phase
+  end
 end