Commit db7025249b45bdfc86b05f91e24b408ccfb46d79
1 parent
03abebca
Exists in
master
and in
22 other branches
profile-suggestions: create only a small and fixed number of suggestions
Showing
2 changed files
with
140 additions
and
73 deletions
Show diff stats
app/models/profile_suggestion.rb
@@ -8,6 +8,10 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -8,6 +8,10 @@ class ProfileSuggestion < ActiveRecord::Base | ||
8 | profile_suggestion.suggestion_type = self.suggestion.class.to_s | 8 | profile_suggestion.suggestion_type = self.suggestion.class.to_s |
9 | end | 9 | end |
10 | 10 | ||
11 | + after_destroy do |profile_suggestion| | ||
12 | + self.class.generate_profile_suggestions(profile_suggestion.person) | ||
13 | + end | ||
14 | + | ||
11 | acts_as_having_settings :field => :categories | 15 | acts_as_having_settings :field => :categories |
12 | 16 | ||
13 | validate :must_be_a_valid_category, :on => :create | 17 | validate :must_be_a_valid_category, :on => :create |
@@ -50,11 +54,11 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -50,11 +54,11 @@ class ProfileSuggestion < ActiveRecord::Base | ||
50 | communities_with_common_tags | 54 | communities_with_common_tags |
51 | ] | 55 | ] |
52 | 56 | ||
53 | - # Number of suggestions | ||
54 | - N_SUGGESTIONS = 30 | 57 | + # Number of suggestions by rule |
58 | + SUGGESTIONS_BY_RULE = 10 | ||
55 | 59 | ||
56 | - # Number max of attempts | ||
57 | - MAX_ATTEMPTS = N_SUGGESTIONS * 2 | 60 | + # Minimum number of suggestions |
61 | + MIN_LIMIT = 15 | ||
58 | 62 | ||
59 | # Number of friends in common | 63 | # Number of friends in common |
60 | COMMON_FRIENDS = 2 | 64 | COMMON_FRIENDS = 2 |
@@ -66,8 +70,12 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -66,8 +70,12 @@ class ProfileSuggestion < ActiveRecord::Base | ||
66 | COMMON_TAGS = 2 | 70 | COMMON_TAGS = 2 |
67 | 71 | ||
68 | def self.register_suggestions(person, suggested_profiles, rule) | 72 | def self.register_suggestions(person, suggested_profiles, rule) |
73 | + already_suggested_profiles = person.profile_suggestions.map(&:suggestion_id).join(',') | ||
74 | + suggested_profiles = suggested_profiles.where("profiles.id NOT IN (#{already_suggested_profiles})") if already_suggested_profiles.present? | ||
75 | + suggested_profiles = suggested_profiles.limit(SUGGESTIONS_BY_RULE) | ||
76 | + return if suggested_profiles.blank? | ||
69 | counter = rule.split(/.*_with_/).last | 77 | counter = rule.split(/.*_with_/).last |
70 | - suggested_profiles.find_each do |suggested_profile| | 78 | + suggested_profiles.each do |suggested_profile| |
71 | suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(suggested_profile.id) | 79 | suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(suggested_profile.id) |
72 | suggestion.send(counter+'=', suggested_profile.common_count.to_i) | 80 | suggestion.send(counter+'=', suggested_profile.common_count.to_i) |
73 | suggestion.save! | 81 | suggestion.save! |
@@ -87,6 +95,7 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -87,6 +95,7 @@ class ProfileSuggestion < ActiveRecord::Base | ||
87 | 95 | ||
88 | def self.people_with_common_friends(person) | 96 | def self.people_with_common_friends(person) |
89 | person_friends = person.friends.map(&:id) | 97 | person_friends = person.friends.map(&:id) |
98 | + return [] if person_friends.blank? | ||
90 | person.environment.people. | 99 | person.environment.people. |
91 | select("profiles.*, suggestions.count AS common_count"). | 100 | select("profiles.*, suggestions.count AS common_count"). |
92 | joins(" | 101 | joins(" |
@@ -100,6 +109,7 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -100,6 +109,7 @@ class ProfileSuggestion < ActiveRecord::Base | ||
100 | 109 | ||
101 | def self.people_with_common_communities(person) | 110 | def self.people_with_common_communities(person) |
102 | person_communities = person.communities.map(&:id) | 111 | person_communities = person.communities.map(&:id) |
112 | + return [] if person_communities.blank? | ||
103 | person.environment.people. | 113 | person.environment.people. |
104 | select("profiles.*, suggestions.count AS common_count"). | 114 | select("profiles.*, suggestions.count AS common_count"). |
105 | joins(" | 115 | joins(" |
@@ -115,6 +125,7 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -115,6 +125,7 @@ class ProfileSuggestion < ActiveRecord::Base | ||
115 | 125 | ||
116 | def self.people_with_common_tags(person) | 126 | def self.people_with_common_tags(person) |
117 | profile_tags = person.articles.select('tags.id').joins(:tags).map(&:id) | 127 | profile_tags = person.articles.select('tags.id').joins(:tags).map(&:id) |
128 | + return [] if profile_tags.blank? | ||
118 | person.environment.people. | 129 | person.environment.people. |
119 | select("profiles.*, suggestions.count as common_count"). | 130 | select("profiles.*, suggestions.count as common_count"). |
120 | joins(" | 131 | joins(" |
@@ -132,6 +143,7 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -132,6 +143,7 @@ class ProfileSuggestion < ActiveRecord::Base | ||
132 | 143 | ||
133 | def self.communities_with_common_friends(person) | 144 | def self.communities_with_common_friends(person) |
134 | person_friends = person.friends.map(&:id) | 145 | person_friends = person.friends.map(&:id) |
146 | + return [] if person_friends.blank? | ||
135 | person.environment.communities. | 147 | person.environment.communities. |
136 | select("profiles.*, suggestions.count AS common_count"). | 148 | select("profiles.*, suggestions.count AS common_count"). |
137 | joins(" | 149 | joins(" |
@@ -147,6 +159,7 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -147,6 +159,7 @@ class ProfileSuggestion < ActiveRecord::Base | ||
147 | 159 | ||
148 | def self.communities_with_common_tags(person) | 160 | def self.communities_with_common_tags(person) |
149 | profile_tags = person.articles.select('tags.id').joins(:tags).map(&:id) | 161 | profile_tags = person.articles.select('tags.id').joins(:tags).map(&:id) |
162 | + return [] if profile_tags.blank? | ||
150 | person.environment.communities. | 163 | person.environment.communities. |
151 | select("profiles.*, suggestions.count AS common_count"). | 164 | select("profiles.*, suggestions.count AS common_count"). |
152 | joins(" | 165 | joins(" |
@@ -164,15 +177,17 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -164,15 +177,17 @@ class ProfileSuggestion < ActiveRecord::Base | ||
164 | 177 | ||
165 | def disable | 178 | def disable |
166 | self.enabled = false | 179 | self.enabled = false |
167 | - self.save | 180 | + self.save! |
181 | + self.class.generate_profile_suggestions(self.person) | ||
168 | end | 182 | end |
169 | 183 | ||
170 | def self.generate_all_profile_suggestions | 184 | def self.generate_all_profile_suggestions |
171 | Delayed::Job.enqueue(ProfileSuggestion::GenerateAllJob.new) unless ProfileSuggestion::GenerateAllJob.exists? | 185 | Delayed::Job.enqueue(ProfileSuggestion::GenerateAllJob.new) unless ProfileSuggestion::GenerateAllJob.exists? |
172 | end | 186 | end |
173 | 187 | ||
174 | - def self.generate_profile_suggestions(person_id) | ||
175 | - Delayed::Job.enqueue ProfileSuggestionsJob.new(person_id) unless ProfileSuggestionsJob.exists?(person_id) | 188 | + def self.generate_profile_suggestions(person, force = false) |
189 | + return if person.profile_suggestions.enabled.count >= MIN_LIMIT && !force | ||
190 | + Delayed::Job.enqueue ProfileSuggestionsJob.new(person.id) unless ProfileSuggestionsJob.exists?(person.id) | ||
176 | end | 191 | end |
177 | 192 | ||
178 | class GenerateAllJob | 193 | class GenerateAllJob |
@@ -181,7 +196,7 @@ class ProfileSuggestion < ActiveRecord::Base | @@ -181,7 +196,7 @@ class ProfileSuggestion < ActiveRecord::Base | ||
181 | end | 196 | end |
182 | 197 | ||
183 | def perform | 198 | def perform |
184 | - Person.find_each {|person| ProfileSuggestion.generate_profile_suggestions(person.id) } | 199 | + Person.find_each {|person| ProfileSuggestion.generate_profile_suggestions(person) } |
185 | end | 200 | end |
186 | end | 201 | end |
187 | 202 |
test/unit/profile_suggestion_test.rb
@@ -221,84 +221,136 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | @@ -221,84 +221,136 @@ class ProfileSuggestionTest < ActiveSupport::TestCase | ||
221 | ProfileSuggestion.calculate_suggestions(person) | 221 | ProfileSuggestion.calculate_suggestions(person) |
222 | end | 222 | end |
223 | 223 | ||
224 | - should 'update existing person suggestion when the number of common friends increase' do | ||
225 | - suggested_person = create_user('test_user').person | ||
226 | - suggestion = ProfileSuggestion.create(:person => person, :suggestion => suggested_person, :common_friends => 2) | ||
227 | - | ||
228 | - friend = create_user('friend').person | ||
229 | - friend2 = create_user('friend2').person | ||
230 | - friend3 = create_user('friend2').person | ||
231 | - person.add_friend friend | ||
232 | - person.add_friend friend2 | ||
233 | - person.add_friend friend3 | ||
234 | - | ||
235 | - friend.add_friend suggested_person | ||
236 | - | ||
237 | - suggested_person.add_friend friend | ||
238 | - suggested_person.add_friend friend2 | ||
239 | - suggested_person.add_friend friend3 | ||
240 | - | ||
241 | - assert_difference 'ProfileSuggestion.find(suggestion.id).common_friends', 1 do | ||
242 | - ProfileSuggestion.register_suggestions(person, ProfileSuggestion.people_with_common_friends(person), 'people_with_common_friends') | 224 | +#FIXME This might not be necessary anymore... |
225 | +# should 'update existing person suggestion when the number of common friends increase' do | ||
226 | +# suggested_person = create_user('test_user').person | ||
227 | +# suggestion = ProfileSuggestion.create(:person => person, :suggestion => suggested_person, :common_friends => 2) | ||
228 | +# | ||
229 | +# friend = create_user('friend').person | ||
230 | +# friend2 = create_user('friend2').person | ||
231 | +# friend3 = create_user('friend2').person | ||
232 | +# person.add_friend friend | ||
233 | +# person.add_friend friend2 | ||
234 | +# person.add_friend friend3 | ||
235 | +# | ||
236 | +# friend.add_friend suggested_person | ||
237 | +# | ||
238 | +# suggested_person.add_friend friend | ||
239 | +# suggested_person.add_friend friend2 | ||
240 | +# suggested_person.add_friend friend3 | ||
241 | +# | ||
242 | +# assert_difference 'ProfileSuggestion.find(suggestion.id).common_friends', 1 do | ||
243 | +# ProfileSuggestion.register_suggestions(person, ProfileSuggestion.people_with_common_friends(person), 'people_with_common_friends') | ||
244 | +# end | ||
245 | +# end | ||
246 | +# | ||
247 | +# should 'update existing person suggestion when the number of common communities increase' do | ||
248 | +# suggested_person = create_user('test_user').person | ||
249 | +# suggestion = ProfileSuggestion.create(:person => person, :suggestion => suggested_person, :common_communities => 1) | ||
250 | +# | ||
251 | +# community.add_member person | ||
252 | +# community.add_member suggested_person | ||
253 | +# | ||
254 | +# community2 = fast_create(Community) | ||
255 | +# community2.add_member person | ||
256 | +# community2.add_member suggested_person | ||
257 | +# | ||
258 | +# assert_difference 'ProfileSuggestion.find(suggestion.id).common_communities', 1 do | ||
259 | +# ProfileSuggestion.register_suggestions(person, ProfileSuggestion.people_with_common_communities(person), 'people_with_common_communities') | ||
260 | +# end | ||
261 | +# end | ||
262 | +# | ||
263 | +# should 'update existing person suggestion when the number of common tags increase' do | ||
264 | +# suggested_person = create_user('test_user').person | ||
265 | +# suggestion = ProfileSuggestion.create(:person => person, :suggestion => suggested_person, :common_tags => 1) | ||
266 | +# | ||
267 | +# create(Article, :created_by => person, :profile => person, :tag_list => 'first-tag, second-tag, third-tag, fourth-tag') | ||
268 | +# create(Article, :created_by => suggested_person, :profile => suggested_person, :tag_list => 'first-tag, second-tag, third-tag') | ||
269 | +# | ||
270 | +# assert_difference 'ProfileSuggestion.find(suggestion.id).common_tags', 2 do | ||
271 | +# ProfileSuggestion.register_suggestions(person, ProfileSuggestion.people_with_common_tags(person), 'people_with_common_tags') | ||
272 | +# end | ||
273 | +# end | ||
274 | +# | ||
275 | +# should 'update existing community suggestion when the number of common friends increase' do | ||
276 | +# suggestion = ProfileSuggestion.create(:person => person, :suggestion => community, :common_friends => 1) | ||
277 | +# | ||
278 | +# member1 = create_user('member1').person | ||
279 | +# member2 = create_user('member2').person | ||
280 | +# | ||
281 | +# person.add_friend member1 | ||
282 | +# person.add_friend member2 | ||
283 | +# | ||
284 | +# community.add_member member1 | ||
285 | +# community.add_member member2 | ||
286 | +# | ||
287 | +# assert_difference 'ProfileSuggestion.find(suggestion.id).common_friends', 1 do | ||
288 | +# ProfileSuggestion.register_suggestions(person, ProfileSuggestion.communities_with_common_friends(person), 'communities_with_common_friends') | ||
289 | +# end | ||
290 | +# | ||
291 | +# end | ||
292 | +# | ||
293 | +# should 'update existing community suggestion when the number of common tags increase' do | ||
294 | +# other_person = create_user('test_user').person | ||
295 | +# | ||
296 | +# suggestion = ProfileSuggestion.create(:person => person, :suggestion => community, :common_tags => 1) | ||
297 | +# | ||
298 | +# create(Article, :created_by => person, :profile => person, :tag_list => 'first-tag, second-tag, third-tag, fourth-tag') | ||
299 | +# create(Article, :created_by => other_person, :profile => community, :tag_list => 'first-tag, second-tag, third-tag') | ||
300 | +# | ||
301 | +# assert_difference 'ProfileSuggestion.find(suggestion.id).common_tags', 2 do | ||
302 | +# ProfileSuggestion.register_suggestions(person, ProfileSuggestion.communities_with_common_tags(person), 'communities_with_common_tags') | ||
303 | +# end | ||
304 | +# end | ||
305 | + | ||
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)) | ||
243 | end | 310 | end |
244 | - end | ||
245 | 311 | ||
246 | - should 'update existing person suggestion when the number of common communities increase' do | ||
247 | - suggested_person = create_user('test_user').person | ||
248 | - suggestion = ProfileSuggestion.create(:person => person, :suggestion => suggested_person, :common_communities => 1) | 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})") | ||
249 | 316 | ||
250 | - community.add_member person | ||
251 | - community.add_member suggested_person | ||
252 | - | ||
253 | - community2 = fast_create(Community) | ||
254 | - community2.add_member person | ||
255 | - community2.add_member suggested_person | ||
256 | - | ||
257 | - assert_difference 'ProfileSuggestion.find(suggestion.id).common_communities', 1 do | ||
258 | - ProfileSuggestion.register_suggestions(person, ProfileSuggestion.people_with_common_communities(person), 'people_with_common_communities') | 317 | + assert_difference 'ProfileSuggestion.count', 1 do |
318 | + ProfileSuggestion.register_suggestions(person, suggested_profiles, 'people_with_common_friends') | ||
259 | end | 319 | end |
260 | end | 320 | end |
261 | 321 | ||
262 | - should 'update existing person suggestion when the number of common tags increase' do | ||
263 | - suggested_person = create_user('test_user').person | ||
264 | - suggestion = ProfileSuggestion.create(:person => person, :suggestion => suggested_person, :common_tags => 1) | ||
265 | - | ||
266 | - create(Article, :created_by => person, :profile => person, :tag_list => 'first-tag, second-tag, third-tag, fourth-tag') | ||
267 | - create(Article, :created_by => suggested_person, :profile => suggested_person, :tag_list => 'first-tag, second-tag, third-tag') | ||
268 | - | ||
269 | - assert_difference 'ProfileSuggestion.find(suggestion.id).common_tags', 2 do | ||
270 | - ProfileSuggestion.register_suggestions(person, ProfileSuggestion.people_with_common_tags(person), 'people_with_common_tags') | 322 | + should 'calculate new suggestions when number of available suggestions reaches the min_limit' do |
323 | + person = create_user('person').person | ||
324 | + (ProfileSuggestion::MIN_LIMIT + 1).times do | ||
325 | + ProfileSuggestion.create!(:person => person, :suggestion => fast_create(Profile)) | ||
271 | end | 326 | end |
272 | - end | ||
273 | - | ||
274 | - should 'update existing community suggestion when the number of common friends increase' do | ||
275 | - suggestion = ProfileSuggestion.create(:person => person, :suggestion => community, :common_friends => 1) | ||
276 | - | ||
277 | - member1 = create_user('member1').person | ||
278 | - member2 = create_user('member2').person | ||
279 | 327 | ||
280 | - person.add_friend member1 | ||
281 | - person.add_friend member2 | 328 | + ProfileSuggestion.expects(:calculate_suggestions) |
282 | 329 | ||
283 | - community.add_member member1 | ||
284 | - community.add_member member2 | 330 | + person.profile_suggestions.enabled.last.disable |
331 | + person.profile_suggestions.enabled.last.destroy | ||
332 | + process_delayed_job_queue | ||
333 | + end | ||
285 | 334 | ||
286 | - assert_difference 'ProfileSuggestion.find(suggestion.id).common_friends', 1 do | ||
287 | - ProfileSuggestion.register_suggestions(person, ProfileSuggestion.communities_with_common_friends(person), 'communities_with_common_friends') | 335 | + should 'not create job to calculate new suggestions if there is already enough suggestions enabled' do |
336 | + person = create_user('person').person | ||
337 | + (ProfileSuggestion::MIN_LIMIT + 1).times do | ||
338 | + ProfileSuggestion.create!(:person => person, :suggestion => fast_create(Profile)) | ||
288 | end | 339 | end |
289 | 340 | ||
341 | + ProfileSuggestion.expects(:calculate_suggestions).never | ||
342 | + ProfileSuggestion.generate_profile_suggestions(person) | ||
343 | + process_delayed_job_queue | ||
290 | end | 344 | end |
291 | 345 | ||
292 | - should 'update existing community suggestion when the number of common tags increase' do | ||
293 | - other_person = create_user('test_user').person | ||
294 | - | ||
295 | - suggestion = ProfileSuggestion.create(:person => person, :suggestion => community, :common_tags => 1) | ||
296 | - | ||
297 | - create(Article, :created_by => person, :profile => person, :tag_list => 'first-tag, second-tag, third-tag, fourth-tag') | ||
298 | - create(Article, :created_by => other_person, :profile => community, :tag_list => 'first-tag, second-tag, third-tag') | ||
299 | - | ||
300 | - assert_difference 'ProfileSuggestion.find(suggestion.id).common_tags', 2 do | ||
301 | - ProfileSuggestion.register_suggestions(person, ProfileSuggestion.communities_with_common_tags(person), 'communities_with_common_tags') | 346 | + should 'be able to force suggestions calculation' do |
347 | + person = create_user('person').person | ||
348 | + (ProfileSuggestion::MIN_LIMIT + 1).times do | ||
349 | + ProfileSuggestion.create!(:person => person, :suggestion => fast_create(Profile)) | ||
302 | end | 350 | end |
351 | + | ||
352 | + ProfileSuggestion.expects(:calculate_suggestions) | ||
353 | + ProfileSuggestion.generate_profile_suggestions(person, true) | ||
354 | + process_delayed_job_queue | ||
303 | end | 355 | end |
304 | end | 356 | end |