Merge pull request #1386 from bookwyrm-social/suggest-users

Make sure unsuitable users don't show up in suggestions
This commit is contained in:
Mouse Reeve 2021-09-08 12:16:13 -07:00 committed by GitHub
commit 9a885456e9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 60 additions and 9 deletions

View file

@ -28,7 +28,7 @@ class FederatedServer(BookWyrmModel):
def block(self): def block(self):
"""block a server""" """block a server"""
self.status = "blocked" self.status = "blocked"
self.save() self.save(update_fields=["status"])
# deactivate all associated users # deactivate all associated users
self.user_set.filter(is_active=True).update( self.user_set.filter(is_active=True).update(
@ -45,7 +45,7 @@ class FederatedServer(BookWyrmModel):
def unblock(self): def unblock(self):
"""unblock a server""" """unblock a server"""
self.status = "federated" self.status = "federated"
self.save() self.save(update_fields=["status"])
self.user_set.filter(deactivation_reason="domain_block").update( self.user_set.filter(deactivation_reason="domain_block").update(
is_active=True, deactivation_reason=None is_active=True, deactivation_reason=None

View file

@ -86,10 +86,12 @@ class SuggestedUsers(RedisStore):
values = self.get_store(self.store_id(user), withscores=True) values = self.get_store(self.store_id(user), withscores=True)
results = [] results = []
# annotate users with mutuals and shared book counts # annotate users with mutuals and shared book counts
for user_id, rank in values[:5]: for user_id, rank in values:
counts = self.get_counts_from_rank(rank) counts = self.get_counts_from_rank(rank)
try: try:
user = models.User.objects.get(id=user_id) user = models.User.objects.get(
id=user_id, is_active=True, bookwyrm_user=True
)
except models.User.DoesNotExist as err: except models.User.DoesNotExist as err:
# if this happens, the suggestions are janked way up # if this happens, the suggestions are janked way up
logger.exception(err) logger.exception(err)
@ -97,6 +99,8 @@ class SuggestedUsers(RedisStore):
user.mutuals = counts["mutuals"] user.mutuals = counts["mutuals"]
# user.shared_books = counts["shared_books"] # user.shared_books = counts["shared_books"]
results.append(user) results.append(user)
if len(results) >= 5:
break
return results return results
@ -178,13 +182,21 @@ def update_suggestions_on_unfollow(sender, instance, **kwargs):
@receiver(signals.post_save, sender=models.User) @receiver(signals.post_save, sender=models.User)
# pylint: disable=unused-argument, too-many-arguments # pylint: disable=unused-argument, too-many-arguments
def add_new_user(sender, instance, created, update_fields=None, **kwargs): def update_user(sender, instance, created, update_fields=None, **kwargs):
"""a new user, wow how cool""" """an updated user, neat"""
# a new user is found, create suggestions for them # a new user is found, create suggestions for them
if created and instance.local: if created and instance.local:
rerank_suggestions_task.delay(instance.id) rerank_suggestions_task.delay(instance.id)
if update_fields and not "discoverable" in update_fields: # we know what fields were updated and discoverability didn't change
if not instance.bookwyrm_user or (
update_fields and not "discoverable" in update_fields
):
return
# deleted the user
if not created and not instance.is_active:
remove_user_task.delay(instance.id)
return return
# this happens on every save, not just when discoverability changes, annoyingly # this happens on every save, not just when discoverability changes, annoyingly
@ -194,6 +206,25 @@ def add_new_user(sender, instance, created, update_fields=None, **kwargs):
remove_user_task.delay(instance.id) remove_user_task.delay(instance.id)
@receiver(signals.post_save, sender=models.FederatedServer)
def domain_level_update(sender, instance, created, update_fields=None, **kwargs):
"""remove users on a domain block"""
if (
not update_fields
or "status" not in update_fields
or instance.application_type != "bookwyrm"
):
return
if instance.status == "blocked":
bulk_remove_instance_task.delay(instance.id)
return
bulk_add_instance_task.delay(instance.id)
# ------------------- TASKS
@app.task(queue="low_priority") @app.task(queue="low_priority")
def rerank_suggestions_task(user_id): def rerank_suggestions_task(user_id):
"""do the hard work in celery""" """do the hard work in celery"""
@ -219,3 +250,17 @@ def remove_suggestion_task(user_id, suggested_user_id):
"""remove a specific user from a specific user's suggestions""" """remove a specific user from a specific user's suggestions"""
suggested_user = models.User.objects.get(id=suggested_user_id) suggested_user = models.User.objects.get(id=suggested_user_id)
suggested_users.remove_suggestion(user_id, suggested_user) suggested_users.remove_suggestion(user_id, suggested_user)
@app.task(queue="low_priority")
def bulk_remove_instance_task(instance_id):
"""remove a bunch of users from recs"""
for user in models.User.objects.filter(federated_server__id=instance_id):
suggested_users.remove_object_from_related_stores(user)
@app.task(queue="low_priority")
def bulk_add_instance_task(instance_id):
"""remove a bunch of users from recs"""
for user in models.User.objects.filter(federated_server__id=instance_id):
suggested_users.rerank_obj(user, update_only=False)

View file

@ -80,7 +80,9 @@ class FederationViews(TestCase):
request.user = self.local_user request.user = self.local_user
request.user.is_superuser = True request.user.is_superuser = True
view(request, server.id) with patch("bookwyrm.suggested_users.bulk_remove_instance_task.delay") as mock:
view(request, server.id)
self.assertEqual(mock.call_count, 1)
server.refresh_from_db() server.refresh_from_db()
self.remote_user.refresh_from_db() self.remote_user.refresh_from_db()
@ -118,7 +120,11 @@ class FederationViews(TestCase):
request.user = self.local_user request.user = self.local_user
request.user.is_superuser = True request.user.is_superuser = True
views.federation.unblock_server(request, server.id) with patch("bookwyrm.suggested_users.bulk_add_instance_task.delay") as mock:
views.federation.unblock_server(request, server.id)
self.assertEqual(mock.call_count, 1)
self.assertEqual(mock.call_args[0][0], server.id)
server.refresh_from_db() server.refresh_from_db()
self.remote_user.refresh_from_db() self.remote_user.refresh_from_db()
self.assertEqual(server.status, "federated") self.assertEqual(server.status, "federated")