Fix various email-related issues

Resolves #1240
- test emails for approved requests that start today
- fixed issue with emails for approved requests that start today
- delete extra space from user email subject
- remove doubled salutations
- changed admin mailer from address
- fix default emails
- rework of user email method

app/controllers/reservations_controller.rb

@@ -311,7 +311,7 @@ def checkout # rubocop:disable all
 
     # Send checkout receipts
     checked_out_reservations.each do |res|
-      UserMailer.reservation_status_update(res, true).deliver
+      UserMailer.reservation_status_update(res, 'checked out').deliver
     end
 
     # prep for receipt page and exit
@@ -399,7 +399,7 @@ def current
   end
 
   def send_receipt
-    if UserMailer.reservation_status_update(@reservation, true).deliver
+    if UserMailer.reservation_status_update(@reservation, 'checked out').deliver
       flash[:notice] = 'Successfully delivered receipt email.'
     else
       flash[:error] = 'Unable to deliver receipt email. Please contact '\
@@ -435,7 +435,8 @@ def approve_request
       "by #{current_user.md_link}"
     if @reservation.save
       flash[:notice] = 'Request successfully approved'
-      UserMailer.reservation_status_update(@reservation).deliver
+      UserMailer.reservation_status_update(@reservation,
+                                           'request approved').deliver
       redirect_to reservations_path(requested: true)
     else
       flash[:error] = 'Oops! Something went wrong. Unable to approve '\

app/mailers/admin_mailer.rb

@@ -1,6 +1,6 @@
 class AdminMailer < ActionMailer::Base
   add_template_helper(ApplicationHelper)
-  default from: 'no-reply@reservations.app'
+  default from: "no-reply@#{ ActionMailer::Base.default_url_options[:host] }"
 
   def notes_reservation_notification(notes_reservations_out,
                                      notes_reservations_in)

app/mailers/user_mailer.rb

@@ -9,23 +9,30 @@ class UserMailer < ActionMailer::Base
   end
 
   # checks the status of the current reservation and sends the appropriate email
-  # receipt forces a check out receipt to be sent
-  def reservation_status_update(reservation, receipt = false) # rubocop:disable all
-    set_app_config
-
-    @reservation = reservation
-    @status = @reservation.human_status
+  # force overrides and sends the specified email
 
-    return if !receipt && @status == 'returned' && @reservation.overdue &&
-              @reservation.equipment_model.late_fee == 0
+  VALID_STATUSES = ['checked out', 'denied', 'due today', 'missed', 'overdue',
+                    'request approved', 'requested', 'returned',
+                    'returned overdue', 'starts today']
 
-    return if receipt && @reservation.checked_out.nil?
+  def reservation_status_update(reservation, force = '') # rubocop:disable all
+    set_app_config
+    @reservation = reservation
 
-    if receipt && (@status == 'due today' || @reservation.overdue)
-      # force sending a check out receipt
-      @status = 'checked out'
+    # abort if the override is not valid
+    if !force.empty?
+      return unless VALID_STATUSES.include?(force)
+      return if force == 'checked out' && @reservation.checked_out.nil?
+      return if force == 'request approved' && !@reservation.approved?
+      @status = force
+    else
+      @status = @reservation.human_status
     end
 
+    # don't send fee emails if there's no fee
+    return if @status == 'returned overdue' &&
+              @reservation.equipment_model.late_fee == 0
+
     if @status == 'reserved'
       # we only send emails for reserved reservations if it was a request
       @status = 'request approved'
@@ -36,7 +43,7 @@ def reservation_status_update(reservation, receipt = false) # rubocop:disable al
     mail(to: @reservation.reserver.email,
          subject: '[Reservations] ' \
                   "#{@reservation.equipment_model.name.capitalize} "\
-                  " #{status_formatted}")
+                  "#{status_formatted}")
   end
 
   private

app/views/app_configs/_form.erb

@@ -65,7 +65,10 @@
       Configure notifications that will be sent to users.
     </p>
     <p>
