From aaa2572a92920acaa376b62d1d92b4767266839c Mon Sep 17 00:00:00 2001 From: Jared Cosulich Date: Tue, 18 Apr 2017 10:31:28 -0400 Subject: [PATCH] queued questions --- app/controllers/attempts_controller.rb | 3 +- app/models/attempt.rb | 13 +++ app/models/recipient_schedule.rb | 53 ++++++++---- ..._queued_questions_to_recipient_schedule.rb | 6 ++ db/schema.rb | 4 +- spec/controllers/attempts_controller_spec.rb | 45 +++++----- spec/lib/tasks/survey_rake_spec.rb | 85 +++++++++++++++---- spec/models/attempt_spec.rb | 6 +- spec/spec_helper.rb | 8 +- 9 files changed, 164 insertions(+), 59 deletions(-) create mode 100644 db/migrate/20170417203127_add_queued_questions_to_recipient_schedule.rb diff --git a/app/controllers/attempts_controller.rb b/app/controllers/attempts_controller.rb index 3f5b990e..f1d5fe4b 100644 --- a/app/controllers/attempts_controller.rb +++ b/app/controllers/attempts_controller.rb @@ -13,9 +13,8 @@ class AttemptsController < ApplicationController return end - attempt.update_attributes( + attempt.save_response( answer_index: twilio_params[:Body].to_i, - responded_at: Time.new, twilio_details: twilio_params.to_h.to_yaml ) diff --git a/app/models/attempt.rb b/app/models/attempt.rb index 628fb1dc..e5cd0b62 100644 --- a/app/models/attempt.rb +++ b/app/models/attempt.rb @@ -14,6 +14,7 @@ class Attempt < ApplicationRecord scope :for_category, -> (category) { joins(:question).merge(Question.for_category(category)) } scope :for_school, -> (school) { joins(:recipient).merge(Recipient.for_school(school)) } scope :with_response, -> { where('answer_index is not null or open_response_id is not null')} + scope :with_no_response, -> { where('answer_index is null and open_response_id is null')} def messages [ @@ -44,6 +45,18 @@ class Attempt < ApplicationRecord question.options[answer_index - 1] end + def save_response(answer_index: nil, twilio_details: nil, responded_at: Time.new) + update_attributes( + answer_index: answer_index, + twilio_details: twilio_details, + responded_at: responded_at + ) + + if recipient_schedule.queued_question_ids.present? + recipient_schedule.update(next_attempt_at: Time.new) + end + end + private def update_school_categories diff --git a/app/models/recipient_schedule.rb b/app/models/recipient_schedule.rb index e9d6f1c6..7eb20419 100644 --- a/app/models/recipient_schedule.rb +++ b/app/models/recipient_schedule.rb @@ -17,30 +17,53 @@ class RecipientSchedule < ApplicationRecord } def next_question - upcoming = upcoming_question_ids.split(/,/) - Question.where(id: upcoming.first).first + if queued_question_ids.present? + next_question_id = queued_question_ids.split(/,/).first + else + next_question_id = upcoming_question_ids.split(/,/).first + end + Question.where(id: next_question_id).first end def attempt_question(send_message: true, question: next_question) return if recipient.opted_out? - attempt = recipient.attempts.create( - schedule: schedule, - recipient_schedule: self, - question: question - ) + unanswered_attempt = recipient.attempts.with_no_response.last + + return if question.nil? && unanswered_attempt.nil? + + if unanswered_attempt.nil? + attempt = recipient.attempts.create( + schedule: schedule, + recipient_schedule: self, + question: question + ) + end + + if send_message && (unanswered_attempt || attempt).send_message + upcoming = upcoming_question_ids.try(:split, /,/) || [] + queued = queued_question_ids.try(:split, /,/) || [] + attempted = attempted_question_ids.try(:split, /,/) || [] + + if question.present? + question_id = [question.id.to_s] + upcoming = upcoming - question_id + if unanswered_attempt.nil? + attempted += question_id + queued -= question_id + else + queued += question_id + end + end - if send_message && attempt.send_message - return if upcoming_question_ids.blank? - upcoming = upcoming_question_ids.split(/,/)[1..-1].join(',') - attempted = ((attempted_question_ids.try(:split, /,/) || []) + [question.id]).join(',') update_attributes( - upcoming_question_ids: upcoming, - attempted_question_ids: attempted, - last_attempt_at: attempt.sent_at, + upcoming_question_ids: upcoming.empty? ? nil : upcoming.join(','), + attempted_question_ids: attempted.empty? ? nil : attempted.join(','), + queued_question_ids: queued.empty? ? nil : queued.join(','), + last_attempt_at: (unanswered_attempt || attempt).sent_at, next_attempt_at: next_attempt_at + (60 * 60 * schedule.frequency_hours) ) end - return attempt + return (unanswered_attempt || attempt) end def self.create_for_recipient(recipient_or_recipient_id, schedule, next_attempt_at=nil) diff --git a/db/migrate/20170417203127_add_queued_questions_to_recipient_schedule.rb b/db/migrate/20170417203127_add_queued_questions_to_recipient_schedule.rb new file mode 100644 index 00000000..766782fe --- /dev/null +++ b/db/migrate/20170417203127_add_queued_questions_to_recipient_schedule.rb @@ -0,0 +1,6 @@ +class AddQueuedQuestionsToRecipientSchedule < ActiveRecord::Migration[5.0] + def change + add_column :recipient_schedules, :queued_question_ids, :string + add_column :recipients, :teacher, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 751d7e29..74c1f97e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170412204724) do +ActiveRecord::Schema.define(version: 20170417203127) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -91,6 +91,7 @@ ActiveRecord::Schema.define(version: 20170412204724) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.datetime "next_attempt_at" + t.string "queued_question_ids" end create_table "recipients", force: :cascade do |t| @@ -110,6 +111,7 @@ ActiveRecord::Schema.define(version: 20170412204724) do t.string "slug" t.integer "attempts_count", default: 0 t.integer "responses_count", default: 0 + t.string "teacher" t.index ["phone"], name: "index_recipients_on_phone", using: :btree t.index ["slug"], name: "index_recipients_on_slug", unique: true, using: :btree end diff --git a/spec/controllers/attempts_controller_spec.rb b/spec/controllers/attempts_controller_spec.rb index c7340ffe..11e517ac 100644 --- a/spec/controllers/attempts_controller_spec.rb +++ b/spec/controllers/attempts_controller_spec.rb @@ -4,38 +4,45 @@ RSpec.describe AttemptsController, type: :controller do let(:valid_session) { {} } - let(:schedule) { Schedule.new } + let!(:recipients) { create_recipients(school, 2) } + let!(:recipient_list) do + school.recipient_lists.create!(name: 'Parents', recipient_ids: recipients.map(&:id).join(',')) + end + + let!(:category) { Category.create(name: 'Test Category')} + let!(:questions) { create_questions(3, category) } + let!(:question_list) do + QuestionList.create!(name: 'Parent Questions', question_ids: questions.map(&:id).join(',')) + end + + let(:schedule) { Schedule.create(name: 'Test Schedule', question_list: question_list, recipient_list: recipient_list) } let(:school) { School.create!(name: 'School') } - let(:recipient) { school.recipients.create!(name: 'Recipient', phone: '+11231231234') } - let(:recipient_schedule) { RecipientSchedule.new } - let(:recipient2) { school.recipients.create!(name: 'Recipient2', phone: '+12342342345') } - let(:recipient_schedule2) { RecipientSchedule.new } + let(:recipient_schedule) { RecipientSchedule.create(recipient: recipients.first, schedule: schedule, next_attempt_at: Time.now) } + let(:recipient_schedule2) { RecipientSchedule.create(recipient: recipients.last, schedule: schedule, next_attempt_at: Time.now) } - let(:category) { Category.create!(name: 'Category') } - let(:question) { create_questions(1, category).first } let!(:first_attempt) { Attempt.create( schedule: schedule, - recipient: recipient, + recipient: recipients.first, recipient_schedule: recipient_schedule, - question: question + question: questions.first ) } let!(:attempt) { Attempt.create( schedule: schedule, - recipient: recipient, + recipient: recipients.first, recipient_schedule: recipient_schedule, - question: question + question: questions.first ) } let!(:attempt2) { Attempt.create( schedule: schedule, - recipient: recipient2, + recipient: recipients.last, recipient_schedule: recipient_schedule2, - question: question + question: questions.first ) } @@ -43,7 +50,7 @@ RSpec.describe AttemptsController, type: :controller do describe "POST #twilio" do context "with valid params" do let(:twilio_attributes) { - {'MessageSid' => 'ewuefhwieuhfweiuhfewiuhf','AccountSid' => 'wefiuwhefuwehfuwefinwefw','MessagingServiceSid' => 'efwneufhwuefhweiufhiuewhf','From' => '+11231231234','To' => '2223334444','Body' => '3','NumMedia' => '0'} + {'MessageSid' => 'ewuefhwieuhfweiuhfewiuhf','AccountSid' => 'wefiuwhefuwehfuwefinwefw','MessagingServiceSid' => 'efwneufhwuefhweiufhiuewhf','From' => '+0000000000','To' => '2223334444','Body' => '3','NumMedia' => '0'} } before :each do @@ -66,19 +73,19 @@ RSpec.describe AttemptsController, type: :controller do end it "sends back a message" do - expect(response.body).to eq "We've registered your response of \"Option 0:1 C\". You are the first person to respond to this question. Once more people have responded you will be able to see all responses at: http://test.host/schools/school/categories/category" + expect(response.body).to eq "We've registered your response of \"Option 0:3 C\". You are the first person to respond to this question. Once more people have responded you will be able to see all responses at: http://test.host/schools/school/categories/test-category" end context "with second response" do let(:twilio_attributes2) { - {'MessageSid' => 'fwefwefewfewfasfsdfdf','AccountSid' => 'wefwegdbvcbrtnrn','MessagingServiceSid' => 'dfvdfvegbdfb','From' => '+12342342345','To' => '2223334444','Body' => '4','NumMedia' => '0'} + {'MessageSid' => 'fwefwefewfewfasfsdfdf','AccountSid' => 'wefwegdbvcbrtnrn','MessagingServiceSid' => 'dfvdfvegbdfb','From' => '+1111111111','To' => '2223334444','Body' => '4','NumMedia' => '0'} } before :each do post :twilio, params: twilio_attributes2 end - it 'creates the second attempt with response for the question' do + it 'updates the second attempt with response for the school' do expect(attempt.question.attempts.for_school(school).with_response.count).to eq(2) end @@ -90,14 +97,14 @@ RSpec.describe AttemptsController, type: :controller do end it "sends back a message" do - expect(response.body).to eq "We've registered your response of \"Option 0:1 D\". 2 people have responded to this question so far. To see all responses visit: http://test.host/schools/school/categories/category" + expect(response.body).to eq "We've registered your response of \"Option 0:3 D\". 2 people have responded to this question so far. To see all responses visit: http://test.host/schools/school/categories/test-category" end end end context 'with stop params' do let(:twilio_attributes) { - {'MessageSid' => 'ewuefhwieuhfweiuhfewiuhf','AccountSid' => 'wefiuwhefuwehfuwefinwefw','MessagingServiceSid' => 'efwneufhwuefhweiufhiuewhf','From' => '+11231231234','To' => '2223334444','Body' => 'sToP','NumMedia' => '0'} + {'MessageSid' => 'ewuefhwieuhfweiuhfewiuhf','AccountSid' => 'wefiuwhefuwehfuwefinwefw','MessagingServiceSid' => 'efwneufhwuefhweiufhiuewhf','From' => '+0000000000','To' => '2223334444','Body' => 'sToP','NumMedia' => '0'} } it "updates the last attempt by recipient phone number" do diff --git a/spec/lib/tasks/survey_rake_spec.rb b/spec/lib/tasks/survey_rake_spec.rb index 19b34b0b..e6cfdaad 100644 --- a/spec/lib/tasks/survey_rake_spec.rb +++ b/spec/lib/tasks/survey_rake_spec.rb @@ -8,13 +8,16 @@ describe "survey:attempt_questions" do end describe "basic flow" do + let(:now) { + n = DateTime.now + n += 1.day until n.on_weekday? + return n + } let(:ready_recipient_schedule) { double('ready recipient schedule', attempt_question: nil) } let(:recipient_schedules) { double("recipient schedules", ready: [ready_recipient_schedule]) } let(:active_schedule) { double("active schedule", recipient_schedules: recipient_schedules) } it "finds all active schedules" do - now = DateTime.now - now += 1.day until now.on_weekday? date = ActiveSupport::TimeZone["UTC"].parse(now.strftime("%Y-%m-%dT20:00:00%z")) Timecop.freeze(date) @@ -36,6 +39,11 @@ describe "survey:attempt_questions" do end describe "complex flow" do + let(:now) { + n = DateTime.now + n += 1.day until n.on_weekday? + return n + } let!(:school) { School.create!(name: 'School') } @@ -64,7 +72,8 @@ describe "survey:attempt_questions" do describe 'First attempt not at specified time' do before :each do - now = DateTime.now + now = DateTime.new + now += 1.day until now.on_weekend? date = ActiveSupport::TimeZone["UTC"].parse(now.strftime("%Y-%m-%dT19:00:00%z")) Timecop.freeze(date) { subject.invoke } end @@ -76,10 +85,7 @@ describe "survey:attempt_questions" do describe 'First attempt at specified time' do - before :each do - now = DateTime.now - now += 1.day until now.on_weekday? date = ActiveSupport::TimeZone["UTC"].parse(now.strftime("%Y-%m-%dT20:00:00%z")) Timecop.freeze(date) { subject.invoke } end @@ -118,20 +124,69 @@ describe "survey:attempt_questions" do describe 'A Week Later' do before :each do - Timecop.freeze(Date.today + 10) { subject.invoke } + recipients[1].attempts.first.update_attributes( + answer_index: 4, + responded_at: Time.new + ) + Timecop.freeze(now + 7) { subject.invoke } end - it 'should create the second attempt for each recipient with a different question' do - recipients.each do |recipient| + it 'should resend the first question if unanswered by a recipient' do + [recipients[0], recipients[2]].each do |recipient| recipient.reload - expect(recipient.attempts.count).to eq(2) + expect(recipient.attempts.count).to eq(1) attempt = recipient.attempts.last - expect(attempt.sent_at).to be_present + expect(attempt.sent_at).to be > 1.day.ago expect(attempt.answer_index).to be_nil + end + end - first_attempt = recipient.attempts.first - expect(first_attempt.question).to_not eq(attempt.question) + it 'should create the second attempt with a new question for each recipient who has answered the first question' do + recipient = recipients[1] + recipient.reload + expect(recipient.attempts.count).to eq(2) + attempt = recipient.attempts.last + expect(attempt.sent_at).to be_present + expect(attempt.answer_index).to be_nil + + first_attempt = recipient.attempts.first + expect(first_attempt.question).to_not eq(attempt.question) + end + end + + describe 'And Then A Recipient Answers The First Attempt' do + before :each do + recipients[1].attempts.first.save_response(answer_index: 4) + + Timecop.freeze(now + 7) + recipients.each do |recipient| + recipient_schedule = schedule.recipient_schedules.for(recipient).first + recipient_schedule.attempt_question end + + @existing_message_count = FakeSMS.messages.length + + Timecop.freeze(now + 8) + recipients[2].attempts.first.save_response(answer_index: 3) + subject.invoke + end + + it 'should create the second attempt with a new question for each recipient who just answered the first question' do + recipient = recipients[2] + recipient.reload + expect(recipient.attempts.count).to eq(2) + attempt = recipient.attempts.last + expect(attempt.sent_at).to be_present + expect(attempt.answer_index).to be_nil + + first_attempt = recipient.attempts.first + expect(first_attempt.question).to_not eq(attempt.question) + end + + it 'should not send anything to anyone else' do + expect(FakeSMS.messages.length).to eq(@existing_message_count + 1) + expect(recipients[0].attempts.count).to eq(1) + expect(recipients[1].attempts.count).to eq(2) end end end @@ -140,9 +195,7 @@ describe "survey:attempt_questions" do before :each do recipients[1].update_attributes(opted_out: true) - - now = DateTime.now - now += 1.day until now.on_weekday? + date = ActiveSupport::TimeZone["UTC"].parse(now.strftime("%Y-%m-%dT20:00:00%z")) Timecop.freeze(date) { subject.invoke } end diff --git a/spec/models/attempt_spec.rb b/spec/models/attempt_spec.rb index 8cc5477a..dd222ace 100644 --- a/spec/models/attempt_spec.rb +++ b/spec/models/attempt_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Attempt, type: :model do let!(:school) { School.create!(name: 'School') } - let!(:recipient) { create_recipients(school, 1).first } + let!(:recipient) { school.recipients.create(name: 'name', phone: "#{1}" * 9) } let!(:recipient_list) do school.recipient_lists.create!(name: 'Parents', recipient_ids: "#{recipient.id}") end @@ -94,7 +94,7 @@ RSpec.describe Attempt, type: :model do it 'should contact the Twilio API' do expect(FakeSMS.messages.length).to eq(1) expect(FakeSMS.messages.last.body).to eq("Question 0:1\n\rOption 0:1 A: Reply 1\n\rOption 0:1 B: Reply 2\n\rOption 0:1 C: Reply 3\n\rOption 0:1 D: Reply 4\n\rOption 0:1 E: Reply 5\n\rReply 'stop' to stop these messages.") - expect(FakeSMS.messages.last.to).to eq('1111111111') + expect(FakeSMS.messages.last.to).to eq('111111111') end it 'should update sent_at' do @@ -102,7 +102,7 @@ RSpec.describe Attempt, type: :model do end it 'should update the recipient phone number' do - expect(attempt.recipient.reload.phone).to eq('+11111111111') + expect(attempt.recipient.reload.phone).to eq('+1111111111') end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a0f32982..af65bbd1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -132,7 +132,9 @@ class FakeSMS end def get(sid) - Struct.new(:to).new("+1#{self.class.messages[sid].to}") + phone = self.class.messages[sid].to + phone = "+1#{phone}" unless phone.starts_with?("+1") + Struct.new(:to).new(phone) end end @@ -140,8 +142,8 @@ def create_recipients(school, count) recipients = [] count.times do |i| recipients << school.recipients.create( - name: "Person#{count}", - phone: "#{count}" * 10, + name: "Person#{i}", + phone: "+" + ("#{i}" * 10), ) end return recipients