From 250949dd1442872396a0037ac66e8ee0b956ea2a Mon Sep 17 00:00:00 2001 From: Todd Sedano Date: Wed, 27 Nov 2013 11:47:00 -0800 Subject: [PATCH] Fixing issue: https://github.com/professor/whiteboard/issues/248 --- app/controllers/effort_logs_controller.rb | 65 +------------------ app/mailers/effort_log_mailer.rb | 12 ---- .../midweek_warning.html.erb | 3 +- .../midweek_warning.text.erb | 3 +- app/views/effort_logs/index.html.erb | 14 ++-- lib/tasks/cron.rake | 2 +- .../effort_log_midweek_warning_email.rake | 6 -- .../effort_logs_controller_spec.rb | 45 ++----------- 8 files changed, 21 insertions(+), 129 deletions(-) diff --git a/app/controllers/effort_logs_controller.rb b/app/controllers/effort_logs_controller.rb index 3dc21f219..7532c8947 100644 --- a/app/controllers/effort_logs_controller.rb +++ b/app/controllers/effort_logs_controller.rb @@ -13,13 +13,9 @@ def create_midweek_warning_email if (!EffortLog.log_effort_week?(Date.today.cwyear, Date.today.cweek)) #We skip weeks that students aren't taking courses logger.info "There is no class this week, so we won't remind students to log effort" -# flash[:error] = 'Students are taking courses this week' -# redirect_to(root_path) return end - @people_with_effort = Array.new - @people_without_effort = Array.new random_scotty_saying = ScottyDogSaying.all.sample.saying courses = Course.remind_about_effort_course_list @@ -27,27 +23,6 @@ def create_midweek_warning_email create_midweek_warning_email_for_course(random_scotty_saying, course_id) end - #No longer required for all SE students - #(with_effort, without_effort) = create_midweek_warning_email_for_se_students(random_scotty_saying) - #@people_with_effort = @people_with_effort + with_effort - #@people_without_effort = @people_without_effort + without_effort - - EffortLogMailer.midweek_warning_admin_report(random_scotty_saying, @people_without_effort, @people_with_effort).deliver - - logger.info "There were #{@people_without_effort.size} without effort." - @people_without_effort.each do |user| - logger.debug "#{user}" - end - logger.info "There were #{@people_with_effort.size} with effort." - @people_with_effort.each do |user| - logger.debug "#{user}" - end - -# respond_to do |format| -# format.html # index.html.erb -# end -# render(:text => "E-Mail sent") - end def create_midweek_warning_email_for_course(random_scotty_saying, course_id) @@ -60,49 +35,11 @@ def create_midweek_warning_email_for_course(random_scotty_saying, course_id) logger.debug "** user #{user.human_name}" effort_log = EffortLog.where(:user_id => user.id, :week_number => week_number, :year => year).first if (!user.emailed_recently(:effort_log)) - if ((effort_log.nil? || effort_log.sum == 0)&&(!user.emailed_recently(:effort_log))) - # logger.debug "** sent email to #{user.human_name} (#{user.id}) for #{week_number} of #{year} in course #{course_id}" - create_midweek_warning_email_send_it(random_scotty_saying, user.id) - @people_without_effort << user.human_name - else - logger.debug "** no email to #{user.human_name} (#{user.id}) for #{week_number} of #{year} in course #{course_id}" - @people_with_effort << user.human_name - end - user.effort_log_warning_email = Time.now - user.save - end - end - end - - def create_midweek_warning_email_for_se_students(random_scotty_saying) - users_without_effort = [] - users_with_effort = [] - year = Date.today.cwyear - week_number = Date.today.cweek - users = User.where(:masters_program => "SE", :is_active => true, :is_alumnus => false) - - users.each do |user| - effort_log = EffortLog.latest_for_user(user.id, week_number, year) - if (!user.emailed_recently(:effort_log)) - if ((effort_log.nil? || effort_log.sum == 0)&&(!user.emailed_recently(:effort_log))) - create_midweek_warning_email_send_it(random_scotty_saying, user.id) - users_without_effort << user.human_name - else - users_with_effort << user.human_name - end + EffortLogMailer.midweek_warning(random_scotty_saying, user).deliver user.effort_log_warning_email = Time.now user.save end end - return users_without_effort, users_with_effort - end - - - def create_midweek_warning_email_send_it(random_scotty_saying, id) - user = User.find_by_id(id) - EffortLogMailer.midweek_warning(random_scotty_saying, user).deliver - #render(:text => "
" + email.encoded + "
") - end def create_endweek_faculty_email diff --git a/app/mailers/effort_log_mailer.rb b/app/mailers/effort_log_mailer.rb index eb302bfb5..2e89451db 100644 --- a/app/mailers/effort_log_mailer.rb +++ b/app/mailers/effort_log_mailer.rb @@ -5,23 +5,11 @@ class EffortLogMailer < ActionMailer::Base def midweek_warning(saying, user) @user = user - email_with_name = @user.human_name + ' <' + @user.email + '>' - attachments["ScottyDogLandscape.jpg"] = File.read("#{Rails.root}/public/images/ScottyDogLandscape.jpg") mail(:to => @user.email, :subject => "Scotty Dog says: #{saying}", :date => Time.now) end - def midweek_warning_admin_report(saying, people_without_effort, people_with_effort) - @saying = saying - @people_without_effort = people_without_effort - @people_with_effort = people_with_effort - #consider using an list array for this up e.g EmailsArray.all.map(&:email).join(", ") - faculty = ['todd.sedano@sv.cmu.edu', 'ed.katz@sv.cmu.edu', 'mel.rosso-llopart@sv.cmu.edu'] - - mail(:to => faculty, :subject => "Scotty Dog Midweek Warning Email Summary", :date => Time.now) - end - def endweek_admin_report(course_id, course_name, faculty_emails) @course_id = course_id @course_name = course_name diff --git a/app/views/effort_log_mailer/midweek_warning.html.erb b/app/views/effort_log_mailer/midweek_warning.html.erb index 7c66edbb4..37c4f7bd5 100644 --- a/app/views/effort_log_mailer/midweek_warning.html.erb +++ b/app/views/effort_log_mailer/midweek_warning.html.erb @@ -1,5 +1,6 @@

