From b3bd6822b20423ed018a1d3b9d675d348e08132d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 30 Jan 2021 12:16:57 -0800 Subject: [PATCH 1/3] Make sure templates render rather than checking template paths --- bookwyrm/tests/views/test_authentication.py | 10 +++++----- bookwyrm/tests/views/test_author.py | 10 +++++++--- bookwyrm/tests/views/test_block.py | 3 ++- bookwyrm/tests/views/test_book.py | 7 ++++--- bookwyrm/tests/views/test_federation.py | 3 ++- bookwyrm/tests/views/test_feed.py | 9 +++++---- bookwyrm/tests/views/test_import.py | 5 +++-- bookwyrm/tests/views/test_invite.py | 5 +++-- bookwyrm/tests/views/test_landing.py | 9 ++++----- bookwyrm/tests/views/test_notifications.py | 3 ++- bookwyrm/tests/views/test_password.py | 13 +++++++------ bookwyrm/tests/views/test_search.py | 5 +++-- bookwyrm/tests/views/test_shelf.py | 3 ++- bookwyrm/tests/views/test_tag.py | 3 ++- bookwyrm/tests/views/test_user.py | 9 +++++---- 15 files changed, 56 insertions(+), 41 deletions(-) diff --git a/bookwyrm/tests/views/test_authentication.py b/bookwyrm/tests/views/test_authentication.py index 655772084..dc52719c9 100644 --- a/bookwyrm/tests/views/test_authentication.py +++ b/bookwyrm/tests/views/test_authentication.py @@ -33,7 +33,7 @@ class AuthenticationViews(TestCase): result = login(request) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'login.html') + result.render() self.assertEqual(result.status_code, 200) request.user = self.local_user @@ -94,7 +94,7 @@ class AuthenticationViews(TestCase): }) response = view(request) self.assertEqual(models.User.objects.count(), 1) - self.assertEqual(response.template_name, 'login.html') + response.render() def test_register_invalid_username(self): ''' gotta have an email ''' @@ -109,7 +109,7 @@ class AuthenticationViews(TestCase): }) response = view(request) self.assertEqual(models.User.objects.count(), 1) - self.assertEqual(response.template_name, 'login.html') + response.render() request = self.factory.post( 'register/', @@ -120,7 +120,7 @@ class AuthenticationViews(TestCase): }) response = view(request) self.assertEqual(models.User.objects.count(), 1) - self.assertEqual(response.template_name, 'login.html') + response.render() request = self.factory.post( 'register/', @@ -131,7 +131,7 @@ class AuthenticationViews(TestCase): }) response = view(request) self.assertEqual(models.User.objects.count(), 1) - self.assertEqual(response.template_name, 'login.html') + response.render() def test_register_closed_instance(self): diff --git a/bookwyrm/tests/views/test_author.py b/bookwyrm/tests/views/test_author.py index c00972f39..c92c47501 100644 --- a/bookwyrm/tests/views/test_author.py +++ b/bookwyrm/tests/views/test_author.py @@ -34,6 +34,7 @@ class AuthorViews(TestCase): remote_id='https://example.com/book/1', parent_work=self.work ) + models.SiteSettings.objects.create() def test_author_page(self): @@ -45,7 +46,8 @@ class AuthorViews(TestCase): is_api.return_value = False result = view(request, author.id) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'author.html') + result.render() + self.assertEqual(result.status_code, 200) self.assertEqual(result.status_code, 200) request = self.factory.get('') @@ -66,7 +68,8 @@ class AuthorViews(TestCase): result = view(request, author.id) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'edit_author.html') + result.render() + self.assertEqual(result.status_code, 200) self.assertEqual(result.status_code, 200) @@ -116,4 +119,5 @@ class AuthorViews(TestCase): resp = view(request, author.id) author.refresh_from_db() self.assertEqual(author.name, 'Test Author') - self.assertEqual(resp.template_name, 'edit_author.html') + resp.render() + self.assertEqual(resp.status_code, 200) diff --git a/bookwyrm/tests/views/test_block.py b/bookwyrm/tests/views/test_block.py index f3a0c2f8b..6f85f2822 100644 --- a/bookwyrm/tests/views/test_block.py +++ b/bookwyrm/tests/views/test_block.py @@ -23,6 +23,7 @@ class BlockViews(TestCase): inbox='https://example.com/users/rat/inbox', outbox='https://example.com/users/rat/outbox', ) + models.SiteSettings.objects.create() def test_block_get(self): @@ -32,7 +33,7 @@ class BlockViews(TestCase): request.user = self.local_user result = view(request) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'preferences/blocks.html') + result.render() self.assertEqual(result.status_code, 200) def test_block_post(self): diff --git a/bookwyrm/tests/views/test_book.py b/bookwyrm/tests/views/test_book.py index 8306b8037..32a407d60 100644 --- a/bookwyrm/tests/views/test_book.py +++ b/bookwyrm/tests/views/test_book.py @@ -33,6 +33,7 @@ class BookViews(TestCase): remote_id='https://example.com/book/1', parent_work=self.work ) + models.SiteSettings.objects.create() def test_book_page(self): @@ -44,7 +45,7 @@ class BookViews(TestCase): is_api.return_value = False result = view(request, self.book.id) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'book.html') + result.render() self.assertEqual(result.status_code, 200) request = self.factory.get('') @@ -63,7 +64,7 @@ class BookViews(TestCase): request.user.is_superuser = True result = view(request, self.book.id) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'edit_book.html') + result.render() self.assertEqual(result.status_code, 200) @@ -116,7 +117,7 @@ class BookViews(TestCase): is_api.return_value = False result = view(request, self.work.id) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'editions.html') + result.render() self.assertEqual(result.status_code, 200) request = self.factory.get('') diff --git a/bookwyrm/tests/views/test_federation.py b/bookwyrm/tests/views/test_federation.py index 2a182a21d..70cf41f6e 100644 --- a/bookwyrm/tests/views/test_federation.py +++ b/bookwyrm/tests/views/test_federation.py @@ -15,6 +15,7 @@ class FederationViews(TestCase): self.local_user = models.User.objects.create_user( 'mouse@local.com', 'mouse@mouse.mouse', 'password', local=True, localname='mouse') + models.SiteSettings.objects.create() def test_federation_page(self): @@ -25,5 +26,5 @@ class FederationViews(TestCase): request.user.is_superuser = True result = view(request) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'settings/federation.html') + result.render() self.assertEqual(result.status_code, 200) diff --git a/bookwyrm/tests/views/test_feed.py b/bookwyrm/tests/views/test_feed.py index da597a7cf..879dd2b69 100644 --- a/bookwyrm/tests/views/test_feed.py +++ b/bookwyrm/tests/views/test_feed.py @@ -21,6 +21,7 @@ class FeedMessageViews(TestCase): title='Example Edition', remote_id='https://example.com/book/1', ) + models.SiteSettings.objects.create() def test_feed(self): @@ -30,7 +31,7 @@ class FeedMessageViews(TestCase): request.user = self.local_user result = view(request, 'local') self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'feed/feed.html') + result.render() self.assertEqual(result.status_code, 200) @@ -45,7 +46,7 @@ class FeedMessageViews(TestCase): is_api.return_value = False result = view(request, 'mouse', status.id) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'feed/status.html') + result.render() self.assertEqual(result.status_code, 200) with patch('bookwyrm.views.feed.is_api_request') as is_api: @@ -66,7 +67,7 @@ class FeedMessageViews(TestCase): is_api.return_value = False result = view(request, 'mouse', status.id) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'feed/status.html') + result.render() self.assertEqual(result.status_code, 200) with patch('bookwyrm.views.feed.is_api_request') as is_api: @@ -83,7 +84,7 @@ class FeedMessageViews(TestCase): request.user = self.local_user result = view(request) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'feed/direct_messages.html') + result.render() self.assertEqual(result.status_code, 200) diff --git a/bookwyrm/tests/views/test_import.py b/bookwyrm/tests/views/test_import.py index 14209f24b..ba8f2457b 100644 --- a/bookwyrm/tests/views/test_import.py +++ b/bookwyrm/tests/views/test_import.py @@ -16,6 +16,7 @@ class ImportViews(TestCase): self.local_user = models.User.objects.create_user( 'mouse@local.com', 'mouse@mouse.mouse', 'password', local=True, localname='mouse') + models.SiteSettings.objects.create() def test_import_page(self): @@ -25,7 +26,7 @@ class ImportViews(TestCase): request.user = self.local_user result = view(request) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'import.html') + result.render() self.assertEqual(result.status_code, 200) @@ -39,5 +40,5 @@ class ImportViews(TestCase): async_result.return_value = [] result = view(request, import_job.id) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'import_status.html') + result.render() self.assertEqual(result.status_code, 200) diff --git a/bookwyrm/tests/views/test_invite.py b/bookwyrm/tests/views/test_invite.py index 857416881..e93e7209b 100644 --- a/bookwyrm/tests/views/test_invite.py +++ b/bookwyrm/tests/views/test_invite.py @@ -18,6 +18,7 @@ class InviteViews(TestCase): self.local_user = models.User.objects.create_user( 'mouse@local.com', 'mouse@mouse.mouse', 'password', local=True, localname='mouse') + models.SiteSettings.objects.create() def test_invite_page(self): @@ -32,7 +33,7 @@ class InviteViews(TestCase): invite.return_value = True result = view(request, 'hi') self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'invite.html') + result.render() self.assertEqual(result.status_code, 200) @@ -44,5 +45,5 @@ class InviteViews(TestCase): request.user.is_superuser = True result = view(request) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'settings/manage_invites.html') + result.render() self.assertEqual(result.status_code, 200) diff --git a/bookwyrm/tests/views/test_landing.py b/bookwyrm/tests/views/test_landing.py index 8576af496..5e0e50cff 100644 --- a/bookwyrm/tests/views/test_landing.py +++ b/bookwyrm/tests/views/test_landing.py @@ -18,6 +18,7 @@ class LandingViews(TestCase): local=True, localname='mouse') self.anonymous_user = AnonymousUser self.anonymous_user.is_authenticated = False + models.SiteSettings.objects.create() def test_home_page(self): @@ -27,13 +28,13 @@ class LandingViews(TestCase): request.user = self.local_user result = view(request) self.assertEqual(result.status_code, 200) - self.assertEqual(result.template_name, 'feed/feed.html') + result.render() request.user = self.anonymous_user result = view(request) self.assertIsInstance(result, TemplateResponse) self.assertEqual(result.status_code, 200) - self.assertEqual(result.template_name, 'discover.html') + result.render() def test_about_page(self): @@ -43,7 +44,7 @@ class LandingViews(TestCase): request.user = self.local_user result = view(request) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'about.html') + result.render() self.assertEqual(result.status_code, 200) @@ -53,5 +54,3 @@ class LandingViews(TestCase): request = self.factory.get('') result = view(request) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'discover.html') - self.assertEqual(result.status_code, 200) diff --git a/bookwyrm/tests/views/test_notifications.py b/bookwyrm/tests/views/test_notifications.py index 683424d5a..24fbde1e6 100644 --- a/bookwyrm/tests/views/test_notifications.py +++ b/bookwyrm/tests/views/test_notifications.py @@ -15,6 +15,7 @@ class NotificationViews(TestCase): self.local_user = models.User.objects.create_user( 'mouse@local.com', 'mouse@mouse.mouse', 'password', local=True, localname='mouse') + models.SiteSettings.objects.create() def test_notifications_page(self): ''' there are so many views, this just makes sure it LOADS ''' @@ -23,7 +24,7 @@ class NotificationViews(TestCase): request.user = self.local_user result = view(request) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'notifications.html') + result.render() self.assertEqual(result.status_code, 200) def test_clear_notifications(self): diff --git a/bookwyrm/tests/views/test_password.py b/bookwyrm/tests/views/test_password.py index 8cac2b13c..9fc37fdb9 100644 --- a/bookwyrm/tests/views/test_password.py +++ b/bookwyrm/tests/views/test_password.py @@ -19,6 +19,7 @@ class PasswordViews(TestCase): local=True, localname='mouse') self.anonymous_user = AnonymousUser self.anonymous_user.is_authenticated = False + models.SiteSettings.objects.create(id=1) def test_password_reset_request(self): @@ -29,7 +30,7 @@ class PasswordViews(TestCase): result = view(request) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'password_reset_request.html') + result.render() self.assertEqual(result.status_code, 200) @@ -43,7 +44,7 @@ class PasswordViews(TestCase): request = self.factory.post('', {'email': 'mouse@mouse.com'}) with patch('bookwyrm.emailing.send_email.delay'): resp = view(request) - self.assertEqual(resp.template_name, 'password_reset_request.html') + resp.render() self.assertEqual( models.PasswordReset.objects.get().user, self.local_user) @@ -56,7 +57,7 @@ class PasswordViews(TestCase): request.user = self.anonymous_user result = view(request, code.code) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'password_reset.html') + result.render() self.assertEqual(result.status_code, 200) @@ -82,7 +83,7 @@ class PasswordViews(TestCase): 'confirm-password': 'hi' }) resp = view(request, 'jhgdkfjgdf') - self.assertEqual(resp.template_name, 'password_reset.html') + resp.render() self.assertTrue(models.PasswordReset.objects.exists()) def test_password_reset_mismatch(self): @@ -94,7 +95,7 @@ class PasswordViews(TestCase): 'confirm-password': 'hihi' }) resp = view(request, code.code) - self.assertEqual(resp.template_name, 'password_reset.html') + resp.render() self.assertTrue(models.PasswordReset.objects.exists()) @@ -106,7 +107,7 @@ class PasswordViews(TestCase): result = view(request) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'preferences/change_password.html') + result.render() self.assertEqual(result.status_code, 200) diff --git a/bookwyrm/tests/views/test_search.py b/bookwyrm/tests/views/test_search.py index 3f1d78503..e5cba99c9 100644 --- a/bookwyrm/tests/views/test_search.py +++ b/bookwyrm/tests/views/test_search.py @@ -33,6 +33,7 @@ class ShelfViews(TestCase): connector_file='self_connector', local=True ) + models.SiteSettings.objects.create() def test_search_json_response(self): @@ -89,7 +90,7 @@ class ShelfViews(TestCase): manager.return_value = [search_result] response = view(request) self.assertIsInstance(response, TemplateResponse) - self.assertEqual(response.template_name, 'search_results.html') + response.render() self.assertEqual( response.context_data['book_results'][0].title, 'Gideon the Ninth') @@ -103,6 +104,6 @@ class ShelfViews(TestCase): with patch('bookwyrm.connectors.connector_manager.search'): response = view(request) self.assertIsInstance(response, TemplateResponse) - self.assertEqual(response.template_name, 'search_results.html') + response.render() self.assertEqual( response.context_data['user_results'][0], self.local_user) diff --git a/bookwyrm/tests/views/test_shelf.py b/bookwyrm/tests/views/test_shelf.py index 95150fe9d..f67bbc562 100644 --- a/bookwyrm/tests/views/test_shelf.py +++ b/bookwyrm/tests/views/test_shelf.py @@ -29,6 +29,7 @@ class ShelfViews(TestCase): identifier='test-shelf', user=self.local_user ) + models.SiteSettings.objects.create() def test_shelf_page(self): @@ -41,7 +42,7 @@ class ShelfViews(TestCase): is_api.return_value = False result = view(request, self.local_user.username, shelf.identifier) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'user/shelf.html') + result.render() self.assertEqual(result.status_code, 200) with patch('bookwyrm.views.shelf.is_api_request') as is_api: diff --git a/bookwyrm/tests/views/test_tag.py b/bookwyrm/tests/views/test_tag.py index 1556139ca..3dfef9a11 100644 --- a/bookwyrm/tests/views/test_tag.py +++ b/bookwyrm/tests/views/test_tag.py @@ -33,6 +33,7 @@ class TagViews(TestCase): remote_id='https://example.com/book/1', parent_work=self.work ) + models.SiteSettings.objects.create() def test_tag_page(self): @@ -46,7 +47,7 @@ class TagViews(TestCase): is_api.return_value = False result = view(request, tag.identifier) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'tag.html') + result.render() self.assertEqual(result.status_code, 200) request = self.factory.get('') diff --git a/bookwyrm/tests/views/test_user.py b/bookwyrm/tests/views/test_user.py index 61fcdb641..616a6575f 100644 --- a/bookwyrm/tests/views/test_user.py +++ b/bookwyrm/tests/views/test_user.py @@ -23,6 +23,7 @@ class UserViews(TestCase): self.rat = models.User.objects.create_user( 'rat@local.com', 'rat@rat.rat', 'password', local=True, localname='rat') + models.SiteSettings.objects.create() def test_user_page(self): @@ -34,7 +35,7 @@ class UserViews(TestCase): is_api.return_value = False result = view(request, 'mouse') self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'user/user.html') + result.render() self.assertEqual(result.status_code, 200) with patch('bookwyrm.views.user.is_api_request') as is_api: @@ -65,7 +66,7 @@ class UserViews(TestCase): is_api.return_value = False result = view(request, 'mouse') self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'user/followers.html') + result.render() self.assertEqual(result.status_code, 200) with patch('bookwyrm.views.user.is_api_request') as is_api: @@ -96,7 +97,7 @@ class UserViews(TestCase): is_api.return_value = False result = view(request, 'mouse') self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'user/following.html') + result.render() self.assertEqual(result.status_code, 200) with patch('bookwyrm.views.user.is_api_request') as is_api: @@ -125,7 +126,7 @@ class UserViews(TestCase): request.user = self.local_user result = view(request) self.assertIsInstance(result, TemplateResponse) - self.assertEqual(result.template_name, 'preferences/edit_user.html') + result.render() self.assertEqual(result.status_code, 200) From 661d49d9cc69915003ed14d3f4f4d4b797d5d4a6 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 30 Jan 2021 17:19:01 -0800 Subject: [PATCH 2/3] Ignore openlibrary editions with little to no metadata Also fixes the isbn problem --- bookwyrm/connectors/openlibrary.py | 31 +++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/bookwyrm/connectors/openlibrary.py b/bookwyrm/connectors/openlibrary.py index 55355131c..cd196d274 100644 --- a/bookwyrm/connectors/openlibrary.py +++ b/bookwyrm/connectors/openlibrary.py @@ -27,9 +27,9 @@ class Connector(AbstractConnector): Mapping('series', formatter=get_first), Mapping('seriesNumber', remote_field='series_number'), Mapping('subjects'), - Mapping('subjectPlaces'), - Mapping('isbn13', formatter=get_first), - Mapping('isbn10', formatter=get_first), + Mapping('subjectPlaces', remote_field='subject_places'), + Mapping('isbn13', remote_field='isbn_13', formatter=get_first), + Mapping('isbn10', remote_field='isbn_10', formatter=get_first), Mapping('lccn', formatter=get_first), Mapping( 'oclcNumber', remote_field='oclc_numbers', @@ -144,9 +144,34 @@ class Connector(AbstractConnector): # we can mass download edition data from OL to avoid repeatedly querying edition_options = self.load_edition_data(work.openlibrary_key) for edition_data in edition_options.get('entries'): + # does this edition have ANY interesting data? + if ignore_edition(edition_data): + continue self.create_edition_from_data(work, edition_data) +def ignore_edition(edition_data): + ''' don't load a million editions that have no metadata ''' + # an isbn, we love to see it + if edition_data.get('isbn_13') or edition_data.get('isbn_10'): + print(edition_data.get('isbn_10')) + return False + # grudgingly, oclc can stay + if edition_data.get('oclc_numbers'): + print(edition_data.get('oclc_numbers')) + return False + # if it has a cover it can stay + if edition_data.get('covers'): + print(edition_data.get('covers')) + return False + # keep non-english editions + if edition_data.get('languages') and \ + 'languages/eng' not in str(edition_data.get('languages')): + print(edition_data.get('languages')) + return False + return True + + def get_description(description_blob): ''' descriptions can be a string or a dict ''' if isinstance(description_blob, dict): From 9833f5a03da6ae2b4a8d298c99506316ccac0fb5 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 30 Jan 2021 17:36:24 -0800 Subject: [PATCH 3/3] Tests creating editions --- .../connectors/test_openlibrary_connector.py | 16 ++++++++++++++++ bookwyrm/tests/data/ol_edition.json | 1 + 2 files changed, 17 insertions(+) diff --git a/bookwyrm/tests/connectors/test_openlibrary_connector.py b/bookwyrm/tests/connectors/test_openlibrary_connector.py index dc4c5f5b1..c277ba045 100644 --- a/bookwyrm/tests/connectors/test_openlibrary_connector.py +++ b/bookwyrm/tests/connectors/test_openlibrary_connector.py @@ -190,3 +190,19 @@ class Openlibrary(TestCase): ''' detect if the loaded json is an edition ''' edition = pick_default_edition(self.edition_list_data['entries']) self.assertEqual(edition['key'], '/books/OL9788823M') + + + def test_create_edition_from_data(self): + ''' okay but can it actually create an edition with proper metadata ''' + work = models.Work.objects.create(title='Hello') + result = self.connector.create_edition_from_data( + work, self.edition_data) + self.assertEqual(result.parent_work, work) + self.assertEqual(result.title, 'Sabriel') + self.assertEqual(result.isbn_10, '0060273224') + self.assertIsNotNone(result.description) + self.assertEqual(result.languages[0], 'English') + self.assertEqual(result.publishers[0], 'Harper Trophy') + self.assertEqual(result.pages, 491) + self.assertEqual(result.subjects[0], 'Fantasy.') + self.assertEqual(result.physical_format, 'Hardcover') diff --git a/bookwyrm/tests/data/ol_edition.json b/bookwyrm/tests/data/ol_edition.json index 459e9dff4..2423364b1 100644 --- a/bookwyrm/tests/data/ol_edition.json +++ b/bookwyrm/tests/data/ol_edition.json @@ -9,6 +9,7 @@ "518848" ] }, + "physical_format": "Hardcover", "lc_classifications": [ "PZ7.N647 Sab 1995" ],