-      Emails make use of variables; variables are placeholders that are automatically replaced with user- and reservation- specific information.<br />
+      Please note that by default all emails sent to users include a salutation (e.g. "Dear John") and a closing (e.g. "Sincerely, John Doe Library")
+    </p>
+    <p>
+    Emails make use of variables; variables are placeholders that are automatically replaced with user- and reservation- specific information.<br />
       You can use the following variables in each email:
     </p>
     <dl>

app/views/user_mailer/_due_today.html.erb

@@ -1 +1 @@
-<p><%= simple_format(replace_variables(@app_configs.upcoming_checkin_email_body)) %></p>
+<%= render html: simple_format(replace_variables(@app_configs.upcoming_checkin_email_body)).html_safe %>

app/views/user_mailer/_missed.html.erb

@@ -1 +1 @@
-<p><%= simple_format(replace_variables(@app_configs.deleted_missed_reservation_email_body)) %></p>
+<%= render html: simple_format(replace_variables(@app_configs.deleted_missed_reservation_email_body)).html_safe %>

app/views/user_mailer/_overdue.html.erb

@@ -1 +1 @@
-<p><%= simple_format(replace_variables(@app_configs.overdue_checkin_email_body)) %></p>
+<%= render html: simple_format(replace_variables(@app_configs.overdue_checkin_email_body)).html_safe %>

app/views/user_mailer/_starts_today.html.erb

@@ -1 +1 @@
-<p><%= simple_format(replace_variables(@app_configs.upcoming_checkout_email_body)) %></p>
+<%= render html: simple_format(replace_variables(@app_configs.upcoming_checkout_email_body)).html_safe %>

db/default_messages/deleted_missed_email

@@ -1,6 +1 @@
-Dear @user@,\n\n\
-  Because you have missed a scheduled equipment checkout, your reservation (number Reservation_id@) has been cancelled. If you believe this is in error, please contact an administrator.\n\n\
-  @equipment_list@\n\n\
-  Thank you,\n\
-  '@department_name@'
-
+Because you have missed a scheduled equipment checkout, your reservation has been cancelled. If you believe this is in error, please contact an administrator.

db/default_messages/overdue_email

@@ -1,10 +1,10 @@
-Dear @user@,\n\n\
-  It looks like you have overdue equipment!\n\n\
-  Please return the following equipment to us as soon as possible. Until then you will be charged a daily late fee of @late_fee@.\n\n\
-  @equipment_list@\n\n\
-  Failure to return equipment will result in the levying of replacement fees, and potential revocation of borrowing privileges.\n\n\
-  Your reservation number is Reservation_id@.\n\n\
-  Thank you,\n\
-  '@department_name@'
+It looks like you have overdue equipment!
 
+Please return the following equipment to us as soon as possible. Until then you will be charged a daily late fee of @late_fee@.
+
+@equipment_list@
+
+Failure to return equipment will result in the levying of replacement fees, and potential revocation of borrowing privileges.
+
+Your reservation number is @reservation_id@.
 

db/default_messages/upcoming_checkin_email

@@ -1,5 +1,9 @@
-  Hey there, you have equipment due! Please return the following items before 4pm on @return_date@.\n\n\
-  @equipment_list@\n\n\
-  If you fail to return your equipment on time you will be subject to a late fee of @late_fee@ per day. If you have lost the item you may additionally have to pay a replacement fee of @replacement_fee@.\n\
-  Log in to Reservations to see if any of your items are eligible for renewal. If you have further questions feel free to contact an employee of @department_name@.\n\n\
-  Your reservation number is @reservation_id@.\n\n\
+Hey there, you have equipment due! Please return the following items before 4pm on @return_date@.
+
+@equipment_list@
+
+If you fail to return your equipment on time you will be subject to a late fee of @late_fee@ per day. If you have lost the item you may additionally have to pay a replacement fee of @replacement_fee@.
+Log in to Reservations to see if any of your items are eligible for renewal. If you have further questions feel free to contact an employee of @department_name@.
+
+Your reservation number is @reservation_id@.
+

db/default_messages/upcoming_checkout_email

