Add bookmark link target setting (#164)

sissbruecker · Oct 3, 2021 · 4f9fcb4 · 4f9fcb4
1 parent da4a813
commit 4f9fcb4
Show file tree

Hide file tree

Showing 12 changed files with 191 additions and 59 deletions.
diff --git a/bookmarks/migrations/0010_userprofile_bookmark_link_target.py 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
@@ -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
@@ -26,7 +26,7 @@ <h2>Archived bookmarks</h2>
                 {% if empty %}
                     {% include 'bookmarks/empty_bookmarks.html' %}
                 {% else %}
-                    {% bookmark_list bookmarks return_url %}
+                    {% bookmark_list bookmarks return_url link_target %}
                 {% endif %}
             </form>
         </section>

diff --git a/bookmarks/templates/bookmarks/bookmark_list.html b/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 %}

diff --git a/bookmarks/templates/bookmarks/index.html b/bookmarks/templates/bookmarks/index.html
@@ -26,7 +26,7 @@ <h2>Bookmarks</h2>
                 {% if empty %}
                     {% include 'bookmarks/empty_bookmarks.html' %}
                 {% else %}
-                    {% bookmark_list bookmarks return_url %}
+                    {% bookmark_list bookmarks return_url link_target %}
                 {% endif %}
             </form>
         </section>

diff --git a/bookmarks/templates/settings/general.html b/bookmarks/templates/settings/general.html
@@ -19,6 +19,10 @@ <h2>Profile</h2>
                     <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>

diff --git 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
@@ -2,7 +2,7 @@
 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 @@ def setUp(self) -> None:
         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 @@ def test_should_list_tags_for_bookmarks_matching_query(self):
 
         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
@@ -2,7 +2,7 @@
 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 @@ def setUp(self) -> None:
         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 @@ def test_should_list_tags_for_bookmarks_matching_query(self):
 
         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')