Commit c6410e3e4be39e5dc54cda1d396faaa8f3d069d6
1 parent
fd9ec40e
Exists in
master
and in
27 other branches
profile-suggestions: connections
Showing
5 changed files
with
88 additions
and
67 deletions
Show diff stats
app/models/profile_suggestion.rb
@@ -4,6 +4,10 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -4,6 +4,10 @@ class ProfileSuggestion < ActiveRecord::Base | ||
4 | 4 | ||
5 | attr_accessible :person, :suggestion, :suggestion_type, :categories, :enabled | 5 | attr_accessible :person, :suggestion, :suggestion_type, :categories, :enabled |
6 | 6 | ||
7 | + has_many :suggestion_connections, :foreign_key => 'suggestion_id' | ||
8 | + has_many :profile_connections, :through => :suggestion_connections, :source => :connection, :source_type => 'Profile' | ||
9 | + has_many :tag_connections, :through => :suggestion_connections, :source => :connection, :source_type => 'ActsAsTaggableOn::Tag' | ||
10 | + | ||
7 | before_create do |profile_suggestion| | 11 | before_create do |profile_suggestion| |
8 | profile_suggestion.suggestion_type = self.suggestion.class.to_s | 12 | profile_suggestion.suggestion_type = self.suggestion.class.to_s |
9 | end | 13 | end |
@@ -45,19 +49,19 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -45,19 +49,19 @@ class ProfileSuggestion < ActiveRecord::Base | ||
45 | 49 | ||
46 | RULES = { | 50 | RULES = { |
47 | :people_with_common_communities => { | 51 | :people_with_common_communities => { |
48 | - :threshold => 2, :weight => 1 | 52 | + :threshold => 2, :weight => 1, :connection => 'Profile' |
49 | }, | 53 | }, |
50 | :people_with_common_friends => { | 54 | :people_with_common_friends => { |
51 | - :threshold => 2, :weight => 1 | 55 | + :threshold => 2, :weight => 1, :connection => 'Profile' |
52 | }, | 56 | }, |
53 | :people_with_common_tags => { | 57 | :people_with_common_tags => { |
54 | - :threshold => 2, :weight => 1 | 58 | + :threshold => 2, :weight => 1, :connection => 'ActsAsTaggableOn::Tag' |
55 | }, | 59 | }, |
56 | :communities_with_common_friends => { | 60 | :communities_with_common_friends => { |
57 | - :threshold => 2, :weight => 1 | 61 | + :threshold => 2, :weight => 1, :connection => 'Profile' |
58 | }, | 62 | }, |
59 | :communities_with_common_tags => { | 63 | :communities_with_common_tags => { |
60 | - :threshold => 2, :weight => 1 | 64 | + :threshold => 2, :weight => 1, :connection => 'ActsAsTaggableOn::Tag' |
61 | } | 65 | } |
62 | } | 66 | } |
63 | 67 | ||
@@ -118,9 +122,21 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -118,9 +122,21 @@ class ProfileSuggestion < ActiveRecord::Base | ||
118 | suggested_profiles.each do |suggested_profile| | 122 | suggested_profiles.each do |suggested_profile| |
119 | suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(suggested_profile.id) | 123 | suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(suggested_profile.id) |
120 | RULES.each do |rule, options| | 124 | RULES.each do |rule, options| |
121 | - value = suggested_profile.send("#{rule}_count").to_i | 125 | + begin |
126 | + value = suggested_profile.send("#{rule}_count").to_i | ||
127 | + rescue NoMethodError | ||
128 | + next | ||
129 | + end | ||
130 | + connections = suggested_profile.send("#{rule}_connections") | ||
131 | + if connections.present? | ||
132 | + connections = connections[1..-2].split(',') | ||
133 | + else | ||
134 | + connections = [] | ||
135 | + end | ||
122 | suggestion.send("#{rule}=", value) | 136 | suggestion.send("#{rule}=", value) |
123 | - #TODO Create suggestion connections. | 137 | + connections.each do |connection_id| |
138 | + SuggestionConnection.create!(:suggestion => suggestion, :connection_id => connection_id, :connection_type => options[:connection]) | ||
139 | + end | ||
124 | suggestion.score += value * options[:weight] | 140 | suggestion.score += value * options[:weight] |
125 | end | 141 | end |
126 | suggestion.save! | 142 | suggestion.save! |
@@ -202,15 +218,25 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -202,15 +218,25 @@ class ProfileSuggestion < ActiveRecord::Base | ||
202 | end | 218 | end |
203 | 219 | ||
204 | def self.all_suggestions(person) | 220 | def self.all_suggestions(person) |
205 | - select_string = "profiles.*, " + RULES.keys.map { |rule| "suggestions.#{counter(rule)} as #{counter(rule)}, suggestions.#{connections(rule)} as #{connections(rule)}" }.join(',') | 221 | + select_string = ["profiles.*"] |
222 | + suggestions_join = [] | ||
223 | + where_string = [] | ||
224 | + valid_rules = [] | ||
206 | previous_rule = nil | 225 | previous_rule = nil |
207 | - suggestions_join = RULES.keys.map do |rule| | 226 | + join_column = nil |
227 | + RULES.each do |rule, options| | ||
228 | + rule_select = self.send(rule, person) | ||
229 | + next if !rule_select.present? | ||
230 | + | ||
231 | + valid_rules << rule | ||
232 | + select_string << "suggestions.#{counter(rule)} as #{counter(rule)}, suggestions.#{connections(rule)} as #{connections(rule)}" | ||
233 | + where_string << "#{counter(rule)} >= #{options[:threshold]}" | ||
208 | rule_select = " | 234 | rule_select = " |
209 | (SELECT profiles.id as #{profile_id(rule)}, | 235 | (SELECT profiles.id as #{profile_id(rule)}, |
210 | #{rule}_sub.#{counter(rule)} as #{counter(rule)}, | 236 | #{rule}_sub.#{counter(rule)} as #{counter(rule)}, |
211 | #{rule}_sub.#{connections(rule)} as #{connections(rule)} | 237 | #{rule}_sub.#{connections(rule)} as #{connections(rule)} |
212 | FROM profiles | 238 | FROM profiles |
213 | - LEFT OUTER JOIN (#{self.send(rule, person)}) as #{rule}_sub | 239 | + LEFT OUTER JOIN (#{rule_select}) as #{rule}_sub |
214 | ON profiles.id = #{rule}_sub.#{profile_id(rule)}) AS #{rule}" | 240 | ON profiles.id = #{rule}_sub.#{profile_id(rule)}) AS #{rule}" |
215 | 241 | ||
216 | if previous_rule.nil? | 242 | if previous_rule.nil? |
@@ -220,10 +246,14 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -220,10 +246,14 @@ class ProfileSuggestion < ActiveRecord::Base | ||
220 | ON #{previous_rule}.#{profile_id(previous_rule)} = #{rule}.#{profile_id(rule)}" | 246 | ON #{previous_rule}.#{profile_id(previous_rule)} = #{rule}.#{profile_id(rule)}" |
221 | end | 247 | end |
222 | previous_rule = rule | 248 | previous_rule = rule |
223 | - result | ||
224 | - end.join(' ') | ||
225 | - join_string = "INNER JOIN (SELECT * FROM #{suggestions_join}) AS suggestions ON profiles.id = suggestions.#{profile_id(RULES.keys.first)}" | ||
226 | - where_string = RULES.map { |rule, options| "#{counter(rule)} >= #{options[:threshold]}"}.join(' OR ') | 249 | + suggestions_join << result |
250 | + end | ||
251 | + | ||
252 | + return if valid_rules.blank? | ||
253 | + | ||
254 | + select_string = select_string.compact.join(',') | ||
255 | + join_string = "INNER JOIN (SELECT * FROM #{suggestions_join.compact.join(' ')}) AS suggestions ON profiles.id = suggestions.#{profile_id(valid_rules.first)}" | ||
256 | + where_string = where_string.compact.join(' OR ') | ||
227 | 257 | ||
228 | person.environment.profiles. | 258 | person.environment.profiles. |
229 | select(select_string). | 259 | select(select_string). |
@@ -0,0 +1,6 @@ | @@ -0,0 +1,6 @@ | ||
1 | +class SuggestionConnection < ActiveRecord::Base | ||
2 | + attr_accessible :suggestion, :connection_type, :connection_id | ||
3 | + | ||
4 | + belongs_to :suggestion, :class_name => 'ProfileSuggestion', :foreign_key => 'suggestion_id' | ||
5 | + belongs_to :connection, :polymorphic => true | ||
6 | +end |
db/migrate/20140811141211_create_suggestion_connections.rb
0 → 100644
@@ -0,0 +1,12 @@ | @@ -0,0 +1,12 @@ | ||
1 | +class CreateSuggestionConnections < ActiveRecord::Migration | ||
2 | + def up | ||
3 | + create_table :suggestion_connections do |t| | ||
4 | + t.references :suggestion, :null => false | ||
5 | + t.references :connection, :polymorphic => true, :null => false | ||
6 | + end | ||
7 | + end | ||
8 | + | ||
9 | + def down | ||
10 | + drop_table :suggestion_connections | ||
11 | + end | ||
12 | +end |
db/schema.rb
@@ -11,7 +11,7 @@ | @@ -11,7 +11,7 @@ | ||
11 | # | 11 | # |
12 | # It's strongly recommended to check this file into your version control system. | 12 | # It's strongly recommended to check this file into your version control system. |
13 | 13 | ||
14 | -ActiveRecord::Schema.define(:version => 20140805205626) do | 14 | +ActiveRecord::Schema.define(:version => 20140811141211) do |
15 | 15 | ||
16 | create_table "abuse_reports", :force => true do |t| | 16 | create_table "abuse_reports", :force => true do |t| |
17 | t.integer "reporter_id" | 17 | t.integer "reporter_id" |
@@ -603,6 +603,12 @@ ActiveRecord::Schema.define(:version => 20140805205626) do | @@ -603,6 +603,12 @@ ActiveRecord::Schema.define(:version => 20140805205626) do | ||
603 | add_index "sessions", ["session_id"], :name => "index_sessions_on_session_id" | 603 | add_index "sessions", ["session_id"], :name => "index_sessions_on_session_id" |
604 | add_index "sessions", ["updated_at"], :name => "index_sessions_on_updated_at" | 604 | add_index "sessions", ["updated_at"], :name => "index_sessions_on_updated_at" |
605 | 605 | ||
606 | + create_table "suggestion_connections", :force => true do |t| | ||
607 | + t.integer "suggestion_id", :null => false | ||
608 | + t.integer "connection_id", :null => false | ||
609 | + t.string "connection_type", :null => false | ||
610 | + end | ||
611 | + | ||
606 | create_table "taggings", :force => true do |t| | 612 | create_table "taggings", :force => true do |t| |
607 | t.integer "tag_id" | 613 | t.integer "tag_id" |
608 | t.integer "taggable_id" | 614 | t.integer "taggable_id" |
test/unit/profile_suggestion_test.rb
@@ -57,7 +57,10 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | @@ -57,7 +57,10 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | ||
57 | p4.add_friend(p6) ; p6.add_friend(p4) | 57 | p4.add_friend(p6) ; p6.add_friend(p4) |
58 | p2.add_friend(p7) ; p7.add_friend(p2) | 58 | p2.add_friend(p7) ; p7.add_friend(p2) |
59 | 59 | ||
60 | - assert_equivalent ProfileSuggestion.people_with_common_friends(p1), [p5,p6] | 60 | + suggestions = ProfileSuggestion.calculate_suggestions(p1) |
61 | + | ||
62 | + assert_includes suggestions, p5 | ||
63 | + assert_includes suggestions, p6 | ||
61 | end | 64 | end |
62 | 65 | ||
63 | should 'calculate people with common_communities' do | 66 | should 'calculate people with common_communities' do |
@@ -81,7 +84,10 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | @@ -81,7 +84,10 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | ||
81 | c3.add_member(p4) | 84 | c3.add_member(p4) |
82 | c4.add_member(p5) | 85 | c4.add_member(p5) |
83 | 86 | ||
84 | - assert_equivalent ProfileSuggestion.people_with_common_communities(p1), [p2,p4] | 87 | + suggestions = ProfileSuggestion.calculate_suggestions(p1) |
88 | + | ||
89 | + assert_includes suggestions, p2 | ||
90 | + assert_includes suggestions, p4 | ||
85 | end | 91 | end |
86 | 92 | ||
87 | should 'calculate people with common_tags' do | 93 | should 'calculate people with common_tags' do |
@@ -121,7 +127,10 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | @@ -121,7 +127,10 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | ||
121 | a52.tag_list = ['onivorism', 'facism'] | 127 | a52.tag_list = ['onivorism', 'facism'] |
122 | a52.save! | 128 | a52.save! |
123 | 129 | ||
124 | - assert_equivalent ProfileSuggestion.people_with_common_tags(p1), [p2, p3] | 130 | + suggestions = ProfileSuggestion.calculate_suggestions(p1) |
131 | + | ||
132 | + assert_includes suggestions, p2 | ||
133 | + assert_includes suggestions, p3 | ||
125 | end | 134 | end |
126 | 135 | ||
127 | should 'calculate communities with common_friends' do | 136 | should 'calculate communities with common_friends' do |
@@ -146,7 +155,10 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | @@ -146,7 +155,10 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | ||
146 | c3.add_member(p2) | 155 | c3.add_member(p2) |
147 | c4.add_member(p3) | 156 | c4.add_member(p3) |
148 | 157 | ||
149 | - assert_equivalent ProfileSuggestion.communities_with_common_friends(p1), [c1,c2] | 158 | + suggestions = ProfileSuggestion.calculate_suggestions(p1) |
159 | + | ||
160 | + assert_includes suggestions, c1 | ||
161 | + assert_includes suggestions, c2 | ||
150 | end | 162 | end |
151 | 163 | ||
152 | should 'calculate communities with common_tags' do | 164 | should 'calculate communities with common_tags' do |
@@ -186,39 +198,10 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | @@ -186,39 +198,10 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | ||
186 | a52.tag_list = ['onivorism', 'facism'] | 198 | a52.tag_list = ['onivorism', 'facism'] |
187 | a52.save! | 199 | a52.save! |
188 | 200 | ||
189 | - assert_equivalent ProfileSuggestion.communities_with_common_tags(p1), [p2, p3] | ||
190 | - end | 201 | + suggestions = ProfileSuggestion.calculate_suggestions(p1) |
191 | 202 | ||
192 | - should 'register suggestions' do | ||
193 | - person = create_user('person').person | ||
194 | - rule = ProfileSuggestion::RULES.sample | ||
195 | - p1 = fast_create(Profile) | ||
196 | - p2 = fast_create(Profile) | ||
197 | - p3 = fast_create(Profile) | ||
198 | - # Hack to simulate a common_count that generated on the rules | ||
199 | - suggestions = Profile.select('profiles.*, profiles.id as common_count').where("id in (#{[p1,p2,p3].map(&:id).join(',')})") | ||
200 | - | ||
201 | - assert_difference 'ProfileSuggestion.count', 3 do | ||
202 | - ProfileSuggestion.register_suggestions(person, suggestions, rule) | ||
203 | - end | ||
204 | - assert_no_difference 'ProfileSuggestion.count' do | ||
205 | - s1 = ProfileSuggestion.find_or_initialize_by_suggestion_id(p1.id) | ||
206 | - assert_equal p1, s1.suggestion | ||
207 | - s2 = ProfileSuggestion.find_or_initialize_by_suggestion_id(p2.id) | ||
208 | - assert_equal p2, s2.suggestion | ||
209 | - s3 = ProfileSuggestion.find_or_initialize_by_suggestion_id(p3.id) | ||
210 | - assert_equal p3, s3.suggestion | ||
211 | - end | ||
212 | - end | ||
213 | - | ||
214 | - should 'calculate every rule suggestion' do | ||
215 | - person = create_user('person').person | ||
216 | - ProfileSuggestion::RULES.each do |rule| | ||
217 | - suggestion = fast_create(Profile) | ||
218 | - ProfileSuggestion.expects(rule).returns([suggestion]) | ||
219 | - ProfileSuggestion.expects(:register_suggestions).with(person, [suggestion], rule).returns(true) | ||
220 | - end | ||
221 | - ProfileSuggestion.calculate_suggestions(person) | 203 | + assert_includes suggestions, p2 |
204 | + assert_includes suggestions, p3 | ||
222 | end | 205 | end |
223 | 206 | ||
224 | #FIXME This might not be necessary anymore... | 207 | #FIXME This might not be necessary anymore... |
@@ -303,22 +286,6 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | @@ -303,22 +286,6 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | ||
303 | # end | 286 | # end |
304 | # end | 287 | # end |
305 | 288 | ||
306 | - should 'register only new suggestions' do | ||
307 | - person = create_user('person').person | ||
308 | - ProfileSuggestion::SUGGESTIONS_BY_RULE.times do | ||
309 | - ProfileSuggestion.create!(:person => person, :suggestion => fast_create(Person)) | ||
310 | - end | ||
311 | - | ||
312 | - person.reload | ||
313 | - new_suggestion = fast_create(Person) | ||
314 | - ids = (person.suggested_people + [new_suggestion]).map(&:id).join(',') | ||
315 | - suggested_profiles = Profile.select('profiles.*, profiles.id as common_count').where("profiles.id IN (#{ids})") | ||
316 | - | ||
317 | - assert_difference 'ProfileSuggestion.count', 1 do | ||
318 | - ProfileSuggestion.register_suggestions(person, suggested_profiles, 'people_with_common_friends') | ||
319 | - end | ||
320 | - end | ||
321 | - | ||
322 | should 'calculate new suggestions when number of available suggestions reaches the min_limit' do | 289 | should 'calculate new suggestions when number of available suggestions reaches the min_limit' do |
323 | person = create_user('person').person | 290 | person = create_user('person').person |
324 | (ProfileSuggestion::MIN_LIMIT + 1).times do | 291 | (ProfileSuggestion::MIN_LIMIT + 1).times do |