To <%= @user.human_name %>,

-

This is a friendly reminder that you have not created an effort log or logged effort for this week.

+

Please update your effort logs today. Effort logs are due each week on Sunday by midnight (Pacific Time). You have a 24 hour grace period. You will not be able to create or edit after logs after the grace period. + We don't remember what we had for lunch yesterday and we don't think you'll remember what you were doing last week.

To log effort



diff --git a/app/views/effort_log_mailer/midweek_warning.text.erb b/app/views/effort_log_mailer/midweek_warning.text.erb index b0c712658..a0359ab07 100644 --- a/app/views/effort_log_mailer/midweek_warning.text.erb +++ b/app/views/effort_log_mailer/midweek_warning.text.erb @@ -1,6 +1,7 @@ To <%= @user.human_name %>, -This is a friendly reminder that you have not created an effort log or logged effort for this week. +Please update your effort logs today. Effort logs are due each week on Sunday by midnight (Pacific Time). You have a 24 hour grace period. You will not be able to create or edit after logs after the grace period. + We don't remember what we had for lunch yesterday and we don't think you'll remember what you were doing last week. href=http://whiteboard.sv.cmu.edu/effort_logs diff --git a/app/views/effort_logs/index.html.erb b/app/views/effort_logs/index.html.erb index 8009fdbbb..56c92b6bf 100644 --- a/app/views/effort_logs/index.html.erb +++ b/app/views/effort_logs/index.html.erb @@ -1,6 +1,14 @@ <% content_for :title, 'Effort Logs' %>

Listing Effort Logs

+
+ +

Note: Every week, you'll be able to create a new effort log entry. Effort logs are due each week on Sunday by + midnight (Pacific Time). + You have a 24 hour grace period. You will not be able to create or edit after logs after the grace period. We don't + remember what we had for lunch yesterday and we don't think you'll remember what you were doing last week.

+
+ @@ -53,11 +61,5 @@
Week number

- -

Note: Every week, you'll be able to create a new effort log entry. Effort logs are due each week on Sunday by - midnight (Pacific Time). - You have a 24 hour grace period. You will not be able to create or edit after logs after the grace period. We don't - remember what we had for lunch yesterday and we don't think you'll remember what you were doing last week.

-

<%= link_to 'Back', '/' %>