@@ -1,4 +1,7 @@
-  Hey there, you have equipment reserved today! Please checkout the following items today:\n\n\
-  @equipment_list@\n\n\
-  If you fail to return your equipment on time you will be subject to a late fee of @late_fee@ per day. If you have lost the item you may additionally have to pay a replacement fee of @replacement_fee@.\n\
-  Log in to Reservations to see if any of your items are eligible for renewal. If you have further questions feel free to contact an employee of @department_name@.\n\n\
+Hey there, you have equipment reserved today! Please checkout the following items today:
+
+@equipment_list@
+
+If you fail to return your equipment on time you will be subject to a late fee of @late_fee@ per day. If you have lost the item you may additionally have to pay a replacement fee of @replacement_fee@.
+Log in to Reservations to see if any of your items are eligible for renewal. If you have further questions feel free to contact an employee of @department_name@.
+

lib/tasks/setup_application.rake

@@ -94,60 +94,23 @@ namespace :app do
     end
 
     # app config default variables
-    terms_of_service_text =  'No terms of service document has been '\
-      'uploaded yet. Please navigate to http://sitelocation/app_configs to '\
-      'add a ToS and edit other application configurations.'
-    upcoming_checkin_email_body =
-      "Dear @user@,\n\n"\
-      'Hey there, you have equipment due! Please return the following items '\
-      "before 4pm on @return_date@.\n\n"\
-      "@equipment_list@\n\n"\
-      'If you fail to return your equipment on time you will be subject to '\
-      'a late fee of @late_fee@ per day. If you have lost the item you may '\
-      "additionally have to pay a replacement fee of @replacement_fee@.\n"\
-      'Log in to Reservations to see if any of your items are eligible for '\
-      'renewal. If you have further questions feel free to contact an '\
-      "employee of @department_name@.\n\n"\
-      "Your reservation number is @reservation_id@.\n\n"\
-      "Thank you,\n"\
-      "@department_name@\n\n"\
-
-    upcoming_checkout_email_body =
-      "Dear @user@,\n\n"\
-      'Hey there, you have an upcoming reservation! You have reserved the '\
-      'following item(s), which will be available today:\n\n'\
-      "@equipment_list@\n\n"\
-      'If you fail to return your equipment on time you will be subject to '\
-      'a late fee of @late_fee@ per day. If you have lost the item you may '\
-      "additionally have to pay a replacement fee of @replacement_fee@.\n"\
-      'Log in to Reservations to see if any of your items are eligible for '\
-      'renewal. If you have further questions feel free to contact an '\
-      "employee of @department_name@.\n\n"\
-      "Your reservation number is @reservation_id@.\n\n"\
-      "Thank you,\n"\
-      "@department_name@\n\n"\
-
-    overdue_checkin_email_body =
-      "Dear @user@,\n\n"\
-      "It looks like you have overdue equipment!\n\n"\
-      'Please return the following equipment to us as soon as possible. '\
-      "Until then you will be charged a daily late fee of @late_fee@.\n\n"\
-      "@equipment_list@\n\n"\
-      'Failure to return equipment will result in the levying of '\
-      'replacement fees, and potential revocation of borrowing privileges.'\
-      "\n\n"\
-      "Your reservation number is @reservation_id@.\n\n"\
-      "Thank you,\n"\
-      '@department_name@'
-
-    deleted_missed_reservation_email_body =
-      "Dear @user@,\n\n"\
-      'Because you have missed a scheduled equipment checkout, your '\
-      'reservation (number @reservation_id@) has been cancelled. If you '\
-      "believe this is in error, please contact an administrator.\n\n"\
-      "@equipment_list@\n\n"\
-      "Thank you,\n"\
-      '@department_name@'
+    DEFAULT_MSGS = File.join(Rails.root, 'db', 'default_messages')
+
+    terms_of_service_text = File.read(File.join(DEFAULT_MSGS, 'tos_text'))
+
+    upcoming_checkin_email_body = File.read(File.join(DEFAULT_MSGS,
+                                                      'upcoming_checkin_email'))
+
+    upcoming_checkout_email_body = File.read(File.join(DEFAULT_MSGS,
+                                                       'upcoming_checkout_email'
+                                                      ))
+
+    overdue_checkin_email_body = File.read(File.join(DEFAULT_MSGS,
+                                                     'overdue_email'))
+
+    deleted_missed_reservation_email_body = File.read(File.join(
+                                                        DEFAULT_MSGS,
+                                                        'deleted_missed_email'))
 
     request_text =
       'The following equipment cannot be reserved because of admin '\

