diff --git a/bookwyrm/models/group.py b/bookwyrm/models/group.py index bd5b8d410..05ed39a27 100644 --- a/bookwyrm/models/group.py +++ b/bookwyrm/models/group.py @@ -73,7 +73,7 @@ class GroupMember(models.Model): ) ).exists(): raise IntegrityError() - # accepts and requests are handled by the GroupInvitation model + # accepts and requests are handled by the GroupMemberInvitation model super().save(*args, **kwargs) @classmethod @@ -150,31 +150,30 @@ class GroupMemberInvitation(models.Model): notification_type=notification_type, ) + @transaction.atomic def accept(self): """turn this request into the real deal""" + GroupMember.from_request(self) - with transaction.atomic(): - GroupMember.from_request(self) + model = apps.get_model("bookwyrm.Notification", require_ready=True) + # tell the group owner + model.objects.create( + user=self.group.user, + related_user=self.user, + related_group=self.group, + notification_type="ACCEPT", + ) - model = apps.get_model("bookwyrm.Notification", require_ready=True) - # tell the group owner - model.objects.create( - user=self.group.user, - related_user=self.user, - related_group=self.group, - notification_type="ACCEPT", - ) - - # let the other members know about it - for membership in self.group.memberships.all(): - member = membership.user - if member not in (self.user, self.group.user): - model.objects.create( - user=member, - related_user=self.user, - related_group=self.group, - notification_type="JOIN", - ) + # let the other members know about it + for membership in self.group.memberships.all(): + member = membership.user + if member not in (self.user, self.group.user): + model.objects.create( + user=member, + related_user=self.user, + related_group=self.group, + notification_type="JOIN", + ) def reject(self): """generate a Reject for this membership request""" diff --git a/bookwyrm/tests/models/test_book_model.py b/bookwyrm/tests/models/test_book_model.py index cad00d43a..d74fa4ca3 100644 --- a/bookwyrm/tests/models/test_book_model.py +++ b/bookwyrm/tests/models/test_book_model.py @@ -1,5 +1,7 @@ """ testing models """ from dateutil.parser import parse + +from imagekit.models import ImageSpecField from django.test import TestCase from django.utils import timezone @@ -26,11 +28,22 @@ class Book(TestCase): def test_remote_id(self): """fanciness with remote/origin ids""" - remote_id = "https://%s/book/%d" % (settings.DOMAIN, self.work.id) + remote_id = f"https://{settings.DOMAIN}/book/{self.work.id}" self.assertEqual(self.work.get_remote_id(), remote_id) self.assertEqual(self.work.remote_id, remote_id) - def test_create_book(self): + def test_generated_links(self): + """links produced from identifiers""" + book = models.Edition.objects.create( + title="ExEd", + parent_work=self.work, + openlibrary_key="OL123M", + inventaire_id="isbn:123", + ) + self.assertEqual(book.openlibrary_link, "https://openlibrary.org/books/OL123M") + self.assertEqual(book.inventaire_link, "https://inventaire.io/entity/isbn:123") + + def test_create_book_invalid(self): """you shouldn't be able to create Books (only editions and works)""" self.assertRaises(ValueError, models.Book.objects.create, title="Invalid Book") diff --git a/bookwyrm/tests/models/test_group.py b/bookwyrm/tests/models/test_group.py index 2dd3cee18..25bbaf424 100644 --- a/bookwyrm/tests/models/test_group.py +++ b/bookwyrm/tests/models/test_group.py @@ -2,7 +2,7 @@ from unittest.mock import patch from django.test import TestCase -from bookwyrm import models, settings +from bookwyrm import models @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") @@ -76,7 +76,8 @@ class Group(TestCase): models.GroupMember.objects.create(group=self.public_group, user=self.capybara) def test_group_members_can_see_private_groups(self, _): - """direct privacy group should not be excluded from group listings for group members viewing""" + """direct privacy group should not be excluded from group listings for group + members viewing""" rat_groups = models.Group.privacy_filter(self.rat).all() badger_groups = models.Group.privacy_filter(self.badger).all() @@ -85,7 +86,8 @@ class Group(TestCase): self.assertTrue(self.private_group in badger_groups) def test_group_members_can_see_followers_only_lists(self, _): - """follower-only group booklists should not be excluded from group booklist listing for group members who do not follower list owner""" + """follower-only group booklists should not be excluded from group booklist + listing for group members who do not follower list owner""" with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): followers_list = models.List.objects.create( @@ -105,7 +107,8 @@ class Group(TestCase): self.assertTrue(followers_list in capybara_lists) def test_group_members_can_see_private_lists(self, _): - """private group booklists should not be excluded from group booklist listing for group members""" + """private group booklists should not be excluded from group booklist listing + for group members""" with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): diff --git a/bookwyrm/tests/views/books/test_book.py b/bookwyrm/tests/views/books/test_book.py index cb73e3813..90f686d5b 100644 --- a/bookwyrm/tests/views/books/test_book.py +++ b/bookwyrm/tests/views/books/test_book.py @@ -231,6 +231,32 @@ class BookViews(TestCase): views.update_book_from_remote(request, self.book.id, "openlibrary.org") self.assertEqual(mock.call_count, 1) + def test_resolve_book(self): + """load a book from search results""" + models.Connector.objects.create( + identifier="openlibrary.org", + name="OpenLibrary", + connector_file="openlibrary", + base_url="https://openlibrary.org", + books_url="https://openlibrary.org", + covers_url="https://covers.openlibrary.org", + search_url="https://openlibrary.org/search?q=", + isbn_search_url="https://openlibrary.org/isbn", + ) + request = self.factory.post( + "", {"remote_id": "https://openlibrary.org/book/123"} + ) + request.user = self.local_user + + with patch( + "bookwyrm.connectors.openlibrary.Connector.get_or_create_book" + ) as mock: + mock.return_value = self.book + result = views.resolve_book(request) + self.assertEqual(mock.call_count, 1) + self.assertEqual(mock.call_args[0][0], "https://openlibrary.org/book/123") + self.assertEqual(result.status_code, 302) + def _setup_cover_url(): """creates cover url mock""" diff --git a/bookwyrm/tests/views/imports/test_import.py b/bookwyrm/tests/views/imports/test_import.py index b8b8b328d..c1c5472f5 100644 --- a/bookwyrm/tests/views/imports/test_import.py +++ b/bookwyrm/tests/views/imports/test_import.py @@ -1,13 +1,14 @@ """ test for app action functionality """ import pathlib from unittest.mock import patch + from django.core.files.uploadedfile import SimpleUploadedFile from django.template.response import TemplateResponse from django.test import TestCase from django.test.client import RequestFactory -from bookwyrm.tests.validate_html import validate_html from bookwyrm import forms, models, views +from bookwyrm.tests.validate_html import validate_html class ImportViews(TestCase): @@ -44,15 +45,29 @@ class ImportViews(TestCase): import_job = models.ImportJob.objects.create(user=self.local_user, mappings={}) request = self.factory.get("") request.user = self.local_user - with patch("bookwyrm.tasks.app.AsyncResult") as async_result: - async_result.return_value = [] - result = view(request, import_job.id) + + result = view(request, import_job.id) + self.assertIsInstance(result, TemplateResponse) validate_html(result.render()) self.assertEqual(result.status_code, 200) + def test_import_status_reformat(self): + """there are so many views, this just makes sure it LOADS""" + view = views.ImportStatus.as_view() + import_job = models.ImportJob.objects.create(user=self.local_user, mappings={}) + request = self.factory.post("") + request.user = self.local_user + with patch( + "bookwyrm.importers.goodreads_import.GoodreadsImporter.update_legacy_job" + ) as mock: + result = view(request, import_job.id) + self.assertEqual(mock.call_args[0][0], import_job) + + self.assertEqual(result.status_code, 302) + def test_start_import(self): - """retry failed items""" + """start a job""" view = views.Import.as_view() form = forms.ImportForm() form.data["source"] = "Goodreads" @@ -74,3 +89,19 @@ class ImportViews(TestCase): job = models.ImportJob.objects.get() self.assertFalse(job.include_reviews) self.assertEqual(job.privacy, "public") + + def test_retry_item(self): + """try again on a single row""" + job = models.ImportJob.objects.create(user=self.local_user, mappings={}) + item = models.ImportItem.objects.create( + index=0, + job=job, + fail_reason="no match", + data={}, + normalized_data={}, + ) + request = self.factory.post("") + request.user = self.local_user + with patch("bookwyrm.importers.importer.import_item_task.delay") as mock: + views.retry_item(request, job.id, item.id) + self.assertEqual(mock.call_count, 1) diff --git a/bookwyrm/tests/views/test_group.py b/bookwyrm/tests/views/test_group.py index b18ce6b4f..40f601f17 100644 --- a/bookwyrm/tests/views/test_group.py +++ b/bookwyrm/tests/views/test_group.py @@ -1,12 +1,11 @@ """ test for app action functionality """ from unittest.mock import patch -from django.contrib.auth import decorators from django.template.response import TemplateResponse from django.test import TestCase from django.test.client import RequestFactory -from bookwyrm import models, views, forms +from bookwyrm import models, views from bookwyrm.tests.validate_html import validate_html @@ -27,16 +26,23 @@ class GroupViews(TestCase): local=True, localname="mouse", ) + self.rat = models.User.objects.create_user( + "rat@local.com", + "rat@rat.rat", + "password", + local=True, + localname="rat", + ) - self.testgroup = models.Group.objects.create( - name="Test Group", - description="Initial description", - user=self.local_user, - privacy="public", - ) - self.membership = models.GroupMember.objects.create( - group=self.testgroup, user=self.local_user - ) + self.testgroup = models.Group.objects.create( + name="Test Group", + description="Initial description", + user=self.local_user, + privacy="public", + ) + self.membership = models.GroupMember.objects.create( + group=self.testgroup, user=self.local_user + ) models.SiteSettings.objects.create() @@ -98,7 +104,6 @@ class GroupViews(TestCase): def test_group_edit(self, _): """test editing a "group" database entry""" - view = views.Group.as_view() request = self.factory.post( "", @@ -117,3 +122,137 @@ class GroupViews(TestCase): self.assertEqual(self.testgroup.name, "Updated Group name") self.assertEqual(self.testgroup.description, "wow") self.assertEqual(self.testgroup.privacy, "direct") + + def test_delete_group(self, _): + """delete a group""" + request = self.factory.post("") + request.user = self.local_user + views.delete_group(request, self.testgroup.id) + self.assertFalse(models.Group.objects.exists()) + + def test_invite_member(self, _): + """invite a member to a group""" + request = self.factory.post( + "", + { + "group": self.testgroup.id, + "user": self.rat.localname, + }, + ) + request.user = self.local_user + result = views.invite_member(request) + self.assertEqual(result.status_code, 302) + + invite = models.GroupMemberInvitation.objects.get() + self.assertEqual(invite.user, self.rat) + self.assertEqual(invite.group, self.testgroup) + + def test_invite_member_twice(self, _): + """invite a member to a group again""" + request = self.factory.post( + "", + { + "group": self.testgroup.id, + "user": self.rat.localname, + }, + ) + request.user = self.local_user + result = views.invite_member(request) + self.assertEqual(result.status_code, 302) + result = views.invite_member(request) + self.assertEqual(result.status_code, 302) + + def test_remove_member_denied(self, _): + """remove member""" + request = self.factory.post( + "", + { + "group": self.testgroup.id, + "user": self.local_user.localname, + }, + ) + request.user = self.local_user + result = views.remove_member(request) + self.assertEqual(result.status_code, 400) + + def test_remove_member_non_member(self, _): + """remove member but wait, that's not a member""" + request = self.factory.post( + "", + { + "group": self.testgroup.id, + "user": self.rat.localname, + }, + ) + request.user = self.local_user + result = views.remove_member(request) + # nothing happens + self.assertEqual(result.status_code, 302) + + def test_remove_member_invited(self, _): + """remove an invited member""" + models.GroupMemberInvitation.objects.create( + user=self.rat, + group=self.testgroup, + ) + request = self.factory.post( + "", + { + "group": self.testgroup.id, + "user": self.rat.localname, + }, + ) + request.user = self.local_user + result = views.remove_member(request) + self.assertEqual(result.status_code, 302) + self.assertFalse(models.GroupMemberInvitation.objects.exists()) + + def test_remove_member_existing_member(self, _): + """remove an invited member""" + models.GroupMember.objects.create( + user=self.rat, + group=self.testgroup, + ) + request = self.factory.post( + "", + { + "group": self.testgroup.id, + "user": self.rat.localname, + }, + ) + request.user = self.local_user + result = views.remove_member(request) + self.assertEqual(result.status_code, 302) + self.assertEqual(models.GroupMember.objects.count(), 1) + self.assertEqual(models.GroupMember.objects.first().user, self.local_user) + notification = models.Notification.objects.get() + self.assertEqual(notification.user, self.rat) + self.assertEqual(notification.related_group, self.testgroup) + self.assertEqual(notification.notification_type, "REMOVE") + + def test_accept_membership(self, _): + """accept an invite""" + models.GroupMemberInvitation.objects.create( + user=self.rat, + group=self.testgroup, + ) + request = self.factory.post("", {"group": self.testgroup.id}) + request.user = self.rat + views.accept_membership(request) + + self.assertFalse(models.GroupMemberInvitation.objects.exists()) + self.assertTrue(self.rat in [m.user for m in self.testgroup.memberships.all()]) + + def test_reject_membership(self, _): + """reject an invite""" + models.GroupMemberInvitation.objects.create( + user=self.rat, + group=self.testgroup, + ) + request = self.factory.post("", {"group": self.testgroup.id}) + request.user = self.rat + views.reject_membership(request) + + self.testgroup.refresh_from_db() + self.assertFalse(models.GroupMemberInvitation.objects.exists()) + self.assertFalse(self.rat in [m.user for m in self.testgroup.memberships.all()]) diff --git a/bookwyrm/tests/views/test_user.py b/bookwyrm/tests/views/test_user.py index ddb029cc9..231ecec99 100644 --- a/bookwyrm/tests/views/test_user.py +++ b/bookwyrm/tests/views/test_user.py @@ -51,6 +51,11 @@ class UserViews(TestCase): def test_user_page(self): """there are so many views, this just makes sure it LOADS""" + # extras that are rendered on the user page + models.AnnualGoal.objects.create( + user=self.local_user, goal=12, privacy="followers" + ) + view = views.User.as_view() request = self.factory.get("") request.user = self.local_user @@ -104,6 +109,18 @@ class UserViews(TestCase): self.assertIsInstance(result, ActivitypubResponse) self.assertEqual(result.status_code, 200) + def test_followers_page_anonymous(self): + """there are so many views, this just makes sure it LOADS""" + view = views.Followers.as_view() + request = self.factory.get("") + request.user = self.anonymous_user + with patch("bookwyrm.views.user.is_api_request") as is_api: + is_api.return_value = False + result = view(request, "mouse") + self.assertIsInstance(result, TemplateResponse) + validate_html(result.render()) + self.assertEqual(result.status_code, 200) + @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") @patch("bookwyrm.activitystreams.populate_stream_task.delay") def test_followers_page_blocked(self, *_): @@ -135,6 +152,18 @@ class UserViews(TestCase): self.assertIsInstance(result, ActivitypubResponse) self.assertEqual(result.status_code, 200) + def test_following_page_anonymous(self): + """there are so many views, this just makes sure it LOADS""" + view = views.Following.as_view() + request = self.factory.get("") + request.user = self.anonymous_user + with patch("bookwyrm.views.user.is_api_request") as is_api: + is_api.return_value = False + result = view(request, "mouse") + self.assertIsInstance(result, TemplateResponse) + validate_html(result.render()) + self.assertEqual(result.status_code, 200) + def test_following_page_blocked(self): """there are so many views, this just makes sure it LOADS""" view = views.Following.as_view() @@ -145,3 +174,15 @@ class UserViews(TestCase): is_api.return_value = False with self.assertRaises(Http404): view(request, "rat") + + def test_hide_suggestions(self): + """update suggestions settings""" + self.assertTrue(self.local_user.show_suggested_users) + request = self.factory.post("") + request.user = self.local_user + + result = views.hide_suggestions(request) + self.assertEqual(result.status_code, 302) + + self.local_user.refresh_from_db() + self.assertFalse(self.local_user.show_suggested_users) diff --git a/bookwyrm/views/group.py b/bookwyrm/views/group.py index 9ee99bffa..f375ac84f 100644 --- a/bookwyrm/views/group.py +++ b/bookwyrm/views/group.py @@ -179,21 +179,14 @@ def delete_group(request, group_id): @login_required def invite_member(request): """invite a member to the group""" - group = get_object_or_404(models.Group, id=request.POST.get("group")) - if not group: - return HttpResponseBadRequest() - user = get_user_from_username(request.user, request.POST["user"]) - if not user: - return HttpResponseBadRequest() if not group.user == request.user: return HttpResponseBadRequest() try: models.GroupMemberInvitation.objects.create(user=user, group=group) - except IntegrityError: pass @@ -204,17 +197,11 @@ def invite_member(request): @login_required def remove_member(request): """remove a member from the group""" - group = get_object_or_404(models.Group, id=request.POST.get("group")) - if not group: - return HttpResponseBadRequest() - user = get_user_from_username(request.user, request.POST["user"]) - if not user: - return HttpResponseBadRequest() # you can't be removed from your own group - if request.POST["user"] == group.user: + if user == group.user: return HttpResponseBadRequest() is_member = models.GroupMember.objects.filter(group=group, user=user).exists() @@ -234,11 +221,9 @@ def remove_member(request): pass if is_member: - try: models.List.remove_from_group(group.user, user) models.GroupMember.remove(group.user, user) - except IntegrityError: pass @@ -271,18 +256,13 @@ def remove_member(request): @login_required def accept_membership(request): """accept an invitation to join a group""" - - group = models.Group.objects.get(id=request.POST["group"]) - if not group: - return HttpResponseBadRequest() - - invite = models.GroupMemberInvitation.objects.get(group=group, user=request.user) - if not invite: - return HttpResponseBadRequest() + group = get_object_or_404(models.Group, id=request.POST.get("group")) + invite = get_object_or_404( + models.GroupMemberInvitation, group=group, user=request.user + ) try: invite.accept() - except IntegrityError: pass @@ -293,19 +273,10 @@ def accept_membership(request): @login_required def reject_membership(request): """reject an invitation to join a group""" + group = get_object_or_404(models.Group, id=request.POST.get("group")) + invite = get_object_or_404( + models.GroupMemberInvitation, group=group, user=request.user + ) - group = models.Group.objects.get(id=request.POST["group"]) - if not group: - return HttpResponseBadRequest() - - invite = models.GroupMemberInvitation.objects.get(group=group, user=request.user) - if not invite: - return HttpResponseBadRequest() - - try: - invite.reject() - - except IntegrityError: - pass - + invite.reject() return redirect(request.user.local_path) diff --git a/bookwyrm/views/imports/import_data.py b/bookwyrm/views/imports/import_data.py index 7f6a4d13f..80386b3de 100644 --- a/bookwyrm/views/imports/import_data.py +++ b/bookwyrm/views/imports/import_data.py @@ -37,33 +37,32 @@ class Import(View): def post(self, request): """ingest a goodreads csv""" form = forms.ImportForm(request.POST, request.FILES) - if form.is_valid(): - include_reviews = request.POST.get("include_reviews") == "on" - privacy = request.POST.get("privacy") - source = request.POST.get("source") + if not form.is_valid(): + return HttpResponseBadRequest() - importer = None - if source == "LibraryThing": - importer = LibrarythingImporter() - elif source == "Storygraph": - importer = StorygraphImporter() - else: - # Default : Goodreads - importer = GoodreadsImporter() + include_reviews = request.POST.get("include_reviews") == "on" + privacy = request.POST.get("privacy") + source = request.POST.get("source") - try: - job = importer.create_job( - request.user, - TextIOWrapper( - request.FILES["csv_file"], encoding=importer.encoding - ), - include_reviews, - privacy, - ) - except (UnicodeDecodeError, ValueError, KeyError): - return HttpResponseBadRequest(_("Not a valid csv file")) + importer = None + if source == "LibraryThing": + importer = LibrarythingImporter() + elif source == "Storygraph": + importer = StorygraphImporter() + else: + # Default : Goodreads + importer = GoodreadsImporter() - importer.start_import(job) + try: + job = importer.create_job( + request.user, + TextIOWrapper(request.FILES["csv_file"], encoding=importer.encoding), + include_reviews, + privacy, + ) + except (UnicodeDecodeError, ValueError, KeyError): + return HttpResponseBadRequest(_("Not a valid csv file")) - return redirect(f"/import/{job.id}") - return HttpResponseBadRequest() + importer.start_import(job) + + return redirect(f"/import/{job.id}") diff --git a/bookwyrm/views/user.py b/bookwyrm/views/user.py index 082408f97..188f8e66e 100644 --- a/bookwyrm/views/user.py +++ b/bookwyrm/views/user.py @@ -34,11 +34,9 @@ class User(View): shelves = user.shelf_set is_self = request.user.id == user.id if not is_self: - follower = user.followers.filter(id=request.user.id).exists() - if follower: - shelves = shelves.filter(privacy__in=["public", "followers"]) - else: - shelves = shelves.filter(privacy="public") + shelves = models.Shelf.privacy_filter( + request.user, privacy_levels=["public", "followers"] + ).filter(user=user) for user_shelf in shelves.all(): if not user_shelf.books.count(): @@ -146,25 +144,6 @@ def annotate_if_follows(user, queryset): ).order_by("-request_user_follows", "-created_date") -class Groups(View): - """list of user's groups view""" - - def get(self, request, username): - """list of groups""" - user = get_user_from_username(request.user, username) - - paginated = Paginator( - models.Group.memberships.filter(user=user).order_by("-created_date"), - PAGE_LENGTH, - ) - data = { - "user": user, - "is_self": request.user.id == user.id, - "group_list": paginated.get_page(request.GET.get("page")), - } - return TemplateResponse(request, "user/groups.html", data) - - @require_POST @login_required def hide_suggestions(request):