diff --git a/ruby/Gemfile b/ruby/Gemfile index 907ca1b79..3912bae07 100644 --- a/ruby/Gemfile +++ b/ruby/Gemfile @@ -37,7 +37,7 @@ end group :test do gem "factory_girl", '4.1.0' - gem "rspec", "2.10.0" + gem "rspec", "2.11" gem 'spork', '0.9.0' gem 'database_cleaner', '0.7.0' end diff --git a/ruby/lib/jam_ruby/models/search.rb b/ruby/lib/jam_ruby/models/search.rb index f51ccd673..cdfcb8d53 100644 --- a/ruby/lib/jam_ruby/models/search.rb +++ b/ruby/lib/jam_ruby/models/search.rb @@ -83,9 +83,10 @@ module JamRuby attr_accessor :user_counters, :page_num, :page_count - PARAM_MUSICIAN = :search_m + PARAM_MUSICIAN = :srch_m M_PER_PAGE = 10 + M_MILES_DEFAULT = 500 M_ORDER_FOLLOWS = ['Most Followed', :followed] M_ORDER_PLAYS = ['Most Plays', :plays] @@ -99,21 +100,20 @@ module JamRuby end def self.musician_search(params={}, current_user=nil) - rel = User.where(:musician => true) + rel = User.musicians unless (instrument = params[:instrument]).blank? rel = rel.joins("RIGHT JOIN musicians_instruments AS minst ON minst.user_id = users.id") .where(['minst.instrument_id = ? AND users.id IS NOT NULL', instrument]) end location_distance, location_city = params[:distance], params[:city] + distance, latlng = nil, [] if location_distance && location_city if geo = MaxMindGeo.where(:city => params[:city]).limit(1).first - citylatlng = [geo.lat, geo.lng] - rel = rel.within(location_distance, :origin => citylatlng) + distance, latlng = location_distance, [geo.lat, geo.lng] end elsif current_user - latlng = [] - if current_user.lat.nil? + if current_user.lat.nil? || current_user.lng.nil? if params[:remote_ip] if geo = MaxMindGeo.ip_lookup(params[:remote_ip]) latlng = [geo.lat, geo.lng] if geo.lat && geo.lng @@ -122,16 +122,15 @@ module JamRuby else latlng = [current_user.lat, current_user.lng] end - distance = location_distance || 50 - unless latlng.blank? - rel = rel.where(['lat IS NOT NULL AND lng IS NOT NULL']) - .within(distance, :origin => latlng) - end + distance = location_distance || M_MILES_DEFAULT + end + unless latlng.blank? + rel = rel.where(['lat IS NOT NULL AND lng IS NOT NULL']) + .within(distance, :origin => latlng) end - sel_str = 'users.*' case ordering = self.musician_order_param(params) - when :plays + when :plays # FIXME: double counting? sel_str = "COUNT(records)+COUNT(sessions) AS play_count, #{sel_str}" rel = rel.joins("LEFT JOIN music_sessions AS sessions ON sessions.user_id = users.id") rel = rel.joins("LEFT JOIN recordings AS records ON records.owner_id = users.id") @@ -150,7 +149,7 @@ module JamRuby end rel = rel.select(sel_str) - perpage = params[:per_page] || M_PER_PAGE + perpage = [params[:per_page] || M_PER_PAGE, 100].min page = [params[:page].to_i, 1].max rel = rel.paginate(:page => page, :per_page => perpage) rel.includes([:instruments, :followings, :friends]) @@ -205,6 +204,8 @@ module JamRuby self end + private + def _count(musician, key) if mm = @user_counters[musician.id] return mm.detect { |ii| ii.is_a?(Hash) }[key] @@ -212,6 +213,8 @@ module JamRuby 0 end + public + def follow_count(musician) _count(musician, COUNT_FOLLOW) end @@ -247,5 +250,17 @@ module JamRuby false end + def self.new_musicians(since_date=Time.now - 1.week, max_count=100, radius=M_MILES_DEFAULT) + return unless block_given? + User.where(['lat IS NOT NULL AND lng IS NOT NULL']).find_each do |usr| + rel = User.musicians + .where(['created_at >= ? AND users.id != ?', since_date, usr.id]) + .within(radius, :origin => [usr.lat, usr.lng]) + .order('created_at DESC') + .limit(max_count) + yield(rel) if 0 < rel.count + end + end + end end diff --git a/ruby/lib/jam_ruby/models/user.rb b/ruby/lib/jam_ruby/models/user.rb index d6364b741..a07554d81 100644 --- a/ruby/lib/jam_ruby/models/user.rb +++ b/ruby/lib/jam_ruby/models/user.rb @@ -138,6 +138,9 @@ module JamRuby validate :email_case_insensitive_uniqueness validate :update_email_case_insensitive_uniqueness, :if => :updating_email + scope :musicians, where(:musician => true) + scope :musicians_geocoded, where(['musician = ? AND lat IS NOT NULL AND lng IS NOT NULL',true]) + def user_progression_fields @user_progression_fields ||= Set.new ["first_downloaded_client_at", "first_ran_client_at", "first_music_session_at", "first_real_music_session_at", "first_good_music_session_at", "first_certified_gear_at", "first_invited_at", "first_friended_at", "first_social_promoted_at" ] end diff --git a/ruby/spec/jam_ruby/models/musician_search_spec.rb b/ruby/spec/jam_ruby/models/musician_search_spec.rb index 5ce018da5..e86bd5db4 100644 --- a/ruby/spec/jam_ruby/models/musician_search_spec.rb +++ b/ruby/spec/jam_ruby/models/musician_search_spec.rb @@ -1,171 +1,218 @@ require 'spec_helper' -describe User do +describe 'Musician search' do before(:each) do @geocode1 = FactoryGirl.create(:geocoder) @geocode2 = FactoryGirl.create(:geocoder) - params = { - first_name: "Example", - last_name: "User", - email: "user1@example.com", - password: "foobar", - password_confirmation: "foobar", - musician: true, - email_confirmed: true, - city: "Apex", - state: "NC", - country: "US" - } @users = [] - @users << @user1 = FactoryGirl.create(:user, params) - params[:email] = "user2@example.com" - @users << @user2 = FactoryGirl.create(:user, params) - params[:email] = "user3@example.com" - @users << @user3 = FactoryGirl.create(:user, params) - params[:email] = "user4@example.com" - @users << @user4 = FactoryGirl.create(:user, params) + @users << @user1 = FactoryGirl.create(:user) + @users << @user2 = FactoryGirl.create(:user) + @users << @user3 = FactoryGirl.create(:user) + @users << @user4 = FactoryGirl.create(:user) end - it "should find all musicians sorted by followers with pagination" do - # establish sorting order - @user4.followers.concat([@user2, @user3, @user4]) - @user3.followers.concat([@user3, @user4]) - @user2.followers.concat([@user1]) - @user4.followers.count.should == 3 + context 'default filter settings' do - UserFollower.count.should == 6 - - # get all the users in correct order - params = { :per_page => @users.size } - results = Search.musician_search(params) - results.musicians.count.should == @users.size - - results.musicians.each_with_index do |uu, idx| - uu.id.should == @users.reverse[idx].id + it "finds all musicians" do + # expects all the users + num = User.musicians.count + results = Search.musician_search({ :per_page => num }) + expect(results.musicians.count).to eq(num) end - # refresh the order to ensure it works right - @user2.followers.concat([@user3, @user4, @user2]) - results = Search.musician_search(params, @user3) - results.musicians[0].id.should == @user2.id + it "finds musicians with proper ordering" do + # the ordering should be create_at since no followers exist + expect(UserFollower.count).to eq(0) + results = Search.musician_search({ :per_page => User.musicians.count }) + results.musicians.each_with_index do |uu, idx| + expect(uu.id).to eq(@users.reverse[idx].id) + end + end - # check the follower count for given entry - results.musicians[0].search_follow_count.to_i.should_not == 0 - # check the follow relationship between current_user and result - results.is_follower?(@user2).should == true + it "sorts musicians by followers" do + # establish sorting order + @user4.followers.concat([@user2, @user3, @user4]) + @user3.followers.concat([@user3, @user4]) + @user2.followers.concat([@user1]) + expect(@user4.followers.count).to be 3 + expect(UserFollower.count).to be 6 + + # refresh the order to ensure it works right + @user2.followers.concat([@user3, @user4, @user2]) + results = Search.musician_search({ :per_page => @users.size }, @user3) + expect(results.musicians[0].id).to eq(@user2.id) + + # check the follower count for given entry + expect(results.musicians[0].search_follow_count.to_i).not_to eq(0) + # check the follow relationship between current_user and result + expect(results.is_follower?(@user2)).to be true + end + + it 'paginates properly' do + # make sure pagination works right + params = { :per_page => 2, :page => 1 } + results = Search.musician_search(params) + expect(results.musicians.count).to be 2 + end - # make sure pagination works right - params = { :per_page => 2, :page => 1 } - results = Search.musician_search(params) - results.musicians.count.should == 2 end - it "should have friends counter " do - Friendship.save(@user1.id, @user2.id) - results = Search.musician_search({}, @user2) - friend = results.musicians.detect { |mm| mm.id == @user1.id } - friend.should_not == nil - results.friend_count(friend).should == 1 - @user1.reload - friend.friends?(@user2).should == true - results.is_friend?(@user1).should == true - end - - def make_recording(uu) - connection = FactoryGirl.create(:connection, :user => uu) + def make_recording(usr) + connection = FactoryGirl.create(:connection, :user => usr) instrument = FactoryGirl.create(:instrument, :description => 'a great instrument') track = FactoryGirl.create(:track, :connection => connection, :instrument => instrument) - music_session = FactoryGirl.create(:music_session, :creator => uu, :musician_access => true) + music_session = FactoryGirl.create(:music_session, :creator => usr, :musician_access => true) music_session.connections << connection music_session.save - recording = Recording.start(music_session, uu) + recording = Recording.start(music_session, usr) recording.stop recording.reload genre = FactoryGirl.create(:genre) - recording.claim(uu, "name", "description", genre, true, true) + recording.claim(usr, "name", "description", genre, true, true) recording.reload recording end - it "should have recording counter " do - recording = make_recording(@user1) - recording.users.length.should == 1 - recording.users.first.should == @user1 - @user1.reload - @user1.recordings.length.should == 1 - @user1.recordings.first.should == recording - recording.claimed_recordings.length.should == 1 - @user1.recordings.detect { |rr| rr == recording }.should_not be nil - - results = Search.musician_search({},@user1) - uu = results.musicians.detect { |mm| mm.id == @user1.id } - uu.should_not == nil - - results.record_count(uu).should == 1 - results.session_count(uu).should == 1 - end - - it "should find musicians sorted by plays" do - make_recording(@user1) - results = Search.musician_search({ :orderby => 'plays' }, @user2) - results.musicians[0].id.should == @user1.id - make_recording(@user2) - make_recording(@user2) - results = Search.musician_search({ :orderby => 'plays' }, @user2) - results.musicians[0].id.should == @user2.id - end - - it "should find all musicians sorted by now playing" do - connection = FactoryGirl.create(:connection, :user => @user3) - music_session = FactoryGirl.create(:music_session, :creator => @user3, :musician_access => true) + def make_session(usr) + connection = FactoryGirl.create(:connection, :user => usr) + music_session = FactoryGirl.create(:music_session, :creator => usr, :musician_access => true) music_session.connections << connection music_session.save - results = Search.musician_search({ :orderby => 'playing' }, @user2) - results.musicians.count.should == 1 - connection = FactoryGirl.create(:connection, :user => @user4) - music_session = FactoryGirl.create(:music_session, :creator => @user4, :musician_access => true) - music_session.connections << connection - music_session.save - results = Search.musician_search({ :orderby => 'playing' }, @user2) - results.musicians.count.should == 2 - connection = FactoryGirl.create(:connection, :user => @user1) - music_session = FactoryGirl.create(:music_session, :creator => @user1, :musician_access => true) - music_session.connections << connection - music_session.save - results = Search.musician_search({ :orderby => 'playing' }, @user2) - results.musicians.count.should == 3 end - it "should find musicians with an instrument" do - minst = FactoryGirl.create(:musician_instrument, { - :user => @user1, - :instrument => Instrument.find('tuba') }) - @user1.musician_instruments << minst - @user1.reload - ii = @user1.instruments.detect { |inst| inst.id == 'tuba' } - ii.should_not be_nil - params = { :instrument => ii.id } - results = Search.musician_search(params) - results.musicians.each do |rr| - rr.instruments.detect { |inst| inst.id=='tuba' }.id.should == ii.id + context 'musician stat counters' do + + it "displays musicians top followings" 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)) end - results.musicians.count.should == 1 + + it "friends stat shows friend count" do + # create friendship record + Friendship.save(@user1.id, @user2.id) + # search on user2 + results = Search.musician_search({}, @user2) + friend = results.musicians.detect { |mm| mm.id == @user1.id } + expect(friend).to_not be_nil + expect(results.friend_count(friend)).to be 1 + @user1.reload + expect(friend.friends?(@user2)).to be true + expect(results.is_friend?(@user1)).to be true + end + + it "recording stat shows recording count" do + recording = make_recording(@user1) + expect(recording.users.length).to be 1 + expect(recording.users.first).to eq(@user1) + @user1.reload + expect(@user1.recordings.length).to be 1 + expect(@user1.recordings.first).to eq(recording) + expect(recording.claimed_recordings.length).to be 1 + expect(@user1.recordings.detect { |rr| rr == recording }).to_not be_nil + + results = Search.musician_search({},@user1) + uu = results.musicians.detect { |mm| mm.id == @user1.id } + expect(uu).to_not be_nil + + expect(results.record_count(uu)).to be 1 + expect(results.session_count(uu)).to be 1 + end + end - it "should find musicians within a given distance of location" do - @user1.lat.should_not == nil - params = { :distance => 10, :city => 'San Francisco' } - results = Search.musician_search(params) - results.musicians.count.should == 0 + context 'musician sorting' do - params = { :distance => 10, :city => 'Apex' } - results = Search.musician_search(params) - results.musicians.count.should == User.count + it "by plays" do + make_recording(@user1) + # order results by num recordings + results = Search.musician_search({ :orderby => 'plays' }, @user2) + expect(results.musicians[0].id).to eq(@user1.id) - params = { :distance => 10 } - results = Search.musician_search(params) - results.musicians.count.should == User.count + # add more data and make sure order still correct + make_recording(@user2); make_recording(@user2) + results = Search.musician_search({ :orderby => 'plays' }, @user2) + expect(results.musicians[0].id).to eq(@user2.id) + end + + it "by now playing" do + # should get 1 result with 1 active session + make_session(@user3) + results = Search.musician_search({ :orderby => 'playing' }, @user2) + expect(results.musicians.count).to be 1 + expect(results.musicians.first.id).to eq(@user3.id) + + # should get 2 results with 2 active sessions + # sort order should be created_at DESC + make_session(@user4) + results = Search.musician_search({ :orderby => 'playing' }, @user2) + expect(results.musicians.count).to be 2 + expect(results.musicians[0].id).to eq(@user4.id) + expect(results.musicians[1].id).to eq(@user3.id) + end + + end + + context 'filter settings' do + it "searches musicisns for an instrument" do + minst = FactoryGirl.create(:musician_instrument, { + :user => @user1, + :instrument => Instrument.find('tuba') }) + @user1.musician_instruments << minst + @user1.reload + ii = @user1.instruments.detect { |inst| inst.id == 'tuba' } + expect(ii).to_not be_nil + results = Search.musician_search({ :instrument => ii.id }) + results.musicians.each do |rr| + expect(rr.instruments.detect { |inst| inst.id=='tuba' }.id).to eq(ii.id) + end + expect(results.musicians.count).to be 1 + end + + it "finds musicians within a given distance of given location" do + num = User.musicians.count + expect(@user1.lat).to_not be_nil + # short distance + results = Search.musician_search({ :per_page => num, + :distance => 10, + :city => 'Apex' }, @user1) + expect(results.musicians.count).to be num + # long distance + results = Search.musician_search({ :per_page => num, + :distance => 1000, + :city => 'Miami', + :state => 'FL' }, @user1) + expect(results.musicians.count).to be num + end + + it "finds musicians within a given distance of users location" do + expect(@user1.lat).to_not be_nil + # uses the location of @user1 + results = Search.musician_search({ :distance => 10, :per_page => User.musicians.count }, @user1) + expect(results.musicians.count).to be User.musicians.count + end + + it "finds no musicians within a given distance of location" do + expect(@user1.lat).to_not be_nil + results = Search.musician_search({ :distance => 10, :city => 'San Francisco' }, @user1) + expect(results.musicians.count).to be 0 + end + + end + + context 'new users' do + it "find nearby" do + # create new user to ensure its excluded from results + usr = FactoryGirl.create(:user, {city: "Austin", state: "TX", country: "US"}) + Search.new_musicians do |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(usrs.count).to eq(User.musicians.count - 1) + end + end end end diff --git a/web/app/assets/javascripts/findMusician.js b/web/app/assets/javascripts/findMusician.js index a16ebed51..3ff605b66 100644 --- a/web/app/assets/javascripts/findMusician.js +++ b/web/app/assets/javascripts/findMusician.js @@ -26,7 +26,7 @@ function search() { did_show_musician_page = true; - var queryString = 'search_m=1&page='+page_num+'&'; + var queryString = 'srch_m=1&page='+page_num+'&'; // order by var orderby = $('.musician-order-by').val(); diff --git a/web/app/views/clients/_musician_filter.html.erb b/web/app/views/clients/_musician_filter.html.erb index 633d9bfc2..4a351876e 100644 --- a/web/app/views/clients/_musician_filter.html.erb +++ b/web/app/views/clients/_musician_filter.html.erb @@ -1,28 +1,35 @@ -