Merge pull request #1672 from bookwyrm-social/unused-view

Removes unused groups view

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():
+        model = apps.get_model("bookwyrm.Notification", require_ready=True)
-            GroupMember.from_request(self)
+        # 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)
+        # let the other members know about it
-            # tell the group owner
+        for membership in self.group.memberships.all():
-            model.objects.create(
+            member = membership.user
-                user=self.group.user,
+            if member not in (self.user, self.group.user):
-                related_user=self.user,
+                model.objects.create(
-                related_group=self.group,
+                    user=member,
-                notification_type="ACCEPT",
+                    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"""

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")
 

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"):
 

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"""

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)

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(
+        self.testgroup = models.Group.objects.create(
-                name="Test Group",
+            name="Test Group",
-                description="Initial description",
+            description="Initial description",
-                user=self.local_user,
+            user=self.local_user,
-                privacy="public",
+            privacy="public",
-            )
+        )
-            self.membership = models.GroupMember.objects.create(
+        self.membership = models.GroupMember.objects.create(
-                group=self.testgroup, user=self.local_user
+            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()])

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)

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 = get_object_or_404(models.Group, id=request.POST.get("group"))
-    group = models.Group.objects.get(id=request.POST["group"])
+    invite = get_object_or_404(
-    if not group:
+        models.GroupMemberInvitation, group=group, user=request.user
-        return HttpResponseBadRequest()
+    )
-
-    invite = models.GroupMemberInvitation.objects.get(group=group, user=request.user)
-    if not invite:
-        return HttpResponseBadRequest()
 
     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"])
+    invite.reject()
-    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
-
     return redirect(request.user.local_path)

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():
+        if not form.is_valid():
-            include_reviews = request.POST.get("include_reviews") == "on"
+            return HttpResponseBadRequest()
-            privacy = request.POST.get("privacy")
-            source = request.POST.get("source")
 
-            importer = None
+        include_reviews = request.POST.get("include_reviews") == "on"
-            if source == "LibraryThing":
+        privacy = request.POST.get("privacy")
-                importer = LibrarythingImporter()
+        source = request.POST.get("source")
-            elif source == "Storygraph":
-                importer = StorygraphImporter()
-            else:
-                # Default : Goodreads
-                importer = GoodreadsImporter()
 
-            try:
+        importer = None
-                job = importer.create_job(
+        if source == "LibraryThing":
-                    request.user,
+            importer = LibrarythingImporter()
-                    TextIOWrapper(
+        elif source == "Storygraph":
-                        request.FILES["csv_file"], encoding=importer.encoding
+            importer = StorygraphImporter()
-                    ),
+        else:
-                    include_reviews,
+            # Default : Goodreads
-                    privacy,
+            importer = GoodreadsImporter()
-                )
-            except (UnicodeDecodeError, ValueError, KeyError):
-                return HttpResponseBadRequest(_("Not a valid csv file"))
 
-            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}")
+        importer.start_import(job)
-        return HttpResponseBadRequest()
+
+        return redirect(f"/import/{job.id}")

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()
+            shelves = models.Shelf.privacy_filter(
-            if follower:
+                request.user, privacy_levels=["public", "followers"]
-                shelves = shelves.filter(privacy__in=["public", "followers"])
+            ).filter(user=user)
-            else:
-                shelves = shelves.filter(privacy="public")
 
         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):