From b3a958b6fb33b1e5ee6d9456e49d99a64aba1ddd Mon Sep 17 00:00:00 2001 From: Bill Cromie Date: Fri, 22 Apr 2016 16:44:43 -0400 Subject: [PATCH] this commit will add: - letters instead of numbers for time slot selection in sms - filter available slots by user and person reservations - resend available slots if person selects reserved slot - change how a person declines over sms - sends url of calendar for person in every offer sms specs passing! Oof. So, callbacks to create things suck. more callback futzing. let vs let! when you need a new thing every time around. potentially a fix for our callback heisenbugs more attempts to prevent heisenbugs. --- .travis.yml | 2 +- Guardfile | 2 +- app/controllers/v2/reservations_controller.rb | 3 +- .../v2/sms_reservations_controller.rb | 48 +++++++++++++--- app/models/v2/event.rb | 42 +++++++++++--- app/models/v2/reservation.rb | 7 ++- app/sms/application_sms.rb | 5 ++ app/sms/event_invitation_sms.rb | 16 +++--- app/sms/time_slot_not_available_sms.rb | 47 ++++++++++++++++ spec/controllers/calendar_controller_spec.rb | 9 +-- .../v2/sms_reservations_controller_spec.rb | 56 ++++++++++++++++--- spec/factories/event_invitations.rb | 3 +- ...to_interview_invitation_over_email_spec.rb | 4 +- .../sms_invitation_to_phone_call_spec.rb | 22 +++----- spec/models/concerns/calendarable_spec.rb | 9 +-- spec/rails_helper.rb | 2 + 16 files changed, 213 insertions(+), 64 deletions(-) create mode 100644 app/sms/time_slot_not_available_sms.rb diff --git a/.travis.yml b/.travis.yml index cf229df4c..6c20e9092 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,8 @@ language: ruby +cache: bundler sudo: required dist: precise bundler_args: "--jobs=4 --retry=3" -cache: bundler before_install: - openssl aes-256-cbc -K $encrypted_bde597b39305_key -iv $encrypted_bde597b39305_iv -in deploy_key.enc -out deploy_key -d - echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config diff --git a/Guardfile b/Guardfile index 75f8e0835..d7319bf96 100644 --- a/Guardfile +++ b/Guardfile @@ -60,7 +60,7 @@ group :red_green_refactor, halt_on_fail: true do watch('app/sms/event_invitation_sms.rb') { 'spec/features/sms_invitation_to_phone_call_spec.rb' } watch('app/sms/reservation_sms.rb') { 'spec/controllers/v2/sms_reservations_controller_spec.rb' } watch('app/sms/decline_invitation_sms.rb') { 'spec/controllers/v2/sms_reservations_controller_spec.rb' } - + watch('app/sms/time_slot_not_available_sms.rb') { 'spec/controllers/v2/sms_reservations_controller_spec.rb' } watch('app/controllers/v2/event_invitations_controller.rb') { 'spec/features/invite_person_to_phone_call_spec.rb' } watch('app/controllers/v2/event_invitations_controller.rb') { 'spec/features/sms_invitation_to_phone_call_spec.rb' } watch('app/controllers/v2/reservations_controller.rb') { 'spec/features/person_responds_to_interview_invitation_over_email_spec.rb' } diff --git a/app/controllers/v2/reservations_controller.rb b/app/controllers/v2/reservations_controller.rb index 43c15332b..fb2226826 100644 --- a/app/controllers/v2/reservations_controller.rb +++ b/app/controllers/v2/reservations_controller.rb @@ -14,8 +14,7 @@ class V2::ReservationsController < ApplicationController def new event = V2::Event.find_by(id: event_params[:event_id]) @person = Person.find_by(token: person_params[:token]) - all_slots = event.available_time_slots - @available_time_slots = filter_reservations([@person, event.user], all_slots) + @available_time_slots = event.available_time_slots(@person) @reservation = V2::Reservation.new(time_slot: V2::TimeSlot.new) end diff --git a/app/controllers/v2/sms_reservations_controller.rb b/app/controllers/v2/sms_reservations_controller.rb index 9753861bb..da45c8fdd 100644 --- a/app/controllers/v2/sms_reservations_controller.rb +++ b/app/controllers/v2/sms_reservations_controller.rb @@ -1,18 +1,22 @@ class V2::SmsReservationsController < ApplicationController skip_before_action :authenticate_user! + # rubocop:disable Metrics/MethodLength def create - send_error_notification && return unless only_numbers_and_at_least_two_of_them? - - if selection == 0 + send_error_notification && return unless valid_message? + if declined? send_decline_notifications(person, event) else - reservation = V2::Reservation.create(person: person, time_slot: time_slot) - send_notifications(person, reservation) + reservation = V2::Reservation.new(person: person, time_slot: time_slot) + if reservation.save + send_notifications(person, reservation) + else + resend_available_slots(person, event) + end end - render text: 'OK' end + # rubocop:enable Metrics/MethodLength private @@ -27,15 +31,21 @@ def person # TODO: needs to be smarter in the edge case where # there are more than 9 slot options and 0 comes up again def selection - message.last.to_i + slot_letter = message.downcase.delete('^a-z') + # "a".ord - ("A".ord + 32) == 0 + # "b".ord - ("A".ord + 32) == 1 + # (0 + 97).chr == a + # (1 + 97).chr == b + slot_letter.ord - ('A'.ord + 32) end def event - V2::Event.find(message.chop) + event_id = message.delete('^0-9') + V2::Event.find_by(id: event_id) end def time_slot - event.time_slots[selection - 1] + event.time_slots[selection] end def send_notifications(person, reservation) @@ -52,10 +62,30 @@ def send_error_notification render text: 'OK' end + def resend_available_slots(person, event) + ::TimeSlotNotAvailableSms.new(to: person, event: event).send + end + def only_numbers_and_at_least_two_of_them? message =~ /^\d(\d)+\s?$/ end + def declined? + # up to 10k events. + message.downcase =~ /^\d{1,5}-decline?/ + end + + def letters_and_numbers_only? + # up to 10k events + message.downcase =~ /\b\d{1,5}[a-z]\b/ + end + + def valid_message? + return true if declined? + return true if letters_and_numbers_only? + false + end + def reservation_params params.permit(:From, :Body) end diff --git a/app/models/v2/event.rb b/app/models/v2/event.rb index aa464f002..20c7a3711 100644 --- a/app/models/v2/event.rb +++ b/app/models/v2/event.rb @@ -28,14 +28,42 @@ class V2::Event < ActiveRecord::Base delegate :duration, to: :event_invitation delegate :buffer, to: :event_invitation - def available_time_slots - available_time_slots = time_slots.find_all { |slot| !slot.reservation.present? } - available_time_slots || [] - end + def available_time_slots(person = nil) + available_slots = time_slots.find_all do |slot| + !slot.reservation.present? + end + + available_slots = filter_reservations([user, person].compact, available_slots) - # for calendaring - def person - nil + available_slots || [] end + private + + def filter_reservations(arr_obj, slots) + arr_obj.each do |obj| + obj.reload + slots = filter_obj_reservations(obj, slots) + end + slots + end + + def filter_obj_reservations(obj, slots) + unless slots.empty? + res = obj.v2_reservations.joins(:time_slot). + where('v2_time_slots.start_time >=?', + DateTime.now.in_time_zone) + + # TODO: refactor + # filtering out slots that overlap. Tricky. + slots = slots.select do |s| + res.any? { |r| not_overlap?(r, s) } + end unless res.empty? + end + slots + end + + def not_overlap?(one, other) + !((one.start_time - other.end_time) * (other.start_time - one.end_time) >= 0) + end end diff --git a/app/models/v2/reservation.rb b/app/models/v2/reservation.rb index bcc63bbb4..de1702763 100644 --- a/app/models/v2/reservation.rb +++ b/app/models/v2/reservation.rb @@ -25,11 +25,14 @@ class V2::Reservation < ActiveRecord::Base has_one :event_invitation, through: :event belongs_to :person + # we almost always need the time_slots and event + default_scope { includes(:time_slot, :event) } + validates :person, presence: true validates :time_slot, presence: true - # we almost always need the time_slots and event - default_scope { includes(:time_slot, :event) } + # can't have the same time slot id twice. + validates :time_slot, uniqueness: true # these overlap validations are super tricksy. # do we check this here? diff --git a/app/sms/application_sms.rb b/app/sms/application_sms.rb index 64af386ef..7bc6d05a2 100644 --- a/app/sms/application_sms.rb +++ b/app/sms/application_sms.rb @@ -8,4 +8,9 @@ def initialize(*) ) @application_number = ENV['TWILIO_NUMBER'] end + + def slot_id_to_char(id) + raise ArgumentError if id >= 26 + (id + 97).chr + end end diff --git a/app/sms/event_invitation_sms.rb b/app/sms/event_invitation_sms.rb index 764bffaae..7579b2517 100644 --- a/app/sms/event_invitation_sms.rb +++ b/app/sms/event_invitation_sms.rb @@ -20,21 +20,19 @@ def send private + # rubocop:disable Metrics/MethodLength, def body body = "Hello #{to.first_name},\n" body << "#{event.description}\n" - body << "If you're available, would you so kind to select one of the possible times below," body << " by texting back its respective number?\n\n" - - body << "#{event.id}0) Decline\n" - event.available_time_slots.each_with_index do |slot, i| - body << "#{event.id}#{i+1}) #{slot.to_time_and_weekday}\n" + event.available_time_slots(to).each_with_index do |slot, i| + body << "'#{event.id}#{slot_id_to_char(i)}' for #{slot.to_time_and_weekday}\n" end - + body << "Or visit https://#{ENV['PRODUCTION_SERVER']}/calendar/?token=#{@to.token} to pick a time.\n" + body << "If none of these times work, please respond with: #{@event.id}0) to decline\n" body << "\nThanks in advance for you time!\n\n" - - # TODO: signature should be configurable - body << 'Best, Kimball team' + body << 'Best, Kimball team' # TODO: signature should be configurable end + # rubocop:enable Metrics/MethodLength, end diff --git a/app/sms/time_slot_not_available_sms.rb b/app/sms/time_slot_not_available_sms.rb new file mode 100644 index 000000000..86c19cab5 --- /dev/null +++ b/app/sms/time_slot_not_available_sms.rb @@ -0,0 +1,47 @@ +# TODO: needs a spec. The spec for SmsReservationsController covers it, +# but a unit test would make coverage more robust +class TimeSlotNotAvailableSms < ApplicationSms + attr_reader :to, :event + + def initialize(to:, event:) + super + @to = to + @event = event + end + + def send + client.messages.create( + from: application_number, + to: to.phone_number, + body: body + ) + end + + def body + body = "Sorry, that time is not longer available for: \n" + body << "#{event.description}\n" + available_slots = event.available_time_slots(to) + body << generate_slot_messages(available_slots) + body << "\nThanks!\n\n" + body << 'Best, Kimball team' # TODO: signature should be configurable + end + + # rubocop:disable Metrics/MethodLength, + def generate_slot_messages(available_slots) + msg = '' + if available_slots.length >= 1 + msg << "If you'd like to still attend," + msg << ' would you so kind to select one of the possible times below,' + msg << " by texting back its respective number?\n\n" + available_slots.each_with_index do |slot, i| + msg << "'#{event.id}#{slot_id_to_char(i)}' for #{slot.to_time_and_weekday}\n" + end + msg << "Or visit https://#{ENV['PRODUCTION_SERVER']}/calendar/?token=#{to.token} to pick a time.\n" + msg << "If none of these times work, please respond with: '#{event.id}-Decline' to decline\n" + else + msg << "There are no more available times for this event.\n" + end + msg + end + # rubocop:enable Metrics/MethodLength, +end diff --git a/spec/controllers/calendar_controller_spec.rb b/spec/controllers/calendar_controller_spec.rb index 95578ac05..5bbf885ab 100644 --- a/spec/controllers/calendar_controller_spec.rb +++ b/spec/controllers/calendar_controller_spec.rb @@ -1,11 +1,12 @@ require 'rails_helper' RSpec.describe CalendarController, type: :controller do - let(:event_invitation) { FactoryGirl.create(:event_invitation) } - let(:person) { event_invitation.invitees.sample } - let(:time_slot) { event_invitation.event.time_slots.sample } - let(:reservation) { V2::Reservation.create(person: person, time_slot: time_slot) } let(:user) { FactoryGirl.create(:user) } + let!(:event_invitation) { FactoryGirl.create(:event_invitation) } + let!(:person) { event_invitation.invitees.sample } + let!(:time_slot) { event_invitation.event.time_slots.sample } + let!(:reservation) { V2::Reservation.create(person: person, time_slot: time_slot) } + describe 'admin user logged in' do before(:each) do diff --git a/spec/controllers/v2/sms_reservations_controller_spec.rb b/spec/controllers/v2/sms_reservations_controller_spec.rb index 8f77e5726..9114405d4 100644 --- a/spec/controllers/v2/sms_reservations_controller_spec.rb +++ b/spec/controllers/v2/sms_reservations_controller_spec.rb @@ -4,10 +4,16 @@ include SmsSpec::Helpers let(:twilio_phone_number) { ENV['TWILIO_NUMBER'] } + let(:user) { FactoryGirl.create(:user) } let!(:event_invitation) { FactoryGirl.create(:event_invitation) } + + let(:user_2) { FactoryGirl.create(:user) } let(:research_subject) { event_invitation.invitees.first } - # this is to ensure we create events. occasionally, the callback is too slow - let(:event) { event_invitation.event } + let(:research_subject_2) { event_invitation.invitees.last } + let!(:event) { + event_invitation.event.user_id = user.id + event_invitation.event + } before do clear_messages @@ -20,14 +26,14 @@ subject { post :create, message } context 'with existing time slot option' do - let(:selected_number) { 1 } + let(:selected_number) { 'a' } let(:body) { "#{event.id}#{selected_number}" } - let(:selected_time) { event.reload.time_slots.first.to_weekday_and_time } + let(:selected_time) { V2::Event.find(event.id).time_slots.first.to_weekday_and_time } it 'reserves the time slot for this person' do + event_invitation.save! + event.save subject - event_invitation.reload - event.reload expect(event.time_slots.first.reservation).to_not be_nil end @@ -46,8 +52,8 @@ end end - context 'with 0' do - let(:body) { "#{event.id}0" } + context 'with eventid-decline' do + let(:body) { "#{event.id}-decline" } it 'sends out a confirmation sms for this person' do subject @@ -74,6 +80,40 @@ expect(current_text_message.body).to eql "Sorry, that's not a valid option" end end + + context 'attempts to select a reserved slot' do + let(:selected_number) { 'a' } + let(:body) { "#{event.id}#{selected_number}" } + let!(:time_slot) { event.available_time_slots.first } + let(:reservation){ + V2::Reservation.create(person: research_subject_2, + time_slot: time_slot) + } + let!(:selected_time) { time_slot.to_weekday_and_time } + + it 'does not create a new reservation' do + reservation.save + subject + expect(V2::Reservation.count).to eq(1) + open_last_text_message_for research_subject.phone_number + not_expected = "An interview has been booked for #{selected_time}" + expect(current_text_message.body).to_not eql not_expected + end + + it 'resends available slots' do + reservation.save + subject + open_last_text_message_for research_subject.phone_number + message_body = current_text_message.body + expect(message_body).to have_text(event.description) + + expected = event.time_slots.last.to_time_and_weekday + expect(message_body).to have_text(expected) + + person_token = research_subject.token + expect(message_body).to have_text(person_token) + end + end end end end diff --git a/spec/factories/event_invitations.rb b/spec/factories/event_invitations.rb index c588b7627..4c32658bd 100644 --- a/spec/factories/event_invitations.rb +++ b/spec/factories/event_invitations.rb @@ -15,7 +15,8 @@ event_invitation.email_addresses = invitees.collect(&:email_address).join(',') start_time = Faker::Time.forward(2) - end_time = start_time + 15.minutes + # at least two slots should be created here + end_time = start_time + (15 * rand(2..5)).minutes event_invitation.date = start_time.strftime('%m/%d/%Y') event_invitation.start_time = start_time.strftime('%H:%M') diff --git a/spec/features/person_responds_to_interview_invitation_over_email_spec.rb b/spec/features/person_responds_to_interview_invitation_over_email_spec.rb index c95a85773..5c75a411f 100644 --- a/spec/features/person_responds_to_interview_invitation_over_email_spec.rb +++ b/spec/features/person_responds_to_interview_invitation_over_email_spec.rb @@ -15,13 +15,11 @@ @event.available_time_slots.each do |time| expect(page).to have_content time.to_time_and_weekday end + selected_time = @event.available_time_slots.first.to_weekday_and_time find('#v2_reservation_time_slot_id_1').set(true) - click_button 'Confirm reservation' - selected_time = @event.available_time_slots.first.to_weekday_and_time - expect(page).to have_content "An interview has been booked for #{selected_time}" admin_email = @event.user.email diff --git a/spec/features/sms_invitation_to_phone_call_spec.rb b/spec/features/sms_invitation_to_phone_call_spec.rb index 1a1a09fcd..7fd2d2d7a 100644 --- a/spec/features/sms_invitation_to_phone_call_spec.rb +++ b/spec/features/sms_invitation_to_phone_call_spec.rb @@ -33,23 +33,19 @@ open_last_text_message_for @research_subject.phone_number - expected = "Hello #{@research_subject.first_name},\n" + slots=[] - expected << "#{event.description}\n" - - expected << "If you're available, would you so kind to select one of the possible times below," - expected << " by texting back its respective number?\n\n" - - expected << "#{event.id}0) Decline\n" - event.available_time_slots.each_with_index do |slot, i| - expected << "#{event.id}#{i+1}) #{slot.to_time_and_weekday}\n" + event.available_time_slots(@research_subject).each_with_index do |slot, i| + slots << "'#{event.id}#{(i + 97).chr}' for #{slot.to_time_and_weekday}\n" end - expected << "\nThanks in advance for you time!\n\n" + # should use the person's name. + expect(current_text_message.body).to have_text(@research_subject.first_name) - # TODO: signature should be configurable - expected << 'Best, Kimball team' + # should describe the event. + expect(current_text_message.body).to have_text(event.description) - expect(current_text_message.body).to eql expected + # needs each slot to be in the message + slots.each { |s| expect(current_text_message.body).to have_text(s) } end end diff --git a/spec/models/concerns/calendarable_spec.rb b/spec/models/concerns/calendarable_spec.rb index 9f486cba6..1229710ae 100644 --- a/spec/models/concerns/calendarable_spec.rb +++ b/spec/models/concerns/calendarable_spec.rb @@ -2,12 +2,13 @@ describe Calendarable do context 'reservation calendar' do - let(:event_invitation) { FactoryGirl.create(:event_invitation) } - let(:person) { event_invitation.invitees.sample } - let(:time_slot) { event_invitation.event.time_slots.sample } - let(:reservation) { V2::Reservation.create(person: person, time_slot: time_slot) } + let!(:event_invitation) { FactoryGirl.create(:event_invitation) } + let!(:person) { event_invitation.invitees.sample } + let!(:time_slot) { event_invitation.event.time_slots.sample } + let!(:reservation) { V2::Reservation.create(person: person, time_slot: time_slot) } it 'can generate an ical event' do + reservation.reload expect(reservation).to respond_to(:to_ics) expect(reservation.to_ics.description).to eq(reservation.description) end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 84f2b5d5c..ff41f7353 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -35,6 +35,8 @@ config.filter_rails_from_backtrace! + config.example_status_persistence_file_path = "#{::Rails.root}/tmp/rspec.data" + config.use_transactional_fixtures = false config.before(:suite) do