spec/mailers/admin_mailer_spec.rb

@@ -5,9 +5,9 @@
     expect(@mail.to.size).to eq(1)
     expect(@mail.to.first).to eq(@app_config.admin_email)
   end
-  it 'is from no-reply@reservations.app' do
-    expect(@mail.from.size).to eq(1)
-    expect(@mail.from.first).to eq('[email protected]')
+  it "is from no-reply@#{ ActionMailer::Base.default_url_options[:host] }" do
+    expect(@mail.from).to eq("no-reply@#{
+      ActionMailer::Base.default_url_options[:host] }")
   end
   it 'should actually send the email' do
     expect(ActionMailer::Base.deliveries.count).to eq(1)

spec/mailers/user_mailer_spec.rb

@@ -65,6 +65,17 @@
         "[Reservations] #{@res.equipment_model.name} Request Approved")
     end
 
+    it 'sends approved request notifications for requests starting today' do
+      @res.update_attributes(status: 'reserved',
+                             flags: Reservation::FLAGS[:request],
+                             start_date: Time.zone.today,
+                             due_date: Time.zone.today + 1)
+      @mail = UserMailer.reservation_status_update(@res,
+                                                   'request approved').deliver
+      expect(@mail.subject).to eq(
+        "[Reservations] #{@res.equipment_model.name} Request Approved")
+    end
+
     it 'sends reminders to check-out' do
       @res.update_attributes(
         FactoryGirl.attributes_for(:upcoming_reservation))
@@ -83,7 +94,7 @@
     it 'sends check-out receipts' do
       @res.update_attributes(
         FactoryGirl.attributes_for(:checked_out_reservation))
-      @mail = UserMailer.reservation_status_update(@res, true).deliver
+      @mail = UserMailer.reservation_status_update(@res).deliver
       expect(@mail.subject).to eq(
         "[Reservations] #{@res.equipment_model.name} Checked Out")
     end
@@ -92,22 +103,22 @@
       @res.update_attributes(
         FactoryGirl.attributes_for(:valid_reservation))
       expect(@res.checked_out).to be_nil
-      @mail = UserMailer.reservation_status_update(@res, true).deliver
+      @mail = UserMailer.reservation_status_update(@res, 'checked out').deliver
       expect(@mail).to be_nil
     end
 
     it 'sends check-out receipts for reservations due today' do
       @res.update_attributes(
         FactoryGirl.attributes_for(:checked_out_reservation,
                                    due_date: Time.zone.today))
-      @mail = UserMailer.reservation_status_update(@res, true).deliver
+      @mail = UserMailer.reservation_status_update(@res, 'checked out').deliver
       expect(@mail.subject).to eq(
         "[Reservations] #{@res.equipment_model.name} Checked Out")
     end
 
     it 'sends check-out receipts for overdue reservations' do
       @res.update_attributes(FactoryGirl.attributes_for(:overdue_reservation))
-      @mail = UserMailer.reservation_status_update(@res, true).deliver
+      @mail = UserMailer.reservation_status_update(@res, 'checked out').deliver
       expect(@mail.subject).to eq(
         "[Reservations] #{@res.equipment_model.name} Checked Out")
     end
@@ -143,5 +154,13 @@
       expect(@mail.subject).to eq(
         "[Reservations] #{@res.equipment_model.name} Returned Overdue")
     end
+
+    it "doesn't send fine emails when there is no late fee" do
+      @res.update_attributes(FactoryGirl.attributes_for(:checked_in_reservation,
+                                                        :overdue))
+      @res.equipment_model.update_attributes(late_fee: 0)
+      @mail = UserMailer.reservation_status_update(@res).deliver
+      expect(@mail).to be_nil
+    end
   end
 end