diff --git a/bookmarks/migrations/0010_userprofile_bookmark_link_target.py b/bookmarks/migrations/0010_userprofile_bookmark_link_target.py new file mode 100644 index 0000000..90883d1 --- /dev/null +++ b/bookmarks/migrations/0010_userprofile_bookmark_link_target.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.6 on 2021-10-03 06:35 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('bookmarks', '0009_bookmark_web_archive_snapshot_url'), + ] + + operations = [ + migrations.AddField( + model_name='userprofile', + name='bookmark_link_target', + field=models.CharField(choices=[('_blank', 'New page'), ('_self', 'Same page')], default='_blank', max_length=10), + ), + ] diff --git a/bookmarks/models.py b/bookmarks/models.py index e772e80..8527e06 100644 --- a/bookmarks/models.py +++ b/bookmarks/models.py @@ -116,16 +116,24 @@ class UserProfile(models.Model): (BOOKMARK_DATE_DISPLAY_ABSOLUTE, 'Absolute'), (BOOKMARK_DATE_DISPLAY_HIDDEN, 'Hidden'), ] + BOOKMARK_LINK_TARGET_BLANK = '_blank' + BOOKMARK_LINK_TARGET_SELF = '_self' + BOOKMARK_LINK_TARGET_CHOICES = [ + (BOOKMARK_LINK_TARGET_BLANK, 'New page'), + (BOOKMARK_LINK_TARGET_SELF, 'Same page'), + ] user = models.OneToOneField(get_user_model(), related_name='profile', on_delete=models.CASCADE) theme = models.CharField(max_length=10, choices=THEME_CHOICES, blank=False, default=THEME_AUTO) bookmark_date_display = models.CharField(max_length=10, choices=BOOKMARK_DATE_DISPLAY_CHOICES, blank=False, default=BOOKMARK_DATE_DISPLAY_RELATIVE) + bookmark_link_target = models.CharField(max_length=10, choices=BOOKMARK_LINK_TARGET_CHOICES, blank=False, + default=BOOKMARK_LINK_TARGET_BLANK) class UserProfileForm(forms.ModelForm): class Meta: model = UserProfile - fields = ['theme', 'bookmark_date_display'] + fields = ['theme', 'bookmark_date_display', 'bookmark_link_target'] @receiver(post_save, sender=get_user_model()) diff --git a/bookmarks/templates/bookmarks/archive.html b/bookmarks/templates/bookmarks/archive.html index a69dd11..5fadac4 100644 --- a/bookmarks/templates/bookmarks/archive.html +++ b/bookmarks/templates/bookmarks/archive.html @@ -26,7 +26,7 @@ {% if empty %} {% include 'bookmarks/empty_bookmarks.html' %} {% else %} - {% bookmark_list bookmarks return_url %} + {% bookmark_list bookmarks return_url link_target %} {% endif %} diff --git a/bookmarks/templates/bookmarks/bookmark_list.html b/bookmarks/templates/bookmarks/bookmark_list.html index ff83f38..e819a09 100644 --- a/bookmarks/templates/bookmarks/bookmark_list.html +++ b/bookmarks/templates/bookmarks/bookmark_list.html @@ -9,7 +9,7 @@
- {{ bookmark.resolved_title }} + {{ bookmark.resolved_title }}
{% if bookmark.tag_names %} @@ -30,7 +30,7 @@ {% if bookmark.web_archive_snapshot_url %} + title="Show snapshot on web archive" target="{{ link_target }}" rel="noopener"> {% endif %} {{ bookmark.date_added|humanize_relative_date }} {% if bookmark.web_archive_snapshot_url %} @@ -44,7 +44,7 @@ {% if bookmark.web_archive_snapshot_url %} + title="Show snapshot on web archive" target="{{ link_target }}" rel="noopener"> {% endif %} {{ bookmark.date_added|humanize_absolute_date }} {% if bookmark.web_archive_snapshot_url %} diff --git a/bookmarks/templates/bookmarks/index.html b/bookmarks/templates/bookmarks/index.html index dd8d4b0..96877da 100644 --- a/bookmarks/templates/bookmarks/index.html +++ b/bookmarks/templates/bookmarks/index.html @@ -26,7 +26,7 @@ {% if empty %} {% include 'bookmarks/empty_bookmarks.html' %} {% else %} - {% bookmark_list bookmarks return_url %} + {% bookmark_list bookmarks return_url link_target %} {% endif %} diff --git a/bookmarks/templates/settings/general.html b/bookmarks/templates/settings/general.html index 23d4fe3..400eef4 100644 --- a/bookmarks/templates/settings/general.html +++ b/bookmarks/templates/settings/general.html @@ -19,6 +19,10 @@ {{ form.bookmark_date_display|add_class:"form-select col-2 col-sm-12" }}
+
+ + {{ form.bookmark_link_target|add_class:"form-select col-2 col-sm-12" }} +
diff --git a/bookmarks/templatetags/bookmarks.py b/bookmarks/templatetags/bookmarks.py index 859df1c..49624e4 100644 --- a/bookmarks/templatetags/bookmarks.py +++ b/bookmarks/templatetags/bookmarks.py @@ -51,11 +51,12 @@ def tag_cloud(context, tags: List[Tag]): @register.inclusion_tag('bookmarks/bookmark_list.html', name='bookmark_list', takes_context=True) -def bookmark_list(context, bookmarks: Page, return_url: str): +def bookmark_list(context, bookmarks: Page, return_url: str, link_target: str = '_blank'): return { 'request': context['request'], 'bookmarks': bookmarks, - 'return_url': return_url + 'return_url': return_url, + 'link_target': link_target, } diff --git a/bookmarks/tests/test_bookmark_archived_view.py b/bookmarks/tests/test_bookmark_archived_view.py index 8920fac..0bce6f0 100644 --- a/bookmarks/tests/test_bookmark_archived_view.py +++ b/bookmarks/tests/test_bookmark_archived_view.py @@ -2,7 +2,7 @@ from django.contrib.auth.models import User from django.test import TestCase from django.urls import reverse -from bookmarks.models import Bookmark, Tag +from bookmarks.models import Bookmark, Tag, UserProfile from bookmarks.tests.helpers import BookmarkFactoryMixin @@ -12,22 +12,22 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin): user = self.get_or_create_test_user() self.client.force_login(user) - def assertVisibleBookmarks(self, response, bookmarks: [Bookmark]): + def assertVisibleBookmarks(self, response, bookmarks: [Bookmark], link_target: str = '_blank'): html = response.content.decode() self.assertContains(response, 'data-is-bookmark-item', count=len(bookmarks)) for bookmark in bookmarks: self.assertInHTML( - '
{1}'.format(bookmark.url, bookmark.resolved_title), + f'{bookmark.resolved_title}', html ) - def assertInvisibleBookmarks(self, response, bookmarks: [Bookmark]): + def assertInvisibleBookmarks(self, response, bookmarks: [Bookmark], link_target: str = '_blank'): html = response.content.decode() for bookmark in bookmarks: self.assertInHTML( - '{1}'.format(bookmark.url, bookmark.resolved_title), + f'{bookmark.resolved_title}', html, count=0 ) @@ -130,3 +130,29 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin): self.assertVisibleTags(response, visible_tags) self.assertInvisibleTags(response, invisible_tags) + + def test_should_open_bookmarks_in_new_page_by_default(self): + visible_bookmarks = [ + self.setup_bookmark(is_archived=True), + self.setup_bookmark(is_archived=True), + self.setup_bookmark(is_archived=True) + ] + + response = self.client.get(reverse('bookmarks:archived')) + + self.assertVisibleBookmarks(response, visible_bookmarks, '_blank') + + def test_should_open_bookmarks_in_same_page_if_specified_in_user_profile(self): + user = self.get_or_create_test_user() + user.profile.bookmark_link_target = UserProfile.BOOKMARK_LINK_TARGET_SELF + user.profile.save() + + visible_bookmarks = [ + self.setup_bookmark(is_archived=True), + self.setup_bookmark(is_archived=True), + self.setup_bookmark(is_archived=True) + ] + + response = self.client.get(reverse('bookmarks:archived')) + + self.assertVisibleBookmarks(response, visible_bookmarks, '_self') diff --git a/bookmarks/tests/test_bookmark_index_view.py b/bookmarks/tests/test_bookmark_index_view.py index d8a69c6..4e06de0 100644 --- a/bookmarks/tests/test_bookmark_index_view.py +++ b/bookmarks/tests/test_bookmark_index_view.py @@ -2,7 +2,7 @@ from django.contrib.auth.models import User from django.test import TestCase from django.urls import reverse -from bookmarks.models import Bookmark, Tag +from bookmarks.models import Bookmark, Tag, UserProfile from bookmarks.tests.helpers import BookmarkFactoryMixin @@ -12,22 +12,22 @@ class BookmarkIndexViewTestCase(TestCase, BookmarkFactoryMixin): user = self.get_or_create_test_user() self.client.force_login(user) - def assertVisibleBookmarks(self, response, bookmarks: [Bookmark]): + def assertVisibleBookmarks(self, response, bookmarks: [Bookmark], link_target: str = '_blank'): html = response.content.decode() self.assertContains(response, 'data-is-bookmark-item', count=len(bookmarks)) for bookmark in bookmarks: self.assertInHTML( - '{1}'.format(bookmark.url, bookmark.resolved_title), + f'{bookmark.resolved_title}', html ) - def assertInvisibleBookmarks(self, response, bookmarks: [Bookmark]): + def assertInvisibleBookmarks(self, response, bookmarks: [Bookmark], link_target: str = '_blank'): html = response.content.decode() for bookmark in bookmarks: self.assertInHTML( - '{1}'.format(bookmark.url, bookmark.resolved_title), + f'{bookmark.resolved_title}', html, count=0 ) @@ -130,3 +130,29 @@ class BookmarkIndexViewTestCase(TestCase, BookmarkFactoryMixin): self.assertVisibleTags(response, visible_tags) self.assertInvisibleTags(response, invisible_tags) + + def test_should_open_bookmarks_in_new_page_by_default(self): + visible_bookmarks = [ + self.setup_bookmark(), + self.setup_bookmark(), + self.setup_bookmark() + ] + + response = self.client.get(reverse('bookmarks:index')) + + self.assertVisibleBookmarks(response, visible_bookmarks, '_blank') + + def test_should_open_bookmarks_in_same_page_if_specified_in_user_profile(self): + user = self.get_or_create_test_user() + user.profile.bookmark_link_target = UserProfile.BOOKMARK_LINK_TARGET_SELF + user.profile.save() + + visible_bookmarks = [ + self.setup_bookmark(), + self.setup_bookmark(), + self.setup_bookmark() + ] + + response = self.client.get(reverse('bookmarks:index')) + + self.assertVisibleBookmarks(response, visible_bookmarks, '_self') diff --git a/bookmarks/tests/test_bookmarks_list_tag.py b/bookmarks/tests/test_bookmarks_list_tag.py index ef0171e..dff1b11 100644 --- a/bookmarks/tests/test_bookmarks_list_tag.py +++ b/bookmarks/tests/test_bookmarks_list_tag.py @@ -4,13 +4,39 @@ from django.template import Template, RequestContext from django.test import TestCase, RequestFactory from django.utils import timezone, formats -from bookmarks.models import UserProfile +from bookmarks.models import Bookmark, UserProfile from bookmarks.tests.helpers import BookmarkFactoryMixin class BookmarkListTagTest(TestCase, BookmarkFactoryMixin): - def render_template(self, bookmarks) -> str: + def assertBookmarksLink(self, html: str, bookmark: Bookmark, link_target: str = '_blank'): + self.assertInHTML( + f'{bookmark.resolved_title}', + html + ) + + def assertDateLabel(self, html: str, label_content: str): + self.assertInHTML(f''' + + {label_content} + + | + ''', html) + + def assertWebArchiveLink(self, html: str, label_content: str, url: str, link_target: str = '_blank'): + self.assertInHTML(f''' + + + {label_content} + + + + | + ''', html) + + def render_template(self, bookmarks: [Bookmark], template: Template) -> str: rf = RequestFactory() request = rf.get('/test') request.user = self.get_or_create_test_user() @@ -18,11 +44,23 @@ class BookmarkListTagTest(TestCase, BookmarkFactoryMixin): page = paginator.page(1) context = RequestContext(request, {'bookmarks': page, 'return_url': '/test'}) - template_to_render = Template( + return template.render(context) + + def render_default_template(self, bookmarks: [Bookmark]) -> str: + template = Template( '{% load bookmarks %}' '{% bookmark_list bookmarks return_url %}' ) - return template_to_render.render(context) + return self.render_template(bookmarks, template) + + def render_template_with_link_target(self, bookmarks: [Bookmark], link_target: str) -> str: + template = Template( + f''' + {{% load bookmarks %}} + {{% bookmark_list bookmarks return_url '{link_target}' %}} + ''' + ) + return self.render_template(bookmarks, template) def setup_date_format_test(self, date_display_setting: str, web_archive_url: str = ''): bookmark = self.setup_bookmark() @@ -36,55 +74,62 @@ class BookmarkListTagTest(TestCase, BookmarkFactoryMixin): def test_should_respect_absolute_date_setting(self): bookmark = self.setup_date_format_test(UserProfile.BOOKMARK_DATE_DISPLAY_ABSOLUTE) - html = self.render_template([bookmark]) + html = self.render_default_template([bookmark]) formatted_date = formats.date_format(bookmark.date_added, 'SHORT_DATE_FORMAT') - self.assertInHTML(f''' - - {formatted_date} - - | - ''', html) + self.assertDateLabel(html, formatted_date) def test_should_render_web_archive_link_with_absolute_date_setting(self): bookmark = self.setup_date_format_test(UserProfile.BOOKMARK_DATE_DISPLAY_ABSOLUTE, 'https://web.archive.org/web/20210811214511/https://wanikani.com/') - html = self.render_template([bookmark]) + html = self.render_default_template([bookmark]) formatted_date = formats.date_format(bookmark.date_added, 'SHORT_DATE_FORMAT') - self.assertInHTML(f''' - - - {formatted_date} - - - - | - ''', html) + self.assertWebArchiveLink(html, formatted_date, bookmark.web_archive_snapshot_url) def test_should_respect_relative_date_setting(self): bookmark = self.setup_date_format_test(UserProfile.BOOKMARK_DATE_DISPLAY_RELATIVE) - html = self.render_template([bookmark]) + html = self.render_default_template([bookmark]) - self.assertInHTML(''' - - 1 week ago - - | - ''', html) + self.assertDateLabel(html, '1 week ago') def test_should_render_web_archive_link_with_relative_date_setting(self): bookmark = self.setup_date_format_test(UserProfile.BOOKMARK_DATE_DISPLAY_RELATIVE, 'https://web.archive.org/web/20210811214511/https://wanikani.com/') - html = self.render_template([bookmark]) - self.assertInHTML(f''' - - - 1 week ago - - - - | - ''', html) + html = self.render_default_template([bookmark]) + + self.assertWebArchiveLink(html, '1 week ago', bookmark.web_archive_snapshot_url) + + def test_bookmark_link_target_should_be_blank_by_default(self): + bookmark = self.setup_bookmark() + + html = self.render_default_template([bookmark]) + + self.assertBookmarksLink(html, bookmark, link_target='_blank') + + def test_bookmark_link_target_should_respect_link_target_parameter(self): + bookmark = self.setup_bookmark() + + html = self.render_template_with_link_target([bookmark], '_self') + + self.assertBookmarksLink(html, bookmark, link_target='_self') + + def test_web_archive_link_target_should_be_blank_by_default(self): + bookmark = self.setup_bookmark() + bookmark.date_added = timezone.now() - relativedelta(days=8) + bookmark.web_archive_snapshot_url = 'https://example.com' + bookmark.save() + + html = self.render_default_template([bookmark]) + + self.assertWebArchiveLink(html, '1 week ago', bookmark.web_archive_snapshot_url, link_target='_blank') + + def test_web_archive_link_target_respect_link_target_parameter(self): + bookmark = self.setup_bookmark() + bookmark.date_added = timezone.now() - relativedelta(days=8) + bookmark.web_archive_snapshot_url = 'https://example.com' + bookmark.save() + + html = self.render_template_with_link_target([bookmark], '_self') + + self.assertWebArchiveLink(html, '1 week ago', bookmark.web_archive_snapshot_url, link_target='_self') diff --git a/bookmarks/tests/test_settings_general_view.py b/bookmarks/tests/test_settings_general_view.py index 642c0c0..e6d7299 100644 --- a/bookmarks/tests/test_settings_general_view.py +++ b/bookmarks/tests/test_settings_general_view.py @@ -26,6 +26,7 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin): form_data = { 'theme': UserProfile.THEME_DARK, 'bookmark_date_display': UserProfile.BOOKMARK_DATE_DISPLAY_HIDDEN, + 'bookmark_link_target': UserProfile.BOOKMARK_LINK_TARGET_SELF, } response = self.client.post(reverse('bookmarks:settings.general'), form_data) @@ -34,3 +35,4 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin): self.assertEqual(response.status_code, 200) 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']) diff --git a/bookmarks/views/bookmarks.py b/bookmarks/views/bookmarks.py index 78a0a73..ea0cae9 100644 --- a/bookmarks/views/bookmarks.py +++ b/bookmarks/views/bookmarks.py @@ -40,6 +40,7 @@ def get_bookmark_view_context(request, query_set, tags, base_url): paginator = Paginator(query_set, _default_page_size) bookmarks = paginator.get_page(page) return_url = generate_return_url(base_url, page, query_string) + link_target = request.user.profile.bookmark_link_target if request.GET.get('tag'): mod = request.GET.copy() @@ -51,7 +52,8 @@ def get_bookmark_view_context(request, query_set, tags, base_url): 'tags': tags, 'query': query_string if query_string else '', 'empty': paginator.count == 0, - 'return_url': return_url + 'return_url': return_url, + 'link_target': link_target, }