From b1b2a2e360ff13f6caf3d4e7ba1c656a8131584a Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sun, 17 Nov 2013 17:29:23 -0600 Subject: [PATCH 01/14] vrfs-775: added new_musicians method and test --- ruby/lib/jam_ruby/models/search.rb | 14 ++++++++++++++ ruby/spec/jam_ruby/models/musician_search_spec.rb | 6 ++++++ 2 files changed, 20 insertions(+) diff --git a/ruby/lib/jam_ruby/models/search.rb b/ruby/lib/jam_ruby/models/search.rb index f51ccd673..59b7d4875 100644 --- a/ruby/lib/jam_ruby/models/search.rb +++ b/ruby/lib/jam_ruby/models/search.rb @@ -247,5 +247,19 @@ module JamRuby false end + def self.new_musicians(since_date=Time.now - 1.week, max_count=100, radius=500) + return unless block_given? + User.where(['lat IS NOT NULL AND lng IS NOT NULL']).find_each do |usr| + rel = User.where(:musician => true) + .where(['created_at >= ? AND users.id != ?', since_date, usr.id]) + .within(radius, :origin => [usr.lat, usr.lng]) + .order('created_at DESC') + .limit(max_count) + if 0 < rel.count + yield rel + end + end + end + end 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..159799d2e 100644 --- a/ruby/spec/jam_ruby/models/musician_search_spec.rb +++ b/ruby/spec/jam_ruby/models/musician_search_spec.rb @@ -168,4 +168,10 @@ describe User do results.musicians.count.should == User.count end + it "should find new musicians nearby" do + Search.new_musicians do |usrs| + usrs.count.should.not == 0 + end + end + end From c0c68798336e8f91631e719893158be1f305eb3e Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sat, 23 Nov 2013 00:03:22 -0600 Subject: [PATCH 02/14] updated rspec version to 2.11 --- ruby/Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 5b1ea983fa709c21a30ba755c61e1000e7a82736 Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sat, 23 Nov 2013 00:04:07 -0600 Subject: [PATCH 03/14] vrfs-774: added upper bound to the per_page param --- ruby/lib/jam_ruby/models/search.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ruby/lib/jam_ruby/models/search.rb b/ruby/lib/jam_ruby/models/search.rb index 59b7d4875..ffd4403f3 100644 --- a/ruby/lib/jam_ruby/models/search.rb +++ b/ruby/lib/jam_ruby/models/search.rb @@ -150,7 +150,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]) @@ -256,7 +256,7 @@ module JamRuby .order('created_at DESC') .limit(max_count) if 0 < rel.count - yield rel + yield(rel) end end end From 68f6225461270bbcad9ccd4c3c840e24b2757cdd Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sat, 23 Nov 2013 00:04:33 -0600 Subject: [PATCH 04/14] vrfs-774: refactoring tests, using expect syntax over should --- .../jam_ruby/models/musician_search_spec.rb | 268 ++++++++++-------- 1 file changed, 147 insertions(+), 121 deletions(-) diff --git a/ruby/spec/jam_ruby/models/musician_search_spec.rb b/ruby/spec/jam_ruby/models/musician_search_spec.rb index 159799d2e..3d2d07c6c 100644 --- a/ruby/spec/jam_ruby/models/musician_search_spec.rb +++ b/ruby/spec/jam_ruby/models/musician_search_spec.rb @@ -1,75 +1,65 @@ require 'spec_helper' -describe User do +describe 'Ad hoc 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 + it "finds all musicians" do + pending + # expects all the users + num = User.where({:musician => true}).count + results = Search.musician_search({ :per_page => num }) + expect(results.musicians.count).to eq(num) - # 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 + # the ordering should be create_at since no followers exist + expect(UserFollower.count).to eq(0) + results.musicians.each_with_index do |uu, idx| + expect(uu.id).to eq(@users.reverse[idx].id) + end 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 "sorts musicians by followers" do + pending + # 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 - # 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 + expect(UserFollower.count).to be 6 - # make sure pagination works right - params = { :per_page => 2, :page => 1 } - results = Search.musician_search(params) - results.musicians.count.should == 2 - end + # 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 + + expect(@user4.top_followings.count).to be 3 + expect(@user4.top_followings.map(&:id)).to match_array((@users - [@user1]).map(&:id)) + end + + it 'paginates properly' do + pending + # make sure pagination works right + params = { :per_page => 2, :page => 1 } + results = Search.musician_search(params) + expect(results.musicians.count).to be 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) @@ -88,89 +78,125 @@ describe User do 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 + context 'musician stat counters' do - 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) - 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 + it "friends stat shows friend count" do + pending + # 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 + pending + 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 + + context 'musician sorting' do + + it "by plays" do + pending + make_recording(@user1) + # order results by num recordings + results = Search.musician_search({ :orderby => 'plays' }, @user2) + expect(results.musicians[0].id).to eq(@user1.id) + + # 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 + connection = FactoryGirl.create(:connection, :user => @user3) + music_session = FactoryGirl.create(:music_session, :creator => @user3, :musician_access => true) + music_session.connections << connection + music_session.save + results = Search.musician_search({ :orderby => 'playing' }, @user2) + results.musicians.count.should be 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 be 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 be 3 + end + + it "should find musicians with an instrument" do + pending + 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 + end + results.musicians.count.should be 1 end - results.musicians.count.should == 1 end it "should find musicians within a given distance of location" do + pending @user1.lat.should_not == nil params = { :distance => 10, :city => 'San Francisco' } results = Search.musician_search(params) - results.musicians.count.should == 0 + results.musicians.count.should be 0 params = { :distance => 10, :city => 'Apex' } results = Search.musician_search(params) - results.musicians.count.should == User.count + results.musicians.count.should be User.count params = { :distance => 10 } results = Search.musician_search(params) - results.musicians.count.should == User.count + results.musicians.count.should be User.count end it "should find new musicians nearby" do + pending + params = { + first_name: "Example", last_name: "User", + email: "#{User.count+1}@example.com", email_confirmed: true, + password: "foobar", password_confirmation: "foobar", + musician: true, + city: "Austin", state: "TX", country: "US" + } + FactoryGirl.create(:user, params) Search.new_musicians do |usrs| - usrs.count.should.not == 0 + usrs.count.should eq(User.count - 1) end end From 432e5ad59be9f961df7e2e13da69c4539ada9a2d Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sat, 23 Nov 2013 01:44:13 -0600 Subject: [PATCH 05/14] vrfs-774: added musicians scope --- ruby/lib/jam_ruby/models/user.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ruby/lib/jam_ruby/models/user.rb b/ruby/lib/jam_ruby/models/user.rb index d6364b741..09043aec5 100644 --- a/ruby/lib/jam_ruby/models/user.rb +++ b/ruby/lib/jam_ruby/models/user.rb @@ -138,6 +138,8 @@ module JamRuby validate :email_case_insensitive_uniqueness validate :update_email_case_insensitive_uniqueness, :if => :updating_email + scope :musicians, where(:musician => 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 From 543ef29579b984ef86b77c6ebb804f3a621845c3 Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sat, 23 Nov 2013 02:04:28 -0600 Subject: [PATCH 06/14] vrfs-775: include musicians scope in new_musicians method --- ruby/lib/jam_ruby/models/search.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ruby/lib/jam_ruby/models/search.rb b/ruby/lib/jam_ruby/models/search.rb index ffd4403f3..8d87a899d 100644 --- a/ruby/lib/jam_ruby/models/search.rb +++ b/ruby/lib/jam_ruby/models/search.rb @@ -250,14 +250,12 @@ module JamRuby def self.new_musicians(since_date=Time.now - 1.week, max_count=100, radius=500) return unless block_given? User.where(['lat IS NOT NULL AND lng IS NOT NULL']).find_each do |usr| - rel = User.where(:musician => true) + 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) - if 0 < rel.count - yield(rel) - end + yield(rel) if 0 < rel.count end end From d9823eac3a36a6520fb16b933c31eb04eea5b23a Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sat, 23 Nov 2013 02:18:12 -0600 Subject: [PATCH 07/14] vrfs-774, vrfs-775: refactored tests into smaller it blocks; added contexts --- .../jam_ruby/models/musician_search_spec.rb | 151 ++++++++++-------- 1 file changed, 83 insertions(+), 68 deletions(-) diff --git a/ruby/spec/jam_ruby/models/musician_search_spec.rb b/ruby/spec/jam_ruby/models/musician_search_spec.rb index 3d2d07c6c..e86bd5db4 100644 --- a/ruby/spec/jam_ruby/models/musician_search_spec.rb +++ b/ruby/spec/jam_ruby/models/musician_search_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'Ad hoc musician search' do +describe 'Musician search' do before(:each) do @geocode1 = FactoryGirl.create(:geocoder) @@ -15,27 +15,27 @@ describe 'Ad hoc musician search' do context 'default filter settings' do it "finds all musicians" do - pending # expects all the users - num = User.where({:musician => true}).count + num = User.musicians.count results = Search.musician_search({ :per_page => num }) expect(results.musicians.count).to eq(num) + end + 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 it "sorts musicians by followers" do - pending # 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 @@ -47,13 +47,9 @@ describe 'Ad hoc musician search' do 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 - - expect(@user4.top_followings.count).to be 3 - expect(@user4.top_followings.map(&:id)).to match_array((@users - [@user1]).map(&:id)) end it 'paginates properly' do - pending # make sure pagination works right params = { :per_page => 2, :page => 1 } results = Search.musician_search(params) @@ -62,26 +58,40 @@ describe 'Ad hoc musician search' do 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 + 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 + end + 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 + it "friends stat shows friend count" do - pending # create friendship record Friendship.save(@user1.id, @user2.id) # search on user2 @@ -95,7 +105,6 @@ describe 'Ad hoc musician search' do end it "recording stat shows recording count" do - pending recording = make_recording(@user1) expect(recording.users.length).to be 1 expect(recording.users.first).to eq(@user1) @@ -118,7 +127,6 @@ describe 'Ad hoc musician search' do context 'musician sorting' do it "by plays" do - pending make_recording(@user1) # order results by num recordings results = Search.musician_search({ :orderby => 'plays' }, @user2) @@ -131,72 +139,79 @@ describe 'Ad hoc musician search' do end it "by now playing" do - connection = FactoryGirl.create(:connection, :user => @user3) - music_session = FactoryGirl.create(:music_session, :creator => @user3, :musician_access => true) - music_session.connections << connection - music_session.save + # should get 1 result with 1 active session + make_session(@user3) results = Search.musician_search({ :orderby => 'playing' }, @user2) - results.musicians.count.should be 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 + 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) - results.musicians.count.should be 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 be 3 + 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 - it "should find musicians with an instrument" do - pending + 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' } - ii.should_not be_nil - params = { :instrument => ii.id } - results = Search.musician_search(params) + expect(ii).to_not be_nil + results = Search.musician_search({ :instrument => ii.id }) results.musicians.each do |rr| - rr.instruments.detect { |inst| inst.id=='tuba' }.id.should == ii.id + expect(rr.instruments.detect { |inst| inst.id=='tuba' }.id).to eq(ii.id) end - results.musicians.count.should be 1 + 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 - it "should find musicians within a given distance of location" do - pending - @user1.lat.should_not == nil - params = { :distance => 10, :city => 'San Francisco' } - results = Search.musician_search(params) - results.musicians.count.should be 0 - - params = { :distance => 10, :city => 'Apex' } - results = Search.musician_search(params) - results.musicians.count.should be User.count - - params = { :distance => 10 } - results = Search.musician_search(params) - results.musicians.count.should be User.count - end - - it "should find new musicians nearby" do - pending - params = { - first_name: "Example", last_name: "User", - email: "#{User.count+1}@example.com", email_confirmed: true, - password: "foobar", password_confirmation: "foobar", - musician: true, - city: "Austin", state: "TX", country: "US" - } - FactoryGirl.create(:user, params) - Search.new_musicians do |usrs| - usrs.count.should eq(User.count - 1) + 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 From 58bed4bc0457ad6b346c3d9c5d8fb3ca507daa77 Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sat, 23 Nov 2013 06:15:49 -0600 Subject: [PATCH 08/14] vrfs774: added musicians_geocoded scope --- ruby/lib/jam_ruby/models/user.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/ruby/lib/jam_ruby/models/user.rb b/ruby/lib/jam_ruby/models/user.rb index 09043aec5..a07554d81 100644 --- a/ruby/lib/jam_ruby/models/user.rb +++ b/ruby/lib/jam_ruby/models/user.rb @@ -139,6 +139,7 @@ module JamRuby 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" ] From 1941c65b66ef8a7671970790e51d7d1f603cd922 Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sat, 23 Nov 2013 06:16:40 -0600 Subject: [PATCH 09/14] vrfs774: fixed bug in latlng/distance when city param used; added M_MILES_DEFAULT; changed value of PARAM_MUSICIAN --- ruby/lib/jam_ruby/models/search.rb | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/ruby/lib/jam_ruby/models/search.rb b/ruby/lib/jam_ruby/models/search.rb index 8d87a899d..e520c4753 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] @@ -106,14 +107,13 @@ module JamRuby 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,13 +122,12 @@ 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 @@ -247,7 +246,7 @@ module JamRuby false end - def self.new_musicians(since_date=Time.now - 1.week, max_count=100, radius=500) + 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 From 0eb9ddc4d68b1ba9fa1ca8918ad8a29460a360d3 Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sat, 23 Nov 2013 06:17:23 -0600 Subject: [PATCH 10/14] vrfs-774: changed value of Search::PARAM_MUSICISN to srch_m --- web/app/assets/javascripts/findMusician.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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(); From 9efefb485e01c9b2a491da0f8c02e62fa430b17e Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sat, 23 Nov 2013 06:17:58 -0600 Subject: [PATCH 11/14] vrfs-774: refactored api tests; added geocoding support --- web/spec/factories.rb | 11 +- web/spec/requests/musician_search_api_spec.rb | 141 +++++++++--------- web/spec/spec_helper.rb | 2 + web/spec/support/request_helpers.rb | 13 ++ 4 files changed, 94 insertions(+), 73 deletions(-) create mode 100644 web/spec/support/request_helpers.rb diff --git a/web/spec/factories.rb b/web/spec/factories.rb index 382d540fe..876b4eec3 100644 --- a/web/spec/factories.rb +++ b/web/spec/factories.rb @@ -128,5 +128,14 @@ FactoryGirl.define do end - + factory :geocoder, :class => JamRuby::MaxMindGeo do + country 'US' + sequence(:region) { |n| ['NC', 'CA'][(n-1).modulo(2)] } + sequence(:city) { |n| ['Apex', 'San Francisco'][(n-1).modulo(2)] } + sequence(:ip_start) { |n| ['1.1.0.0', '1.1.255.255'][(n-1).modulo(2)] } + sequence(:ip_end) { |n| ['1.2.0.0', '1.2.255.255'][(n-1).modulo(2)] } + sequence(:lat) { |n| [35.73265, 37.7742075][(n-1).modulo(2)] } + sequence(:lng) { |n| [-78.85029, -122.4155311][(n-1).modulo(2)] } + end + end diff --git a/web/spec/requests/musician_search_api_spec.rb b/web/spec/requests/musician_search_api_spec.rb index ca6e48a75..11680a2a5 100644 --- a/web/spec/requests/musician_search_api_spec.rb +++ b/web/spec/requests/musician_search_api_spec.rb @@ -6,89 +6,86 @@ describe "Musician Search API", :type => :api do def get_query(query={}) qstr = '' - query.each { |kk,vv| qstr += "#{kk}=#{vv}&" } - get "/api/search.json?#{Search::PARAM_MUSICIAN}=1&#{qstr}" + query.each { |kk,vv| qstr += "#{kk}=#{CGI::escape(vv.to_s)}&" } + uri = "/api/search.json?#{Search::PARAM_MUSICIAN}=1&#{qstr}" + get uri, "CONTENT_TYPE" => 'application/json' end - - describe "profile page" do + describe "musician search page" do let(:user) { FactoryGirl.create(:user) } before(:each) do - post '/sessions', "session[email]" => user.email, "session[password]" => user.password - rack_mock_session.cookie_jar["remember_token"].should == user.remember_token - end - - it "blank search" do - get_query - last_response.status.should == 200 - sobj = JSON.parse(last_response.body) - sobj.count.should >= 1 - sobj['musicians'].count.should == [Search::M_PER_PAGE, - User.where(:musician => true).count].min - end - - it "instrument search" do - minst = FactoryGirl.create(:musician_instrument, { - :user => user, - :instrument => Instrument.find('tuba') }) - user.musician_instruments << minst - - get_query({:instrument => 'tuba'}) - last_response.status.should == 200 - response = JSON.parse(last_response.body) - - response["musicians"].length.should == 1 - musician = response["musicians"][0] - musician["id"].should == user.id - instruments = musician['instruments'] - instruments.detect { |ii| ii['instrument_id'] == 'tuba' }.should_not == nil - instruments.detect { |ii| ii['instrument_id'] == 'electric guitar' }.should_not == nil - end - - it "following search" do - 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" - } + 2.downto(1) { FactoryGirl.create(:geocoder) } @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) - @user4.followers.concat([@user2, @user3, @user4]) - @user3.followers.concat([@user3, @user4]) - @user2.followers.concat([@user4]) - @user4.followers.count.should == 3 + @users << @user1 = FactoryGirl.create(:user) + @users << @user2 = FactoryGirl.create(:user) + @users << @user3 = FactoryGirl.create(:user) + @users << @user4 = FactoryGirl.create(:user) - Friendship.save(@user1.id, @user2.id) + post '/sessions', "session[email]" => user.email, "session[password]" => user.password + expect(rack_mock_session.cookie_jar["remember_token"]).to eq(user.remember_token) + end + it "default search" do get_query - last_response.status.should == 200 - response = JSON.parse(last_response.body) - - musician = response["musicians"][0] - musician["id"].should == @user4.id - followings= musician['followings'] - followings.length.should == 3 - musician['follow_count'].to_i.should > 0 + good_response + expect(json['musicians'].count).to be [Search::M_PER_PAGE, User.musicians_geocoded.count].min + end - friend = response['musicians'].detect { |mm| mm['id'] == @user1.id } - friend['friend_count'].should == 1 + context 'location filtering' do - musician['recording_count'].should == 0 - musician['session_count'].should == 0 + it "gets no musicians for out of range locations" do + get_query({:city => 'San Francisco', :distance => 100}) + good_response + expect((json['musicians'] || []).count).to be 0 + end + + it "gets musicians for long-distance locations" do + get_query({:city => 'Miami', :distance => 1000}) + good_response + expect(json['musicians'].count).to be >= 1 + end + + end + + context 'instrument filtering' do + + it "gets musicians for default instrument" do + get_query({:instrument => 'electric guitar'}) + good_response + expect(json.count).to be >= 1 + end + + it "gets no musicians for unused instruments" do + get_query({:instrument => 'tuba'}) + good_response + expect(json.count).to be 0 + end + + end + + context 'results have expected data' do + it "has follower stats " do + @user4.followers.concat([@user2, @user3, @user4]) + @user3.followers.concat([@user3, @user4]) + @user2.followers.concat([@user4]) + expect(@user4.followers.count).to be 3 + get_query + good_response + musician = json["musicians"][0] + expect(musician["id"]).to eq(@user4.id) + followings = musician['followings'] + expect(followings.length).to be 3 + expect(musician['follow_count'].to_i).to be > 0 + end + + it "has friend stats" do + Friendship.save(@user1.id, @user2.id) + get_query + good_response + friend = json['musicians'].detect { |mm| mm['id'] == @user1.id } + expect(friend['friend_count']).to be 1 + end end end end diff --git a/web/spec/spec_helper.rb b/web/spec/spec_helper.rb index d9011c7b8..9a46ea0cc 100644 --- a/web/spec/spec_helper.rb +++ b/web/spec/spec_helper.rb @@ -93,6 +93,8 @@ Spork.prefork do # rspec-rails. config.infer_base_class_for_anonymous_controllers = false + config.include Requests::JsonHelpers, type: :request + config.before(:suite) do end diff --git a/web/spec/support/request_helpers.rb b/web/spec/support/request_helpers.rb new file mode 100644 index 000000000..85dfba23c --- /dev/null +++ b/web/spec/support/request_helpers.rb @@ -0,0 +1,13 @@ +module Requests + module JsonHelpers + + def json + @json ||= JSON.parse(last_response.body) + end + + def good_response + expect(last_response.status).to be 200 + end + + end +end From b50c46dd323c6de09c59ac9be3cb72f94188bace Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sat, 23 Nov 2013 16:25:27 -0600 Subject: [PATCH 12/14] vrfs-774: converted raw html into rails helpers --- .../views/clients/_musician_filter.html.erb | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) 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 @@ -
-
-
Filter By:
+<%= content_tag(:div, :style => "min-width:770px;") do -%> + <%= content_tag(:div, :class => 'filter-element') do -%> + <%= content_tag(:div, 'Filter By:', :class => 'filter-element', :style => "padding-top:3px;") %> <%= select_tag(:musician_order_by, options_for_select(Search::M_ORDERINGS), {:class => 'musician-order-by'} ) %> -
-
+ <% end -%> + <%= content_tag(:div, :class => 'filter-element') do -%> -
Instrument:
-
+ <%= content_tag(:div, 'Instrument:', :class => 'filter-element') %> + <%= content_tag(:div, :class => 'filter-element', :id => "find-musician-instrument") do -%> <%= select_tag(:instrument, options_for_select([['Any', '']].concat(JamRuby::Instrument.all.collect { |ii| [ii.description, ii.id] })), {:class => 'instrument-list'} ) %> -
-
+ <% end -%> + <% end -%> -
-
Within
-
- -
-
miles of <%= current_user.current_city(request.remote_ip) %>
-
-
- REFRESH -
- -
+ <%= content_tag(:div, :class => 'filter-element') do -%> + <%= content_tag(:div, 'Within', :class => 'filter-element') %> + <%= content_tag(:div, :class => 'query-distance-params', :style => "height:25px;") do -%> + <%= text_field_tag(:query_distance, + Search::M_MILES_DEFAULT.to_s, + :id => :musician_query_distance, + :placeholder => Search::M_MILES_DEFAULT.to_s) %> + <% end -%> + <%= content_tag(:div, :class => 'filter-element') do -%> + miles of <%= content_tag(:span, current_user.current_city(request.remote_ip), :id => 'musician-filter-city') %> + <% end -%> + <% end -%> + <%= content_tag(:div, + link_to('REFRESH', '#', + :id => 'btn-refresh-musicians', + :style => 'text-decoration:none', + :class => 'button-grey'), + :class => 'right mr10') %> +<% end -%> From 114daf2818ecd16ad4bf6b963d18145e8797831f Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sat, 23 Nov 2013 16:54:01 -0600 Subject: [PATCH 13/14] vrfs-774: replaced raw HTML with rails helpers --- web/app/views/clients/_musicians.html.erb | 35 +++++++++-------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/web/app/views/clients/_musicians.html.erb b/web/app/views/clients/_musicians.html.erb index d660a363e..2fad4a25b 100644 --- a/web/app/views/clients/_musicians.html.erb +++ b/web/app/views/clients/_musicians.html.erb @@ -1,26 +1,19 @@ -
-
-
-
- <%= image_tag "content/icon_musicians.png", {:height => 19, :width => 19} %> -
- -

musicians

+<%= content_tag(:div, :layout => 'screen', 'layout-id' => 'musicians', :class => "screen secondary") do -%> + <%= content_tag(:div, :class => :content) do -%> + <%= content_tag(:div, :class => 'content-head') do -%> + <%= content_tag(:div, image_tag("content/icon_musicians.png", {:height => 19, :width => 19}), :class => 'content-icon') %> + <%= content_tag(:h1, 'musicians') %> <%= render "screen_navigation" %> -
-
-
- <%= render :partial => "musician_filter" %> -
-
-
-
-
-
-
-
-
+ <% end -%> + <%= form_tag('', :id => 'find-musician-form') do -%> + <%= content_tag(:div, render(:partial => "musician_filter"), :class => 'musician-filter', :id => 'session-controls') %> + <%= content_tag(:div, :class => 'content-scroller') do -%> + <%= content_tag(:div, content_tag(:div, '', :id => 'musician-filter-results'), :class => 'content-wrapper musician-wrapper') %> + <% end -%> + <% end -%> + <% end -%> +<% end -%>