Make Internet Archive integration opt-in (#250)

* Make web archive integration opt-in

* Add toast message about web archive integration opt-in

* Improve wording for web archive setting

* Add toast admin

* Fix toast clear button visited styles

* Add test for redirect

* Improve wording

* Ensure redirects to same domain

* Improve wording

* Fix snapshot test

Co-authored-by: Sascha Ißbrücker <sascha.issbruecker@gmail.com>
This commit is contained in:
Sascha Ißbrücker
2022-05-14 09:46:51 +02:00
committed by GitHub
parent 56173aea3f
commit f92c3dd403
25 changed files with 376 additions and 67 deletions

View File

@@ -28,7 +28,7 @@ class BookmarkListTagTest(TestCase, BookmarkFactoryMixin):
self.assertInHTML(f'''
<span class="date-label text-gray text-sm">
<a href="{url}"
title="Show snapshot on web archive" target="{link_target}" rel="noopener">
title="Show snapshot on the Internet Archive Wayback Machine" target="{link_target}" rel="noopener">
<span>{label_content}</span>
<span>∞</span>
</a>

View File

@@ -23,7 +23,7 @@ class BookmarkServiceTestCase(TestCase, BookmarkFactoryMixin):
bookmark_data = Bookmark(url='https://example.com')
bookmark = create_bookmark(bookmark_data, 'tag1,tag2', self.user)
mock_create_web_archive_snapshot.assert_called_once_with(bookmark.id, False)
mock_create_web_archive_snapshot.assert_called_once_with(self.user, bookmark, False)
def test_update_should_create_web_archive_snapshot_if_url_did_change(self):
with patch.object(tasks, 'create_web_archive_snapshot') as mock_create_web_archive_snapshot:
@@ -31,7 +31,7 @@ class BookmarkServiceTestCase(TestCase, BookmarkFactoryMixin):
bookmark.url = 'https://example.com/updated'
update_bookmark(bookmark, 'tag1,tag2', self.user)
mock_create_web_archive_snapshot.assert_called_once_with(bookmark.id, True)
mock_create_web_archive_snapshot.assert_called_once_with(self.user, bookmark, True)
def test_update_should_not_create_web_archive_snapshot_if_url_did_not_change(self):
with patch.object(tasks, 'create_web_archive_snapshot') as mock_create_web_archive_snapshot:

View File

@@ -5,8 +5,8 @@ from background_task.models import Task
from django.contrib.auth.models import User
from django.test import TestCase, override_settings
from bookmarks.models import Bookmark
from bookmarks.services.tasks import create_web_archive_snapshot, schedule_bookmarks_without_snapshots
from bookmarks.models import Bookmark, UserProfile
from bookmarks.services import tasks
from bookmarks.tests.helpers import BookmarkFactoryMixin, disable_logging
@@ -26,6 +26,11 @@ class MockWaybackUrlWithSaveError:
class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin):
def setUp(self):
user = self.get_or_create_test_user()
user.profile.web_archive_integration = UserProfile.WEB_ARCHIVE_INTEGRATION_ENABLED
user.profile.save()
@disable_logging
def run_pending_task(self, task_function):
func = getattr(task_function, 'task_function', None)
@@ -48,16 +53,16 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin):
bookmark = self.setup_bookmark()
with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://example.com')):
create_web_archive_snapshot(bookmark.id, False)
self.run_pending_task(create_web_archive_snapshot)
tasks.create_web_archive_snapshot(self.get_or_create_test_user(), bookmark, False)
self.run_pending_task(tasks._create_web_archive_snapshot_task)
bookmark.refresh_from_db()
self.assertEqual(bookmark.web_archive_snapshot_url, 'https://example.com')
def test_create_web_archive_snapshot_should_handle_missing_bookmark_id(self):
with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://example.com')) as mock_wayback_url:
create_web_archive_snapshot(123, False)
self.run_pending_task(create_web_archive_snapshot)
tasks._create_web_archive_snapshot_task(123, False)
self.run_pending_task(tasks._create_web_archive_snapshot_task)
mock_wayback_url.assert_not_called()
@@ -67,15 +72,15 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin):
with patch.object(waybackpy, 'Url',
return_value=MockWaybackUrlWithSaveError()):
with self.assertRaises(NotImplementedError):
create_web_archive_snapshot(bookmark.id, False)
self.run_pending_task(create_web_archive_snapshot)
tasks.create_web_archive_snapshot(self.get_or_create_test_user(), bookmark, False)
self.run_pending_task(tasks._create_web_archive_snapshot_task)
def test_create_web_archive_snapshot_should_skip_if_snapshot_exists(self):
bookmark = self.setup_bookmark(web_archive_snapshot_url='https://example.com')
with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://other.com')):
create_web_archive_snapshot(bookmark.id, False)
self.run_pending_task(create_web_archive_snapshot)
tasks.create_web_archive_snapshot(self.get_or_create_test_user(), bookmark, False)
self.run_pending_task(tasks._create_web_archive_snapshot_task)
bookmark.refresh_from_db()
self.assertEqual(bookmark.web_archive_snapshot_url, 'https://example.com')
@@ -84,8 +89,8 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin):
bookmark = self.setup_bookmark(web_archive_snapshot_url='https://example.com')
with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://other.com')):
create_web_archive_snapshot(bookmark.id, True)
self.run_pending_task(create_web_archive_snapshot)
tasks.create_web_archive_snapshot(self.get_or_create_test_user(), bookmark, True)
self.run_pending_task(tasks._create_web_archive_snapshot_task)
bookmark.refresh_from_db()
self.assertEqual(bookmark.web_archive_snapshot_url, 'https://other.com')
@@ -93,7 +98,16 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin):
@override_settings(LD_DISABLE_BACKGROUND_TASKS=True)
def test_create_web_archive_snapshot_should_not_run_when_background_tasks_are_disabled(self):
bookmark = self.setup_bookmark()
create_web_archive_snapshot(bookmark.id, False)
tasks.create_web_archive_snapshot(self.get_or_create_test_user(), bookmark, False)
self.assertEqual(Task.objects.count(), 0)
def test_create_web_archive_snapshot_should_not_run_when_web_archive_integration_is_disabled(self):
self.user.profile.web_archive_integration = UserProfile.WEB_ARCHIVE_INTEGRATION_DISABLED
self.user.profile.save()
bookmark = self.setup_bookmark()
tasks.create_web_archive_snapshot(self.get_or_create_test_user(), bookmark, False)
self.assertEqual(Task.objects.count(), 0)
@@ -104,9 +118,9 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin):
self.setup_bookmark()
with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://example.com')):
schedule_bookmarks_without_snapshots(user.id)
self.run_pending_task(schedule_bookmarks_without_snapshots)
self.run_all_pending_tasks(create_web_archive_snapshot)
tasks.schedule_bookmarks_without_snapshots(user)
self.run_pending_task(tasks._schedule_bookmarks_without_snapshots_task)
self.run_all_pending_tasks(tasks._create_web_archive_snapshot_task)
for bookmark in Bookmark.objects.all():
self.assertEqual(bookmark.web_archive_snapshot_url, 'https://example.com')
@@ -118,9 +132,9 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin):
self.setup_bookmark(web_archive_snapshot_url='https://example.com')
with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://other.com')):
schedule_bookmarks_without_snapshots(user.id)
self.run_pending_task(schedule_bookmarks_without_snapshots)
self.run_all_pending_tasks(create_web_archive_snapshot)
tasks.schedule_bookmarks_without_snapshots(user)
self.run_pending_task(tasks._schedule_bookmarks_without_snapshots_task)
self.run_all_pending_tasks(tasks._create_web_archive_snapshot_task)
for bookmark in Bookmark.objects.all():
self.assertEqual(bookmark.web_archive_snapshot_url, 'https://example.com')
@@ -136,9 +150,9 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin):
self.setup_bookmark(user=other_user)
with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://example.com')):
schedule_bookmarks_without_snapshots(user.id)
self.run_pending_task(schedule_bookmarks_without_snapshots)
self.run_all_pending_tasks(create_web_archive_snapshot)
tasks.schedule_bookmarks_without_snapshots(user)
self.run_pending_task(tasks._schedule_bookmarks_without_snapshots_task)
self.run_all_pending_tasks(tasks._create_web_archive_snapshot_task)
for bookmark in Bookmark.objects.all().filter(owner=user):
self.assertEqual(bookmark.web_archive_snapshot_url, 'https://example.com')
@@ -148,7 +162,13 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin):
@override_settings(LD_DISABLE_BACKGROUND_TASKS=True)
def test_schedule_bookmarks_without_snapshots_should_not_run_when_background_tasks_are_disabled(self):
user = self.get_or_create_test_user()
schedule_bookmarks_without_snapshots(user.id)
tasks.schedule_bookmarks_without_snapshots(self.user)
self.assertEqual(Task.objects.count(), 0)
def test_schedule_bookmarks_without_snapshots_should_not_run_when_web_archive_integration_is_disabled(self):
self.user.profile.web_archive_integration = UserProfile.WEB_ARCHIVE_INTEGRATION_DISABLED
self.user.profile.save()
tasks.schedule_bookmarks_without_snapshots(self.user)
self.assertEqual(Task.objects.count(), 0)

View File

@@ -55,4 +55,4 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin):
with patch.object(tasks, 'schedule_bookmarks_without_snapshots') as mock_schedule_bookmarks_without_snapshots:
import_netscape_html(test_html, user)
mock_schedule_bookmarks_without_snapshots.assert_called_once_with(user.id)
mock_schedule_bookmarks_without_snapshots.assert_called_once_with(user)

View File

@@ -27,6 +27,7 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin):
'theme': UserProfile.THEME_DARK,
'bookmark_date_display': UserProfile.BOOKMARK_DATE_DISPLAY_HIDDEN,
'bookmark_link_target': UserProfile.BOOKMARK_LINK_TARGET_SELF,
'web_archive_integration': UserProfile.WEB_ARCHIVE_INTEGRATION_ENABLED,
}
response = self.client.post(reverse('bookmarks:settings.general'), form_data)
@@ -36,3 +37,4 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin):
self.assertEqual(self.user.profile.theme, form_data['theme'])
self.assertEqual(self.user.profile.bookmark_date_display, form_data['bookmark_date_display'])
self.assertEqual(self.user.profile.bookmark_link_target, form_data['bookmark_link_target'])
self.assertEqual(self.user.profile.web_archive_integration, form_data['web_archive_integration'])

View File

@@ -12,4 +12,4 @@ class SignalsTestCase(TestCase, BookmarkFactoryMixin):
with patch.object(tasks, 'schedule_bookmarks_without_snapshots') as mock_schedule_bookmarks_without_snapshots:
self.client.force_login(user)
mock_schedule_bookmarks_without_snapshots.assert_called_once_with(user.id)
mock_schedule_bookmarks_without_snapshots.assert_called_once_with(user)

View File

@@ -0,0 +1,108 @@
from django.contrib.auth.models import User
from django.test import TestCase
from django.urls import reverse
from bookmarks.models import Toast
from bookmarks.tests.helpers import BookmarkFactoryMixin, random_sentence, disable_logging
class ToastsViewTestCase(TestCase, BookmarkFactoryMixin):
def setUp(self) -> None:
user = self.get_or_create_test_user()
self.client.force_login(user)
def create_toast(self, user: User = None, message: str = None, acknowledged: bool = False):
if not user:
user = self.user
if not message:
message = random_sentence()
toast = Toast(owner=user, key='test', message=message, acknowledged=acknowledged)
toast.save()
return toast
def test_should_render_unacknowledged_toasts(self):
self.create_toast()
self.create_toast()
self.create_toast(acknowledged=True)
response = self.client.get(reverse('bookmarks:index'))
# Should render toasts container
self.assertContains(response, '<div class="toasts container grid-lg">')
# Should render two toasts
self.assertContains(response, '<div class="toast">', count=2)
def test_should_not_render_acknowledged_toasts(self):
self.create_toast(acknowledged=True)
self.create_toast(acknowledged=True)
self.create_toast(acknowledged=True)
response = self.client.get(reverse('bookmarks:index'))
# Should not render toasts container
self.assertContains(response, '<div class="toasts container grid-lg">', count=0)
# Should not render toasts
self.assertContains(response, '<div class="toast">', count=0)
def test_should_not_render_toasts_of_other_users(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
self.create_toast(user=other_user)
self.create_toast(user=other_user)
self.create_toast(user=other_user)
response = self.client.get(reverse('bookmarks:index'))
# Should not render toasts container
self.assertContains(response, '<div class="toasts container grid-lg">', count=0)
# Should not render toasts
self.assertContains(response, '<div class="toast">', count=0)
def test_toast_content(self):
toast = self.create_toast()
expected_toast = f'''
<div class="toast">
{toast.message}
<a href="{reverse('bookmarks:toasts.acknowledge', args=[toast.id])}?return_url={reverse('bookmarks:index')}" class="btn btn-clear float-right"></a>
</div>
'''
response = self.client.get(reverse('bookmarks:index'))
html = response.content.decode()
self.assertInHTML(expected_toast, html)
def test_acknowledge_toast(self):
toast = self.create_toast()
self.client.get(reverse('bookmarks:toasts.acknowledge', args=[toast.id]))
toast.refresh_from_db()
self.assertTrue(toast.acknowledged)
def test_acknowledge_toast_should_redirect_to_return_url(self):
toast = self.create_toast()
return_url = reverse('bookmarks:settings.general')
acknowledge_url = reverse('bookmarks:toasts.acknowledge', args=[toast.id])
acknowledge_url = acknowledge_url + '?return_url=' + return_url
response = self.client.get(acknowledge_url)
self.assertRedirects(response, return_url)
def test_acknowledge_toast_should_redirect_to_index_by_default(self):
toast = self.create_toast()
response = self.client.get(reverse('bookmarks:toasts.acknowledge', args=[toast.id]))
self.assertRedirects(response, reverse('bookmarks:index'))
@disable_logging
def test_acknowledge_toast_should_not_acknowledge_other_users_toast(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
toast = self.create_toast(user=other_user)
response = self.client.get(reverse('bookmarks:toasts.acknowledge', args=[toast.id]))
self.assertEqual(response.status_code, 404)