diff --git a/lib/tasks/cron.rake b/lib/tasks/cron.rake index 799b5096e..e10a32ad2 100644 --- a/lib/tasks/cron.rake +++ b/lib/tasks/cron.rake @@ -13,7 +13,7 @@ task :cron do puts "---Running task :cron" - if Date.today.wday == 5 # run on Fridays + if Date.today.wday == 5 || Date.today.wday == 0 # run on Fridays and Sundays puts "----Updating cmu:effort_log_midweek_warning_email" Rake::Task['cmu:effort_log_midweek_warning_email'].invoke puts "----done." diff --git a/lib/tasks/effort_log_midweek_warning_email.rake b/lib/tasks/effort_log_midweek_warning_email.rake index 787000f9e..3adb50751 100644 --- a/lib/tasks/effort_log_midweek_warning_email.rake +++ b/lib/tasks/effort_log_midweek_warning_email.rake @@ -4,12 +4,6 @@ require 'rake' namespace :cmu do desc "Create midweek Scotty Dog warning emails" task(:effort_log_midweek_warning_email => :environment) do - ## RAILS_ENV = ENV['RAILS_ENV'] = 'test' - - #if !File.exists?(Dir.pwd+"/config/database.yml") - # FileUtils.copy(Dir.pwd+"/config/database.cc.yml", Dir.pwd+"/config/database.yml") - #end - a = EffortLogsController.new() a.create_midweek_warning_email() diff --git a/spec/controllers/effort_logs_controller_spec.rb b/spec/controllers/effort_logs_controller_spec.rb index e3280b1b3..1b8b55f01 100644 --- a/spec/controllers/effort_logs_controller_spec.rb +++ b/spec/controllers/effort_logs_controller_spec.rb @@ -126,7 +126,7 @@ context "and there are courses that have students" do before do Course.stub(:remind_about_effort_course_list).and_return([FactoryGirl.create(:mfse)]) - @person = User.new(:first_name => "Frodo", :last_name => "Baggins", :human_name => "") + @person = User.new(:first_name => "Frodo", :last_name => "Baggins", :human_name => "Frodo Baggins") Course.any_instance.stub(:registered_students).and_return([@person]) end @@ -137,19 +137,19 @@ it "it should send an email if the person has never been emailed" do @person.effort_log_warning_email = nil - subject.should_receive(:create_midweek_warning_email_send_it) + EffortLogMailer.should_receive(:midweek_warning).and_return(double(EffortLogMailer, :deliver => true)) subject.create_midweek_warning_email end it "it should send an email if the person not been emailed recently " do @person.effort_log_warning_email = Time.now - 3.day - subject.should_receive(:create_midweek_warning_email_send_it) + EffortLogMailer.should_receive(:midweek_warning).and_return(double(EffortLogMailer, :deliver => true)) subject.create_midweek_warning_email end it "it should not send an email if the person has been emailed recently" do @person.effort_log_warning_email = Time.now - subject.should_not_receive(:create_midweek_warning_email_send_it) + EffortLogMailer.should_not_receive(:midweek_warning) subject.create_midweek_warning_email end end @@ -162,19 +162,19 @@ it "it should send an email if the person has never been emailed" do @person.effort_log_warning_email = nil - subject.should_receive(:create_midweek_warning_email_send_it) + EffortLogMailer.should_receive(:midweek_warning).and_return(double(EffortLogMailer, :deliver => true)) subject.create_midweek_warning_email end it "it should send an email if the person not been emailed recently " do @person.effort_log_warning_email = Time.now - 3.day - subject.should_receive(:create_midweek_warning_email_send_it) + EffortLogMailer.should_receive(:midweek_warning).and_return(double(EffortLogMailer, :deliver => true)) subject.create_midweek_warning_email end it "it should not send an email if the person has been emailed recently" do @person.effort_log_warning_email = Time.now - subject.should_not_receive(:create_midweek_warning_email_send_it) + EffortLogMailer.should_not_receive(:midweek_warning) subject.create_midweek_warning_email end end @@ -189,36 +189,5 @@ end end - context "it should send midweekly reminder email to SE students" do - it "who have not logged effort" do - person_who_needs_reminder = FactoryGirl.create(:student_sam, :effort_log_warning_email => Date.today - 1.day) - User.stub(:where).and_return([person_who_needs_reminder]) - EffortLog.stub!(:latest_for_user).and_return(nil) - (people_without_effort, people_with_effort) = subject.create_midweek_warning_email_for_se_students("random saying") - people_without_effort[0].should == person_who_needs_reminder.human_name - people_with_effort.size.should == 0 - end - - it "but skip those who have already been emailed" do - person_whose_been_reminded = FactoryGirl.create(:faculty_frank, :effort_log_warning_email => Date.today) - User.stub(:where).and_return([person_whose_been_reminded]) - EffortLog.stub!(:latest_for_user).and_return(nil) - - (people_without_effort, people_with_effort) = subject.create_midweek_warning_email_for_se_students("random saying") - people_without_effort.size.should == 0 - people_with_effort.size.should == 0 - end - - it "and not bother people who have logged effort" do - person_who_has_logged_effort = FactoryGirl.create(:admin_andy, :effort_log_warning_email => Date.today - 7.days) - User.stub(:where).and_return([person_who_has_logged_effort]) - EffortLog.stub(:latest_for_user).and_return(mock_model(EffortLog, :sum => 1)) - - (people_without_effort, people_with_effort) = subject.create_midweek_warning_email_for_se_students("random saying") - people_without_effort.size.should == 0 - people_with_effort[0].should == person_who_has_logged_effort.human_name - end - - end end \ No newline at end of file