diff --git a/app/controllers/account/appointments_controller.rb b/app/controllers/account/appointments_controller.rb index a85694413..b59b6b1e7 100644 --- a/app/controllers/account/appointments_controller.rb +++ b/app/controllers/account/appointments_controller.rb @@ -22,13 +22,7 @@ def create @appointment = @member.appointments.new if @appointment.update(appointment_params) - message = if merge_simultaneous_appointments - "Your existing appointment scheduled for #{helpers.appointment_date_and_time(@appointment)} has been updated." - else - "Your appointment was scheduled for #{helpers.appointment_date_and_time(@appointment)}." - end - MemberMailer.with(member: @member, appointment: @appointment).appointment_confirmation.deliver_later - redirect_to account_appointments_path, success: message + merge_and_complete_appointment_update else load_holds_and_loans load_appointment_slots @@ -47,12 +41,7 @@ def update @member = current_user.member if @appointment.update(appointment_params) - message = if merge_simultaneous_appointments - "Your existing appointment scheduled for #{helpers.appointment_date_and_time(@appointment)} has been updated." - else - "Your appointment scheduled for #{helpers.appointment_date_and_time(@appointment)} was updated." - end - redirect_to account_appointments_path, success: message + merge_and_complete_appointment_update else load_holds_and_loans load_appointment_slots @@ -84,12 +73,19 @@ def load_holds_and_loans @loans = @member.loans.includes(:item, member: {appointments: :loans}).checked_out end - def merge_simultaneous_appointments + # In cases where a member already has an appointment during the selected time slot for the current appointment, + # merge those appointments together to avoid confusion for members or staff. + def merge_and_complete_appointment_update simultaneous_appointment = @member.appointments.simultaneous(@appointment).first if simultaneous_appointment simultaneous_appointment.merge!(@appointment) - true + message = "Your existing appointment scheduled for #{helpers.appointment_date_and_time(simultaneous_appointment)} has been updated." + MemberMailer.with(member: @member, appointment: simultaneous_appointment).appointment_confirmation.deliver_later + else + message = "Your appointment was scheduled for #{helpers.appointment_date_and_time(@appointment)}." + MemberMailer.with(member: @member, appointment: @appointment).appointment_confirmation.deliver_later end + redirect_to account_appointments_path, success: message end def load_appointment_for_editing diff --git a/app/views/renewal/payments/new.html.erb b/app/views/renewal/payments/new.html.erb index f73d95e65..7c0553554 100644 --- a/app/views/renewal/payments/new.html.erb +++ b/app/views/renewal/payments/new.html.erb @@ -12,7 +12,6 @@
Last year you paid <%= @amount.format(no_cents: true) %>. This year we’re encouraging our library members who can afford to do so to give a little more. As always, though, we don’t turn anyone away for an inability to pay. -
diff --git a/test/controllers/account/appointments_controller_test.rb b/test/controllers/account/appointments_controller_test.rb index d5e46cb7e..0504dc4f4 100644 --- a/test/controllers/account/appointments_controller_test.rb +++ b/test/controllers/account/appointments_controller_test.rb @@ -5,31 +5,79 @@ class AppointmentsControllerTest < ActionDispatch::IntegrationTest include Devise::Test::IntegrationHelpers setup do - @appointment = FactoryBot.build(:appointment) @user = FactoryBot.create(:user) @member = FactoryBot.create(:member, user: @user) + sign_in @user + end + + private def create_appointment @hold = FactoryBot.create(:started_hold, member: @member) + @appointment = FactoryBot.build(:appointment) @appointment.holds << @hold @appointment.member = @member @appointment.starts_at = "2020-10-05 7:00AM" @appointment.ends_at = "2020-10-05 8:00AM" @appointment.save + end - sign_in @user + private def assert_enqueued_email(mailer, method, params: {}) + assert_enqueued_with(job: ActionMailer::MailDeliveryJob, args: ->(job_arguments) { + job_mailer, job_method, _delivery, rest = *job_arguments + assert_equal mailer.to_s, job_mailer + assert_equal method.to_s, job_method + assert_equal(params, rest[:params]) + }) + end + + test "creates a new appointment to pickup items on hold" do + @hold = FactoryBot.create(:started_hold, member: @member) + @event = FactoryBot.create(:appointment_slot_event) + + post account_appointments_path, params: {appointment: { + hold_ids: [@hold.id], + comment: "Excited to start on my project!", + time_range_string: "#{@event.start}..#{@event.finish}" + }} + + @appointment = @member.appointments.last + assert_enqueued_email(MemberMailer, :appointment_confirmation, params: {member: @member, appointment: @appointment}) + + assert_redirected_to account_appointments_path + assert_equal 1, @member.appointments.count + end + + test "adds additional items to an existing appointment via creation form" do + create_appointment + @hold = FactoryBot.create(:started_hold, member: @member) + + post account_appointments_path, params: {appointment: { + hold_ids: [@hold.id], + comment: "Excited to start on my project!", + time_range_string: "#{@appointment.starts_at}..#{@appointment.ends_at}" + }} + + @appointment = @member.appointments.last + assert_enqueued_email(MemberMailer, :appointment_confirmation, params: {member: @member, appointment: @appointment}) + + assert_redirected_to account_appointments_path + assert_equal 1, @member.appointments.count end test "should cancel appointment without affecting holds" do + create_appointment delete account_appointment_path(@appointment) assert_equal 1, @member.holds.count, "Member holds should not be deleted when an appointment is cancelled" assert_redirected_to account_appointments_path end test "should get edit appointment" do + create_appointment get edit_account_appointment_path(@appointment) assert_response :success end test "should update appointment" do + create_appointment assert_equal 1, @appointment.holds.count @hold2 = FactoryBot.create(:hold, member: @member) @@ -41,6 +89,7 @@ class AppointmentsControllerTest < ActionDispatch::IntegrationTest end test "should not update appointment with invalid params" do + create_appointment put account_appointment_path(@appointment), params: {appointment: {hold_ids: [], time_range_string: @appointment.time_range_string, comment: @appointment.comment}} assert_template :edit @@ -51,6 +100,7 @@ class AppointmentsControllerTest < ActionDispatch::IntegrationTest end test "should not update appointment scheduled after any holds expire" do + create_appointment @expired_hold = create(:hold, member: @member) @expired_hold.start!(@appointment.starts_at - Hold::HOLD_LENGTH - 1.days) put account_appointment_path(@appointment), params: {appointment: {hold_ids: [@hold.id, @expired_hold.id], time_range_string: @appointment.time_range_string, comment: @appointment.comment}} diff --git a/test/factories/events.rb b/test/factories/events.rb index d58471cb9..abf05294f 100644 --- a/test/factories/events.rb +++ b/test/factories/events.rb @@ -22,4 +22,14 @@ summary { "Librarian" } description { "More information about the event" } end + + factory :appointment_slot_event, class: Event do + library { Library.first || create(:library) } + calendar_id { Event.appointment_slot_calendar_id } + calendar_event_id + start { 3.days.since } + finish { 3.days.since + 2.hours } + summary { "Appointment slot" } + description { "Pick up or drop off items" } + end end