From aa863240b9b4b551b9935f62a9a7e7a951637000 Mon Sep 17 00:00:00 2001 From: Seth Call Date: Fri, 1 Aug 2014 16:17:04 -0500 Subject: [PATCH] * VRFS-1992 finished (daily session email improvements) --- .../views/admin/batch_emails/_form.html.erb | 2 +- ...ms_index_sms_index_use_user_instrument.sql | 2 +- ruby/lib/jam_ruby/database.rb | 30 +++ .../models/email_batch_scheduled_sessions.rb | 105 ++++++-- ruby/spec/factories.rb | 14 +- .../email_batch_spec_scheduled_session.rb | 253 ++++++++++++++---- ruby/spec/support/utilities.rb | 5 + web/spec/factories.rb | 15 +- 8 files changed, 350 insertions(+), 76 deletions(-) diff --git a/admin/app/views/admin/batch_emails/_form.html.erb b/admin/app/views/admin/batch_emails/_form.html.erb index b04b7f0ef..d491ec0ab 100644 --- a/admin/app/views/admin/batch_emails/_form.html.erb +++ b/admin/app/views/admin/batch_emails/_form.html.erb @@ -1,4 +1,4 @@ -<%= semantic_form_for([:admin, resource], :url => resource.new_record? ? admin_batch_emails_path : "#{Gon.Global.prefix}/admin/batch_emails/#{resource.id}") do |f| %> +<%= semantic_form_for([:admin, resource], :url => resource.new_record? ? admin_batch_emails_path : "#{Gon.global.prefix}/admin/batch_emails/#{resource.id}") do |f| %> <%= f.inputs do %> <%= f.input(:from_email, :label => "From Email", :input_html => {:maxlength => 64}) %> <%= f.input(:subject, :label => "Subject", :input_html => {:maxlength => 128}) %> diff --git a/db/up/current_scores_ams_index_sms_index_use_user_instrument.sql b/db/up/current_scores_ams_index_sms_index_use_user_instrument.sql index a3f7b7b10..3d8c2b5af 100644 --- a/db/up/current_scores_ams_index_sms_index_use_user_instrument.sql +++ b/db/up/current_scores_ams_index_sms_index_use_user_instrument.sql @@ -3,7 +3,7 @@ DROP VIEW current_scores; CREATE OR REPLACE VIEW current_scores AS - SELECT * FROM (SELECT * , row_number() OVER (PARTITION BY alocidispid, blocidispid, scorer ORDER BY full_score DESC) AS pcnum FROM + SELECT * FROM (SELECT * , row_number() OVER (PARTITION BY alocidispid, blocidispid, scorer ORDER BY full_score DESC) AS pcnum FROM (SELECT * FROM (SELECT percent_rank() over (PARTITION BY alocidispid, blocidispid ORDER BY full_score ASC) AS pc, * FROM (SELECT tmp.*, (COALESCE(a_users.last_jam_audio_latency, 13) + COALESCE(b_users.last_jam_audio_latency, 13) + tmp.score) AS full_score, a_users.last_jam_audio_latency AS a_audio_latency, b_users.last_jam_audio_latency AS b_audio_latency FROM diff --git a/ruby/lib/jam_ruby/database.rb b/ruby/lib/jam_ruby/database.rb index 9b357405a..90a251a1f 100644 --- a/ruby/lib/jam_ruby/database.rb +++ b/ruby/lib/jam_ruby/database.rb @@ -54,5 +54,35 @@ module JamRuby GeoIpBlocks.connection.execute "CREATE TABLE #{copied_name} (LIKE #{table_name} INCLUDING DEFAULTS INCLUDING CONSTRAINTS INCLUDING COMMENTS INCLUDING STORAGE)" copied_name end + + def self.dump(query) + result = ActiveRecord::Base.connection.execute(query) + fields = result.fields + header = "" + fields.each do |field| + header << field + header << ', ' + end + header = header[0..header.length - 3] + puts header + + result.each do |row| + row_out = '' + fields.each do |field| + if row[field].nil? + row_out << 'NULL' + else + if block_given? + row_out << yield(field, row[field]).to_s + else + row_out << row[field].to_s + end + end + row_out << ', ' + end + row_out = row_out[0..row_out.length - 3] + puts row_out + end + end end end \ No newline at end of file diff --git a/ruby/lib/jam_ruby/models/email_batch_scheduled_sessions.rb b/ruby/lib/jam_ruby/models/email_batch_scheduled_sessions.rb index 0cded52ed..017a22443 100644 --- a/ruby/lib/jam_ruby/models/email_batch_scheduled_sessions.rb +++ b/ruby/lib/jam_ruby/models/email_batch_scheduled_sessions.rb @@ -42,6 +42,7 @@ module JamRuby # receiver_id - user ID that could be in the session # receiver_score_idx - the user's last_jam_locidispid # instrument_id - the user's matching instrument for a open session slot. If this is NULL, it means 'ANY INSTRUMENT' + # invited_user_id # # tmp_matches # ----------- @@ -105,6 +106,62 @@ module JamRuby self.update_attribute(:test_emails, @counters.inspect) end + def query + ActiveRecord::Base.connection.execute(< '#{earliest_session_create_time}' AND + msess.created_at < '#{latest_session_create_time}' AND + scheduled_start >= '#{earliest_session_start_time}' AND + (rrrs.rsvp_slot_id IS NULL OR rrrs.chosen != TRUE)) AS tmp_candidate_sessions +INNER JOIN + (SELECT + users.id AS receiver_id, + users.last_jam_locidispid AS receiver_score_idx, + mi.instrument_id + INTO TEMP TABLE tmp_candidate_recipients + FROM users + INNER JOIN musicians_instruments AS mi ON mi.user_id = users.id + INNER JOIN tmp_candidate_sessions ON tmp_candidate_sessions.instrument_id = mi.instrument_id OR + tmp_candidate_sessions.is_unstructured_rsvp = TRUE OR + tmp_candidate_sessions.invited_user_id = users.id + + WHERE + users.last_jam_locidispid IS NOT NULL AND + users.musician = TRUE AND + users.subscribe_email = TRUE) AS tmp_candidate_recipients +INNER JOIN tmp_candidate_sessions ON tmp_candidate_sessions.creator_score_idx = current_scores.alocidispid +INNER JOIN tmp_candidate_recipients ON tmp_candidate_recipients.receiver_score_idx = current_scores.blocidispid +WHERE + current_scores.full_score < #{max_score} AND + tmp_candidate_recipients.receiver_id != tmp_candidate_sessions.creator_id +GROUP BY + tmp_candidate_recipients.receiver_id, + tmp_candidate_sessions.session_id, + latency +SQL + ) + end + def fetch_recipients(per_page=BATCH_SIZE) objs = [] @@ -122,6 +179,8 @@ module JamRuby sessions = MusicSession.select("music_sessions.*, tmp_matches.latency") .joins("INNER JOIN tmp_matches ON tmp_matches.session_id = music_sessions.id") .where(["tmp_matches.receiver_id = ?", receiver.id]) + .order('tmp_matches.latency') + .limit(20) .includes([:genre, :creator]) block_given? ? yield(receiver, sessions) : objs << [receiver, sessions] end @@ -162,43 +221,49 @@ SELECT msess.user_id AS creator_id, users.last_jam_locidispid AS creator_score_idx, rs.instrument_id, - invitations.receiver_id AS invited_user_id + invitations.receiver_id AS invited_user_id, + msess.is_unstructured_rsvp, + msess.open_rsvps INTO TEMP TABLE tmp_candidate_sessions FROM music_sessions msess INNER JOIN users ON users.id = msess.user_id INNER JOIN rsvp_slots AS rs ON rs.music_session_id = msess.id LEFT JOIN rsvp_requests_rsvp_slots AS rrrs ON rrrs.rsvp_slot_id = rs.id -LEFT JOIN invitations ON invitations.music_session_id = msess.id +LEFT JOIN invitations ON open_rsvps = FALSE AND invitations.music_session_id = msess.id WHERE - open_rsvps = TRUE AND + (msess.is_unstructured_rsvp = TRUE OR (rrrs.id IS NULL OR rrrs.chosen != TRUE)) AND users.last_jam_locidispid IS NOT NULL AND msess.created_at > '#{earliest_session_create_time}' AND msess.created_at < '#{latest_session_create_time}' AND - scheduled_start >= '#{earliest_session_start_time}' AND - (rrrs.rsvp_slot_id IS NULL OR rrrs.chosen != TRUE) + scheduled_start >= '#{earliest_session_start_time}' #{limit_sql} SQL ActiveRecord::Base.connection.execute(sql) end def _collect_eligible_recipients - ActiveRecord::Base.connection.execute("DROP TABLE IF EXISTS tmp_candidate_recipients") + ActiveRecord::Base.connection.execute("DROP TABLE IF EXISTS tmp_candidate_recipients").check limit_sql = (self.snapshot? && 0 < ENV[ENV_QUERY_LIMIT].to_i) ? "LIMIT #{ENV[ENV_QUERY_LIMIT]}" : '' # load eligible recipients into tmp table sql =< JamRuby::User do + ignore do + specific_instruments nil + end + sequence(:email) { |n| "person_#{n}@example.com"} sequence(:first_name) { |n| "Person" } sequence(:last_name) { |n| "#{n}" } @@ -17,8 +21,14 @@ FactoryGirl.define do #u.association :musician_instrument, factory: :musician_instrument, user: u - before(:create) do |user| - user.musician_instruments << FactoryGirl.build(:musician_instrument, user: user) + before(:create) do |user, evaluator| + if evaluator.specific_instruments + evaluator.specific_instruments.each do |instrument| + user.musician_instruments << FactoryGirl.build(:musician_instrument, user: user, instrument: instrument) + end + else + user.musician_instruments << FactoryGirl.build(:musician_instrument, user: user) + end end factory :fan do diff --git a/ruby/spec/jam_ruby/models/email_batch_spec_scheduled_session.rb b/ruby/spec/jam_ruby/models/email_batch_spec_scheduled_session.rb index c4b951cd0..0d69c0619 100644 --- a/ruby/spec/jam_ruby/models/email_batch_spec_scheduled_session.rb +++ b/ruby/spec/jam_ruby/models/email_batch_spec_scheduled_session.rb @@ -10,36 +10,62 @@ describe EmailBatchScheduledSessions do UserMailer.deliveries.clear end + def find_user_sessions(obj, user) + found = obj.find{|item| item[0] == user} + found.should_not be_nil + found[1] + end + describe 'daily scheduled' do # before { pending } let (:scheduled_batch) { FactoryGirl.create(:email_batch_scheduled_session) } - let (:drums) { FactoryGirl.create(:instrument, :description => 'drums') } - let (:guitar) { FactoryGirl.create(:instrument, :description => 'guitar') } - let (:bass) { FactoryGirl.create(:instrument, :description => 'bass') } - let (:vocals) { FactoryGirl.create(:instrument, :description => 'vocal') } + let (:drums) { Instrument.find('drums') } + let (:guitar) { Instrument.find('acoustic guitar') } + let (:bass) { Instrument.find('bass guitar') } + let (:vocals) { Instrument.find('voice') } + let (:electric_guitar) { Instrument.find('electric guitar')} - let (:drummer) { FactoryGirl.create(:user, :last_jam_locidispid => 1, :last_jam_addr => 1) } - let (:guitarist) { FactoryGirl.create(:user, :last_jam_locidispid => 1, :last_jam_addr => 1) } - let (:bassist) { FactoryGirl.create(:user, :last_jam_locidispid => 1, :last_jam_addr => 1) } - let (:vocalist) { FactoryGirl.create(:user, :last_jam_locidispid => 1, :last_jam_addr => 1) } - let (:loser) { FactoryGirl.create(:user, :last_jam_locidispid => 2, :last_jam_addr => 2) } + let (:drummer) { FactoryGirl.create(:user, first_name: 'drummer', :last_jam_locidispid => 1, :last_jam_addr => 1, specific_instruments: [drums, guitar]) } + let (:guitarist) { FactoryGirl.create(:user, first_name: 'guitarist', :last_jam_locidispid => 1, :last_jam_addr => 1, specific_instruments: [guitar, drums]) } + let (:bassist) { FactoryGirl.create(:user, first_name: 'bassist', :last_jam_locidispid => 1, :last_jam_addr => 1, specific_instruments: [bass]) } + let (:vocalist) { FactoryGirl.create(:user, first_name: 'vocalist', :last_jam_locidispid => 1, :last_jam_addr => 1, specific_instruments: [vocals, electric_guitar]) } + let (:loser) { FactoryGirl.create(:user, first_name: 'loser', :last_jam_locidispid => 2, :last_jam_addr => 2, specific_instruments: [vocals, drums]) } + + let (:session3_creator) {FactoryGirl.create(:user, first_name: 'session3_creator', :last_jam_locidispid => 3, :last_jam_addr => 3, specific_instruments: [vocals]) } + let (:session4_creator) {FactoryGirl.create(:user, first_name: 'session4_creator', :last_jam_locidispid => 4, :last_jam_addr => 4, specific_instruments: [vocals]) } let (:session1) do FactoryGirl.create(:music_session, :creator => drummer, :scheduled_start => Time.now() + 2.days, - :musician_access => true, - :approval_required => false, + :open_rsvps=> true, + :is_unstructured_rsvp => false, :created_at => Time.now - 1.hour) end let (:session2) do FactoryGirl.create(:music_session, :creator => drummer, + :open_rsvps => false, + :is_unstructured_rsvp => false, + :scheduled_start => Time.now() + 2.days, + :created_at => Time.now - 1.hour) + end + let (:session3) do + FactoryGirl.create(:music_session, + :creator => session3_creator, + :open_rsvps => true, + :is_unstructured_rsvp => true, + :scheduled_start => Time.now() + 2.days, + :created_at => Time.now - 1.hour) + end + let (:session4) do + FactoryGirl.create(:music_session, + :creator => session4_creator, + :open_rsvps => true, + :is_unstructured_rsvp => true, :scheduled_start => Time.now() + 2.days, - :musician_access => true, - :approval_required => false, :created_at => Time.now - 1.hour) end @@ -49,23 +75,9 @@ describe EmailBatchScheduledSessions do JamRuby::Score.delete_all scheduled_batch.reset! - drummer.musician_instruments << FactoryGirl.build(:musician_instrument, user: drummer, instrument: drums, proficiency_level: 2) - drummer.musician_instruments << FactoryGirl.build(:musician_instrument, user: drummer, instrument: guitar, proficiency_level: 2) - - guitarist.musician_instruments << FactoryGirl.build(:musician_instrument, user: guitarist, instrument: guitar, proficiency_level: 2) - guitarist.musician_instruments << FactoryGirl.build(:musician_instrument, user: guitarist, instrument: bass, proficiency_level: 2) - - bassist.musician_instruments << FactoryGirl.build(:musician_instrument, user: bassist, instrument: bass, proficiency_level: 2) - bassist.musician_instruments << FactoryGirl.build(:musician_instrument, user: bassist, instrument: guitar, proficiency_level: 2) - - vocalist.musician_instruments << FactoryGirl.build(:musician_instrument, user: vocalist, instrument: vocals, proficiency_level: 2) - - loser.musician_instruments << FactoryGirl.build(:musician_instrument, user: loser, instrument: vocals, proficiency_level: 2) - loser.musician_instruments << FactoryGirl.build(:musician_instrument, user: loser, instrument: drums, proficiency_level: 2) - - FactoryGirl.create(:rsvp_slot, :instrument => drums, :music_session => session1) - FactoryGirl.create(:rsvp_slot, :instrument => guitar, :music_session => session1) - FactoryGirl.create(:rsvp_slot, :instrument => bass, :music_session => session1) + @drums_rsvp_slot = FactoryGirl.create(:rsvp_slot, :instrument => drums, :music_session => session1) + @guitar_rsvp_slot = FactoryGirl.create(:rsvp_slot, :instrument => guitar, :music_session => session1) + @bass_rsvp_slot = FactoryGirl.create(:rsvp_slot, :instrument => bass, :music_session => session1) FactoryGirl.create(:rsvp_slot, :instrument => drums, :music_session => session2) FactoryGirl.create(:rsvp_slot, :instrument => guitar, :music_session => session2) FactoryGirl.create(:rsvp_slot, :instrument => bass, :music_session => session2) @@ -73,34 +85,175 @@ describe EmailBatchScheduledSessions do # oo.rsvp_request_slot.update_attributes(chosen: true) # oo = FactoryGirl.create(:rsvp_request, :user => vocalist, :rsvp_slot => oo) # oo.rsvp_request_slot.update_attributes(chosen: true) - - end - - it 'sets up data properly' do - JamRuby::Score.createx(1, 'a', 1, 1, 'a', 1, 10) - JamRuby::Score.createx(1, 'a', 1, 2, 'a', 2, Score::MAX_YELLOW_LATENCY + 1) expect(drummer.instruments.include?(drums)).to eq(true) expect(drummer.instruments.include?(guitar)).to eq(true) - obj = scheduled_batch.fetch_recipients - obj.each_with_index do |u, i| - puts obj[i][0].email + end + + describe "everyone but loser has good enough scores" do + + before(:each) do + JamRuby::Score.createx(1, 'a', 1, 1, 'a', 1, 10) + JamRuby::Score.createx(1, 'a', 1, 2, 'a', 2, (APP_CONFIG.max_yellow_full_score * 2) + 1) + @matching_instruments_users = [guitarist, bassist] + @good_score_users = [guitarist, bassist, vocalist] # not adding the drummer because he's the creator (he does have a good score to these others though) + end + + describe "session1 is_unstructured" do + before(:each) do + session1.is_unstructured_rsvp = true + session1.save! + end + + it 'finds anyone with good enough scores' do + obj = scheduled_batch.fetch_recipients + found_users = obj.map{ |user_and_sessions| user_and_sessions[0]} + @good_score_users.should =~ found_users + + scheduled_batch.deliver_batch + expect(UserMailer.deliveries.length).to eq(@good_score_users.length) + end + end + + it 'finds anyone with good enough scores and matching instruments' do + obj = scheduled_batch.fetch_recipients + + #Database.dump("SELECT * FROM tmp_candidate_recipients") do |field, value| + # if field == 'receiver_id' + # User.find(value).first_name + # else + # value.to_s + # end + #end + + found_users = obj.map{ |user_and_sessions| user_and_sessions[0]} + @matching_instruments_users.should =~ found_users + + scheduled_batch.deliver_batch + expect(UserMailer.deliveries.length).to eq(@matching_instruments_users.length) + end + + describe "closes a RSVP slot in session1" do + before(:each) do + request = FactoryGirl.create(:rsvp_request, user: loser) + FactoryGirl.create(:rsvp_request_rsvp_slot, chosen: true, rsvp_request: request, rsvp_slot: @bass_rsvp_slot) + end + + it "finds one less" do + # the bass slot is closed; so we should not longer find the bassist in the results + @matching_instruments_users.delete(bassist).should_not be_nil + + obj = scheduled_batch.fetch_recipients + found_users = obj.map{ |user_and_sessions| user_and_sessions[0]} + @matching_instruments_users.should =~ found_users + + scheduled_batch.deliver_batch + expect(UserMailer.deliveries.length).to eq(@matching_instruments_users.length) + end + end + + describe "2 more open sessions" do + before(:each) do + JamRuby::Score.createx(1, 'a', 1, session3_creator.last_jam_locidispid, 'a', 1, 5) + JamRuby::Score.createx(1, 'a', 1, session4_creator.last_jam_locidispid, 'a', 2, 20) + session3.touch + session4.touch + end + + it "sessions are sorted by latency" do + obj = scheduled_batch.fetch_recipients + obj.length.should == @matching_instruments_users.length + [drummer, vocalist].length + + # verify that all 4 users have the right sessions, in the right order + @matching_instruments_users.each do |user| + user_sessions = find_user_sessions(obj, user) + user_sessions.should == [session3, session1, session4] + end + + user_sessions = find_user_sessions(obj, drummer) + user_sessions.should == [session3, session4] + + user_sessions = find_user_sessions(obj, vocalist) + user_sessions.should == [session3, session4] + end + end + + describe "21 more sessions" do + before(:each) do + 20.times do |i| + creator = FactoryGirl.create(:user, :last_jam_locidispid => 1, :last_jam_addr => 1) + FactoryGirl.create(:music_session, + :creator => creator, + :open_rsvps => true, + :is_unstructured_rsvp => true, + :scheduled_start => Time.now() + 2.days, + :created_at => Time.now - 1.hour) + end + end + + it "prevents more than 20 sessions per user" do + obj = scheduled_batch.fetch_recipients + obj.length.should == 20 + 4 # 4 is vocalist, drummer, guitarist, and bassist. 20 is the users made in the before block + + + # the bassist should *potentially* the drummer's session, and these new 20 sessions. But we limit to 20. + user_sessions = find_user_sessions(obj, bassist) + user_sessions.length.should == 20 + end + end + + it 'finds anyone with good enough scores and matching instruments' do + obj = scheduled_batch.fetch_recipients + + #Database.dump("SELECT * FROM tmp_candidate_recipients") do |field, value| + # if field == 'receiver_id' + # User.find(value).first_name + # else + # value.to_s + # end + #end + + found_users = obj.map{ |user_and_sessions| user_and_sessions[0]} + @matching_instruments_users.should =~ found_users + + scheduled_batch.deliver_batch + expect(UserMailer.deliveries.length).to eq(@matching_instruments_users.length) + end + + describe "open_rsvp=false" do + before(:each) do + session1.open_rsvps = false + session1.save! + session2.open_rsvps = false + session2.save! + end + + it "won't find an open_rsvps=false session without an invitation" do + obj = scheduled_batch.fetch_recipients + expect(obj.count).to eq(0) + end + + describe "with invitations" do + let(:invitation) {friend(drummer, guitarist); FactoryGirl.create(:invitation, sender: drummer, receiver:guitarist, music_session: session1)} + + before(:each) do + invitation.touch + end + + it "finds user with the invite" do + obj = scheduled_batch.fetch_recipients + found_users = obj.map{ |user_and_sessions| user_and_sessions[0]} + [guitarist].should =~ found_users + + scheduled_batch.deliver_batch + expect(UserMailer.deliveries.length).to eq(1) + end + end end - expect(obj.count).to eq(2) - scheduled_batch.deliver_batch - expect(UserMailer.deliveries.length).to eq(2) end - it "won't find an open_rsvps=false session" do - session1.open_rsvps = false - session1.save! - session2.open_rsvps = false - session2.save! - obj = scheduled_batch.fetch_recipients - expect(obj.count).to eq(0) - end it 'handles large batches' do creators = [] diff --git a/ruby/spec/support/utilities.rb b/ruby/spec/support/utilities.rb index fa7080d9b..b9db7f8ac 100644 --- a/ruby/spec/support/utilities.rb +++ b/ruby/spec/support/utilities.rb @@ -182,4 +182,9 @@ def fake_geo_124_zip(geoisp_csv) end zipfile +end + +def friend(user1, user2) + FactoryGirl.create(:friendship, user: user1, friend: user2) + FactoryGirl.create(:friendship, user: user2, friend: user1) end \ No newline at end of file diff --git a/web/spec/factories.rb b/web/spec/factories.rb index 117d2f85f..30f4fe32b 100644 --- a/web/spec/factories.rb +++ b/web/spec/factories.rb @@ -2,6 +2,10 @@ include ActionDispatch::TestProcess # added for artifact_update http://stackover FactoryGirl.define do factory :user, :class => JamRuby::User do + ignore do + specific_instruments nil + end + sequence(:email) { |n| "person_#{n}@example.com"} sequence(:first_name) { |n| "Person" } sequence(:last_name) { |n| "#{n}" } @@ -33,8 +37,15 @@ FactoryGirl.define do end end - before(:create) do |user| - user.musician_instruments << FactoryGirl.build(:musician_instrument, user: user) + before(:create) do |user, evaluator| + if evaluator.specific_instruments + evaluator.specific_instruments.each do |instrument| + puts "burp: " + user.musician_instruments << FactoryGirl.build(:musician_instrument, user: user, instrument: instrument) + end + else + user.musician_instruments << FactoryGirl.build(:musician_instrument, user: user) + end end factory :user_two_instruments do