Add bookmark link target setting (#164)

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),
+        ),
+    ]

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

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 %}
             </form>
         </section>

bookmarks/templates/bookmarks/bookmark_list.html

@@ -9,7 +9,7 @@
                 <i class="form-icon"></i>
             </label>
             <div class="title truncate">
-                <a href="{{ bookmark.url }}" target="_blank" rel="noopener">{{ bookmark.resolved_title }}</a>
+                <a href="{{ bookmark.url }}" target="{{ link_target }}" rel="noopener">{{ bookmark.resolved_title }}</a>
             </div>
             <div class="description truncate">
                 {% if bookmark.tag_names %}
@@ -30,7 +30,7 @@
                     <span class="date-label text-gray text-sm">
                         {% if bookmark.web_archive_snapshot_url %}
                         <a href="{{ bookmark.web_archive_snapshot_url }}"
-                           title="Show snapshot on web archive" target="_blank" rel="noopener">
+                           title="Show snapshot on web archive" target="{{ link_target }}" rel="noopener">
                         {% endif %}
                         <span>{{ bookmark.date_added|humanize_relative_date }}</span>
                         {% if bookmark.web_archive_snapshot_url %}
@@ -44,7 +44,7 @@
                     <span class="date-label text-gray text-sm">
                         {% if bookmark.web_archive_snapshot_url %}
                         <a href="{{ bookmark.web_archive_snapshot_url }}"
-                           title="Show snapshot on web archive" target="_blank" rel="noopener">
+                           title="Show snapshot on web archive" target="{{ link_target }}" rel="noopener">
                         {% endif %}
                         <span>{{ bookmark.date_added|humanize_absolute_date }}</span>
                         {% if bookmark.web_archive_snapshot_url %}

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 %}
             </form>
         </section>

bookmarks/templates/settings/general.html

@@ -19,6 +19,10 @@
                     <label for="{{ form.bookmark_date_display.id_for_label }}" class="form-label">Bookmark date format</label>
                     {{ form.bookmark_date_display|add_class:"form-select col-2 col-sm-12" }}
                 </div>
+                <div class="form-group">
+                    <label for="{{ form.bookmark_link_target.id_for_label }}" class="form-label">Open bookmarks in</label>
+                    {{ form.bookmark_link_target|add_class:"form-select col-2 col-sm-12" }}
+                </div>
                 <div class="form-group">
                     <input type="submit" value="Save" class="btn btn-primary mt-2">
                 </div>

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,
     }
 
 

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(
-                '<a href="{0}" target="_blank" rel="noopener">{1}</a>'.format(bookmark.url, bookmark.resolved_title),
+                f'<a href="{bookmark.url}" target="{link_target}" rel="noopener">{bookmark.resolved_title}</a>',
                 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(
-                '<a href="{0}" target="_blank" rel="noopener">{1}</a>'.format(bookmark.url, bookmark.resolved_title),
+                f'<a href="{bookmark.url}" target="{link_target}" rel="noopener">{bookmark.resolved_title}</a>',
                 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')

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(
-                '<a href="{0}" target="_blank" rel="noopener">{1}</a>'.format(bookmark.url, bookmark.resolved_title),
+                f'<a href="{bookmark.url}" target="{link_target}" rel="noopener">{bookmark.resolved_title}</a>',
                 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(
-                '<a href="{0}" target="_blank" rel="noopener">{1}</a>'.format(bookmark.url, bookmark.resolved_title),
+                f'<a href="{bookmark.url}" target="{link_target}" rel="noopener">{bookmark.resolved_title}</a>',
                 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')

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'<a href="{bookmark.url}" target="{link_target}" rel="noopener">{bookmark.resolved_title}</a>',
+            html
+        )
+
+    def assertDateLabel(self, html: str, label_content: str):
+        self.assertInHTML(f'''
+        <span class="date-label text-gray text-sm">
+            <span>{label_content}</span>
+        </span>
+        <span class="text-gray text-sm">|</span>
+        ''', html)
+
+    def assertWebArchiveLink(self, html: str, label_content: str, url: str, link_target: str = '_blank'):
+        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">
+                <span>{label_content}</span>
+                <span>∞</span>
+            </a>
+        </span>
+        <span class="text-gray text-sm">|</span>
+        ''', 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'''
+        self.assertDateLabel(html, formatted_date)
-        <span class="date-label text-gray text-sm">
-            <span>{formatted_date}</span>
-        </span>
-        <span class="text-gray text-sm">|</span>
-        ''', html)
 
     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'''
+        self.assertWebArchiveLink(html, formatted_date, bookmark.web_archive_snapshot_url)
-        <span class="date-label text-gray text-sm">
-            <a href="{bookmark.web_archive_snapshot_url}"
-               title="Show snapshot on web archive" target="_blank" rel="noopener">
-                <span>{formatted_date}</span>
-                <span>∞</span>
-            </a>
-        </span>
-        <span class="text-gray text-sm">|</span>
-        ''', html)
 
     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('''
+        self.assertDateLabel(html, '1 week ago')
-        <span class="date-label text-gray text-sm">
-            <span>1 week ago</span>
-        </span>
-        <span class="text-gray text-sm">|</span>
-        ''', html)
 
     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])
+        html = self.render_default_template([bookmark])
-        self.assertInHTML(f'''
+
-        <span class="date-label text-gray text-sm">
+        self.assertWebArchiveLink(html, '1 week ago', bookmark.web_archive_snapshot_url)
-            <a href="{bookmark.web_archive_snapshot_url}"
+
-               title="Show snapshot on web archive" target="_blank" rel="noopener">
+    def test_bookmark_link_target_should_be_blank_by_default(self):
-                <span>1 week ago</span>
+        bookmark = self.setup_bookmark()
-                <span>∞</span>
+
-            </a>
+        html = self.render_default_template([bookmark])
-        </span>
+
-        <span class="text-gray text-sm">|</span>
+        self.assertBookmarksLink(html, bookmark, link_target='_blank')
-        ''', html)
+
+    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')

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'])

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,
     }