diff --git a/ruby/lib/jam_ruby/models/search.rb b/ruby/lib/jam_ruby/models/search.rb index 0605abd35..7517df44b 100644 --- a/ruby/lib/jam_ruby/models/search.rb +++ b/ruby/lib/jam_ruby/models/search.rb @@ -292,12 +292,24 @@ module JamRuby end def self.new_musicians(usr, since_date) - # todo scott turn this into score .within(radius, :origin => [usr.lat, usr.lng]) + # this attempts to find interesting musicians to tell another musician about where interesting + # is "has a good score and was created recently" + # we're sort of depending upon usr being a musicians_geocoded as well... + # this appears to only be called from EmailBatchNewMusician#deliver_batch_sets! which is + # an offline process and thus uses the last jam location as "home base" - rel = User.musicians + locidispid = usr.last_jam_locidispid + score_limit = 60 + limit = 50 + + rel = User.musicians_geocoded .where(['created_at >= ? AND users.id != ?', since_date, usr.id]) - .order('created_at DESC') - .limit(50) + .joins('inner join scores on users.last_jam_locidispid = scores.alocidispid') + .where(['scores.blocidispid = ?', locidispid]) + .where(['scores.score <= ?', score_limit]) + .order('scores.score') # best scores first + .order('users.created_at DESC') # then most recent + .limit(limit) objs = rel.all.to_a diff --git a/ruby/lib/jam_ruby/models/user.rb b/ruby/lib/jam_ruby/models/user.rb index 408719774..f22144887 100644 --- a/ruby/lib/jam_ruby/models/user.rb +++ b/ruby/lib/jam_ruby/models/user.rb @@ -140,8 +140,8 @@ module JamRuby scope :musicians, where(:musician => true) scope :fans, where(:musician => false) - # todo scott someone with locidispid - scope :geocoded_users, where(['lat IS NOT NULL AND lng IS NOT NULL']) - # todo scott geocoded_users that are musicians - scope :musicians_geocoded, musicians.geocoded_users + scope :geocoded_users, where(['last_jam_locidispid IS NOT NULL']) + scope :musicians_geocoded, musicians.geocoded_users scope :email_opt_in, where(:subscribe_email => true) def user_progression_fields @@ -1177,12 +1177,12 @@ module JamRuby def self.deliver_new_musician_notifications(since_date=nil) since_date ||= Time.now-1.week - # todo scott return musicians with locidispid not null - # self.geocoded_users.find_each do |usr| - # Search.new_musicians(usr, since_date) do |new_nearby| - # UserMailer.new_musicians(usr, new_nearby).deliver - # end - # end + # return musicians with locidispid not null + self.musicians_geocoded.find_each do |usr| + Search.new_musicians(usr, since_date) do |new_nearby| + UserMailer.new_musicians(usr, new_nearby).deliver + end + end end def facebook_invite! diff --git a/ruby/spec/jam_ruby/models/musician_search_spec.rb b/ruby/spec/jam_ruby/models/musician_search_spec.rb index a80c1322c..289a8431b 100644 --- a/ruby/spec/jam_ruby/models/musician_search_spec.rb +++ b/ruby/spec/jam_ruby/models/musician_search_spec.rb @@ -5,29 +5,94 @@ describe 'Musician search' do before(:each) do # @geocode1 = FactoryGirl.create(:geocoder) # @geocode2 = FactoryGirl.create(:geocoder) - @users = [] - @users << @user1 = FactoryGirl.create(:user) - @users << @user2 = FactoryGirl.create(:user) - @users << @user3 = FactoryGirl.create(:user) - @users << @user4 = FactoryGirl.create(:user) + t = Time.now - 10.minute + + @user1 = FactoryGirl.create(:user, created_at: t+1.minute, last_jam_locidispid: 1) + @user2 = FactoryGirl.create(:user, created_at: t+2.minute, last_jam_locidispid: 2) + @user3 = FactoryGirl.create(:user, created_at: t+3.minute, last_jam_locidispid: 3) + @user4 = FactoryGirl.create(:user, created_at: t+4.minute, last_jam_locidispid: 4) + @user5 = FactoryGirl.create(:user, created_at: t+5.minute, last_jam_locidispid: 5) + @user6 = FactoryGirl.create(:user, created_at: t+6.minute) # not geocoded + @user7 = FactoryGirl.create(:user, created_at: t+7.minute, musician: false) # not musician + + @musicians = [] + @musicians << @user1 + @musicians << @user2 + @musicians << @user3 + @musicians << @user4 + @musicians << @user5 + @musicians << @user6 + + @geomusicians = [] + @geomusicians << @user1 + @geomusicians << @user2 + @geomusicians << @user3 + @geomusicians << @user4 + @geomusicians << @user5 + + Score.delete_all + Score.createx(1, 'a', 1, 1, 'a', 1, 10) + Score.createx(1, 'a', 1, 2, 'b', 2, 20) + Score.createx(1, 'a', 1, 3, 'c', 3, 30) + Score.createx(1, 'a', 1, 4, 'd', 4, 40) + Score.createx(2, 'b', 2, 3, 'c', 3, 15) + Score.createx(2, 'b', 2, 4, 'd', 4, 70) end - context 'default filter settings' do + context 'default filter settings' do it "finds all musicians" do - # expects all the users - num = User.musicians.count - results = Search.musician_filter({ :per_page => num }) - expect(results.results.count).to eq(num) - expect(results.search_type).to be(:musicians_filter) + # expects all the musicians + results = Search.musician_filter + results.search_type.should == :musicians_filter + results.results.count.should == @musicians.length + results.results.should eq @musicians.reverse + end + + it "finds all musicians page 1" do + # expects all the musicians + results = Search.musician_filter({page: 1}) + results.search_type.should == :musicians_filter + results.results.count.should == @musicians.length + results.results.should eq @musicians.reverse + end + + it "finds all musicians page 2" do + # expects no musicians (all fit on page 1) + results = Search.musician_filter({page: 2}) + results.search_type.should == :musicians_filter + results.results.count.should == 0 + end + + it "finds all musicians page 1 per_page 3" do + # expects three of the musicians + results = Search.musician_filter({per_page: 3}) + results.search_type.should == :musicians_filter + results.results.count.should == 3 + results.results.should eq @musicians.reverse.slice(0, 3) + end + + it "finds all musicians page 2 per_page 3" do + # expects two of the musicians + results = Search.musician_filter({page: 2, per_page: 3}) + results.search_type.should == :musicians_filter + results.results.count.should == 3 + results.results.should eq @musicians.reverse.slice(3, 3) + end + + it "finds all musicians page 3 per_page 3" do + # expects two of the musicians + results = Search.musician_filter({page: 3, per_page: 3}) + results.search_type.should == :musicians_filter + results.results.count.should == 0 end it "finds musicians with proper ordering" do - # the ordering should be create_at since no followers exist + # the ordering should be create_at desc since no followers exist expect(Follow.count).to eq(0) results = Search.musician_filter({ :per_page => User.musicians.count }) results.results.each_with_index do |uu, idx| - expect(uu.id).to eq(@users.reverse[idx].id) + expect(uu.id).to eq(@musicians.reverse[idx].id) end end @@ -90,7 +155,7 @@ describe 'Musician search' do f3.save # @user2.followers.concat([@user3, @user4, @user2]) - results = Search.musician_filter({ :per_page => @users.size }, @user3) + results = Search.musician_filter({ :per_page => @musicians.size }, @user3) expect(results.results[0].id).to eq(@user2.id) # check the follower count for given entry @@ -154,8 +219,8 @@ describe 'Musician search' do # @user4.followers.concat([@user4]) # @user3.followers.concat([@user4]) # @user2.followers.concat([@user4]) - expect(@user4.top_followings.count).to be 3 - expect(@user4.top_followings.map(&:id)).to match_array((@users - [@user1]).map(&:id)) + expect(@user4.top_followings.count).to eq 3 + expect(@user4.top_followings.map(&:id)).to match_array((@musicians - [@user1, @user5, @user6]).map(&:id)) end it "friends stat shows friend count" do @@ -275,30 +340,43 @@ describe 'Musician search' do context 'new users' do - it "find nearby" do - pending 'todo scott fix this test so it does something' - # create new user outside 500 from Apex to ensure its excluded from results - FactoryGirl.create(:user, {city: "Austin", state: "TX", country: "US"}) - User.geocoded_users.find_each do |usr| - Search.new_musicians(usr, Time.now - 1.week) do |new_usrs| - # the newly created user is not nearby the existing users (which are in Apex, NC) - # and that user is not included in query - expect(new_usrs.count).to eq(User.musicians.count - 1) - end - end + it "find three for user1" do + # user2..4 are scored against user1 + ms = Search.new_musicians(@user1, Time.now - 1.week) + ms.should_not be_nil + ms.length.should == 3 + ms.should eq [@user2, @user3, @user4] end - it "sends new musician email" do - pending 'todo scott fix this test so it does something' - # create new user outside 500 from Apex to ensure its excluded from results - FactoryGirl.create(:user, {city: "Austin", state: "TX", country: "US"}) - User.geocoded_users.find_each do |usr| - Search.new_musicians(usr, Time.now - 1.week) do |new_usrs| - # the newly created user is not nearby the existing users (which are in Apex, NC) - # and that user is not included in query - expect(new_usrs.count).to eq(User.musicians.count - 1) - end - end + it "find two for user2" do + # user1,3,4 are scored against user1, but user4 is bad + ms = Search.new_musicians(@user2, Time.now - 1.week) + ms.should_not be_nil + ms.length.should == 2 + ms.should eq [@user3, @user1] + end + + it "find two for user3" do + # user1..2 are scored against user3 + ms = Search.new_musicians(@user3, Time.now - 1.week) + ms.should_not be_nil + ms.length.should == 2 + ms.should eq [@user2, @user1] + end + + it "find one for user4" do + # user1..2 are scored against user4, but user2 is bad + ms = Search.new_musicians(@user4, Time.now - 1.week) + ms.should_not be_nil + ms.length.should == 1 + ms.should eq [@user1] + end + + it "find none for user5" do + # user1..4 are not scored against user5 + ms = Search.new_musicians(@user5, Time.now - 1.week) + ms.should_not be_nil + ms.length.should == 0 end end