diff --git a/bookmarks/api/routes.py b/bookmarks/api/routes.py index f394f8a..0351976 100644 --- a/bookmarks/api/routes.py +++ b/bookmarks/api/routes.py @@ -6,7 +6,7 @@ from rest_framework.routers import DefaultRouter from bookmarks import queries from bookmarks.api.serializers import BookmarkSerializer, TagSerializer -from bookmarks.models import Bookmark, Tag +from bookmarks.models import Bookmark, BookmarkFilters, Tag, User from bookmarks.services.bookmarks import archive_bookmark, unarchive_bookmark from bookmarks.services.website_loader import load_website_metadata @@ -42,6 +42,16 @@ class BookmarkViewSet(viewsets.GenericViewSet, data = serializer(page, many=True).data return self.get_paginated_response(data) + @action(methods=['get'], detail=False) + def shared(self, request): + filters = BookmarkFilters(request) + user = User.objects.filter(username=filters.user).first() + query_set = queries.query_shared_bookmarks(user, filters.query) + page = self.paginate_queryset(query_set) + serializer = self.get_serializer_class() + data = serializer(page, many=True).data + return self.get_paginated_response(data) + @action(methods=['post'], detail=True) def archive(self, request, pk): bookmark = self.get_object() diff --git a/bookmarks/api/serializers.py b/bookmarks/api/serializers.py index 4e2be52..29769dc 100644 --- a/bookmarks/api/serializers.py +++ b/bookmarks/api/serializers.py @@ -21,6 +21,7 @@ class BookmarkSerializer(serializers.ModelSerializer): 'website_description', 'is_archived', 'unread', + 'shared', 'tag_names', 'date_added', 'date_modified' @@ -37,6 +38,7 @@ class BookmarkSerializer(serializers.ModelSerializer): description = serializers.CharField(required=False, allow_blank=True, default='') is_archived = serializers.BooleanField(required=False, default=False) unread = serializers.BooleanField(required=False, default=False) + shared = serializers.BooleanField(required=False, default=False) # Override readonly tag_names property to allow passing a list of tag names to create/update tag_names = TagListField(required=False, default=[]) @@ -47,12 +49,13 @@ class BookmarkSerializer(serializers.ModelSerializer): bookmark.description = validated_data['description'] bookmark.is_archived = validated_data['is_archived'] bookmark.unread = validated_data['unread'] + bookmark.shared = validated_data['shared'] tag_string = build_tag_string(validated_data['tag_names']) return create_bookmark(bookmark, tag_string, self.context['user']) def update(self, instance: Bookmark, validated_data): # Update fields if they were provided in the payload - for key in ['url', 'title', 'description', 'unread']: + for key in ['url', 'title', 'description', 'unread', 'shared']: if key in validated_data: setattr(instance, key, validated_data[key]) diff --git a/bookmarks/components/SearchAutoComplete.svelte b/bookmarks/components/SearchAutoComplete.svelte index 6b1e9e9..c04ecc2 100644 --- a/bookmarks/components/SearchAutoComplete.svelte +++ b/bookmarks/components/SearchAutoComplete.svelte @@ -8,8 +8,9 @@ export let placeholder; export let value; export let tags; - export let mode = 'default'; + export let mode = ''; export let apiClient; + export let filters; let isFocus = false; let isOpen = false; @@ -112,9 +113,12 @@ let bookmarks = [] if (value && value.length >= 3) { - const fetchedBookmarks = mode === 'archive' - ? await apiClient.getArchivedBookmarks(value, {limit: 5, offset: 0}) - : await apiClient.getBookmarks(value, {limit: 5, offset: 0}) + const path = mode ? `/${mode}` : '' + const suggestionFilters = { + ...filters, + q: value + } + const fetchedBookmarks = await apiClient.listBookmarks(suggestionFilters, {limit: 5, offset: 0, path}) bookmarks = fetchedBookmarks.map(bookmark => { const fullLabel = bookmark.title || bookmark.website_title || bookmark.url const label = clampText(fullLabel, 60) diff --git a/bookmarks/components/api.js b/bookmarks/components/api.js index 06ed981..31641a5 100644 --- a/bookmarks/components/api.js +++ b/bookmarks/components/api.js @@ -3,18 +3,19 @@ export class ApiClient { this.baseUrl = baseUrl } - getBookmarks(query, options = {limit: 100, offset: 0}) { - const encodedQuery = encodeURIComponent(query) - const url = `${this.baseUrl}bookmarks/?q=${encodedQuery}&limit=${options.limit}&offset=${options.offset}` - - return fetch(url) - .then(response => response.json()) - .then(data => data.results) - } - - getArchivedBookmarks(query, options = {limit: 100, offset: 0}) { - const encodedQuery = encodeURIComponent(query) - const url = `${this.baseUrl}bookmarks/archived/?q=${encodedQuery}&limit=${options.limit}&offset=${options.offset}` + listBookmarks(filters, options = {limit: 100, offset: 0, path: ''}) { + const query = [ + `limit=${options.limit}`, + `offset=${options.offset}`, + ] + Object.keys(filters).forEach(key => { + const value = filters[key] + if (value) { + query.push(`${key}=${encodeURIComponent(value)}`) + } + }) + const queryString = query.join('&') + const url = `${this.baseUrl}bookmarks${options.path}/?${queryString}` return fetch(url) .then(response => response.json()) diff --git a/bookmarks/migrations/0016_bookmark_shared.py b/bookmarks/migrations/0016_bookmark_shared.py new file mode 100644 index 0000000..a780b44 --- /dev/null +++ b/bookmarks/migrations/0016_bookmark_shared.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.14 on 2022-08-02 18:42 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('bookmarks', '0015_feedtoken'), + ] + + operations = [ + migrations.AddField( + model_name='bookmark', + name='shared', + field=models.BooleanField(default=False), + ), + ] diff --git a/bookmarks/migrations/0017_userprofile_enable_sharing.py b/bookmarks/migrations/0017_userprofile_enable_sharing.py new file mode 100644 index 0000000..bc676a2 --- /dev/null +++ b/bookmarks/migrations/0017_userprofile_enable_sharing.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.14 on 2022-08-04 09:08 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('bookmarks', '0016_bookmark_shared'), + ] + + operations = [ + migrations.AddField( + model_name='userprofile', + name='enable_sharing', + field=models.BooleanField(default=False), + ), + ] diff --git a/bookmarks/models.py b/bookmarks/models.py index eb8f8df..4705a1f 100644 --- a/bookmarks/models.py +++ b/bookmarks/models.py @@ -5,6 +5,7 @@ from typing import List from django import forms from django.contrib.auth import get_user_model from django.contrib.auth.models import User +from django.core.handlers.wsgi import WSGIRequest from django.db import models from django.db.models.signals import post_save from django.dispatch import receiver @@ -54,6 +55,7 @@ class Bookmark(models.Model): web_archive_snapshot_url = models.CharField(max_length=2048, blank=True) unread = models.BooleanField(default=False) is_archived = models.BooleanField(default=False) + shared = models.BooleanField(default=False) date_added = models.DateTimeField() date_modified = models.DateTimeField() date_accessed = models.DateTimeField(blank=True, null=True) @@ -100,12 +102,19 @@ class BookmarkForm(forms.ModelForm): description = forms.CharField(required=False, widget=forms.Textarea()) unread = forms.BooleanField(required=False) + shared = forms.BooleanField(required=False) # Hidden field that determines whether to close window/tab after saving the bookmark auto_close = forms.CharField(required=False) class Meta: model = Bookmark - fields = ['url', 'tag_string', 'title', 'description', 'unread', 'auto_close'] + fields = ['url', 'tag_string', 'title', 'description', 'unread', 'shared', 'auto_close'] + + +class BookmarkFilters: + def __init__(self, request: WSGIRequest): + self.query = request.GET.get('q') or '' + self.user = request.GET.get('user') or '' class UserProfile(models.Model): @@ -145,12 +154,13 @@ class UserProfile(models.Model): default=BOOKMARK_LINK_TARGET_BLANK) web_archive_integration = models.CharField(max_length=10, choices=WEB_ARCHIVE_INTEGRATION_CHOICES, blank=False, default=WEB_ARCHIVE_INTEGRATION_DISABLED) + enable_sharing = models.BooleanField(default=False, null=False) class UserProfileForm(forms.ModelForm): class Meta: model = UserProfile - fields = ['theme', 'bookmark_date_display', 'bookmark_link_target', 'web_archive_integration'] + fields = ['theme', 'bookmark_date_display', 'bookmark_link_target', 'web_archive_integration', 'enable_sharing'] @receiver(post_save, sender=get_user_model()) diff --git a/bookmarks/queries.py b/bookmarks/queries.py index eaaa888..2dd8568 100644 --- a/bookmarks/queries.py +++ b/bookmarks/queries.py @@ -1,3 +1,5 @@ +from typing import Optional + from django.contrib.auth.models import User from django.db.models import Q, Count, Aggregate, CharField, Value, BooleanField, QuerySet @@ -27,7 +29,13 @@ def query_archived_bookmarks(user: User, query_string: str) -> QuerySet: .filter(is_archived=True) -def _base_bookmarks_query(user: User, query_string: str) -> QuerySet: +def query_shared_bookmarks(user: Optional[User], query_string: str) -> QuerySet: + return _base_bookmarks_query(user, query_string) \ + .filter(shared=True) \ + .filter(owner__profile__enable_sharing=True) + + +def _base_bookmarks_query(user: Optional[User], query_string: str) -> QuerySet: # Add aggregated tag info to bookmark instances query_set = Bookmark.objects \ .annotate(tag_count=Count('tags'), @@ -35,7 +43,8 @@ def _base_bookmarks_query(user: User, query_string: str) -> QuerySet: tag_projection=Value(True, BooleanField())) # Filter for user - query_set = query_set.filter(owner=user) + if user: + query_set = query_set.filter(owner=user) # Split query into search terms and tags query = _parse_query_string(query_string) @@ -88,6 +97,22 @@ def query_archived_bookmark_tags(user: User, query_string: str) -> QuerySet: return query_set.distinct() +def query_shared_bookmark_tags(user: Optional[User], query_string: str) -> QuerySet: + bookmarks_query = query_shared_bookmarks(user, query_string) + + query_set = Tag.objects.filter(bookmark__in=bookmarks_query) + + return query_set.distinct() + + +def query_shared_bookmark_users(query_string: str) -> QuerySet: + bookmarks_query = query_shared_bookmarks(None, query_string) + + query_set = User.objects.filter(bookmark__in=bookmarks_query) + + return query_set.distinct() + + def get_user_tags(user: User): return Tag.objects.filter(owner=user).all() diff --git a/bookmarks/services/bookmarks.py b/bookmarks/services/bookmarks.py index 1fc842d..30394ce 100644 --- a/bookmarks/services/bookmarks.py +++ b/bookmarks/services/bookmarks.py @@ -117,6 +117,7 @@ def _merge_bookmark_data(from_bookmark: Bookmark, to_bookmark: Bookmark): to_bookmark.title = from_bookmark.title to_bookmark.description = from_bookmark.description to_bookmark.unread = from_bookmark.unread + to_bookmark.shared = from_bookmark.shared def _update_website_metadata(bookmark: Bookmark): diff --git a/bookmarks/templates/bookmarks/archive.html b/bookmarks/templates/bookmarks/archive.html index 0c0c5cd..c0405ce 100644 --- a/bookmarks/templates/bookmarks/archive.html +++ b/bookmarks/templates/bookmarks/archive.html @@ -14,7 +14,7 @@

Archived bookmarks

- {% bookmark_search query tags mode='archive' %} + {% bookmark_search filters tags mode='archived' %} {% include 'bookmarks/bulk_edit/toggle.html' %}
diff --git a/bookmarks/templates/bookmarks/bookmark_list.html b/bookmarks/templates/bookmarks/bookmark_list.html index 4554b93..c32f193 100644 --- a/bookmarks/templates/bookmarks/bookmark_list.html +++ b/bookmarks/templates/bookmarks/bookmark_list.html @@ -54,21 +54,29 @@ | {% endif %} - Edit - {% if bookmark.is_archived %} - + {% if bookmark.owner == request.user %} + {# Bookmark owner actions #} + Edit + {% if bookmark.is_archived %} + + {% else %} + + {% endif %} + + {% if bookmark.unread %} + | + + {% endif %} {% else %} - - {% endif %} - - {% if bookmark.unread %} - | - + {# Shared bookmark actions #} + Shared by + {{ bookmark.owner.username }} + {% endif %} diff --git a/bookmarks/templates/bookmarks/form.html b/bookmarks/templates/bookmarks/form.html index bddbeb6..556141c 100644 --- a/bookmarks/templates/bookmarks/form.html +++ b/bookmarks/templates/bookmarks/form.html @@ -75,6 +75,18 @@ Unread bookmarks can be filtered for, and marked as read after you had a chance to look at them. + {% if request.user.profile.enable_sharing %} +
+ +
+ Share this bookmark with other users. +
+
+ {% endif %}
{% if auto_close %} diff --git a/bookmarks/templates/bookmarks/index.html b/bookmarks/templates/bookmarks/index.html index 80ddde4..487b040 100644 --- a/bookmarks/templates/bookmarks/index.html +++ b/bookmarks/templates/bookmarks/index.html @@ -14,7 +14,7 @@

Bookmarks

- {% bookmark_search query tags %} + {% bookmark_search filters tags %} {% include 'bookmarks/bulk_edit/toggle.html' %}
diff --git a/bookmarks/templates/bookmarks/nav_menu.html b/bookmarks/templates/bookmarks/nav_menu.html index a2af266..b7dfe85 100644 --- a/bookmarks/templates/bookmarks/nav_menu.html +++ b/bookmarks/templates/bookmarks/nav_menu.html @@ -15,6 +15,11 @@
  • Archived
  • + {% if request.user.profile.enable_sharing %} +
  • + Shared +
  • + {% endif %}
  • Unread
  • @@ -47,6 +52,11 @@
  • Archived
  • + {% if request.user.profile.enable_sharing %} +
  • + Shared +
  • + {% endif %}
  • Unread
  • diff --git a/bookmarks/templates/bookmarks/search.html b/bookmarks/templates/bookmarks/search.html index c591cd5..a4b35a2 100644 --- a/bookmarks/templates/bookmarks/search.html +++ b/bookmarks/templates/bookmarks/search.html @@ -3,10 +3,13 @@
    + value="{{ filters.query }}">
    + {% if filters.user %} + + {% endif %}
    @@ -15,6 +18,11 @@ window.addEventListener("load", function() { const currentTagsString = '{{ tags_string }}'; const currentTags = currentTagsString.split(' '); + const uniqueTags = [...new Set(currentTags)] + const filters = { + q: '{{ filters.query }}', + user: '{{ filters.user }}', + } const apiClient = new linkding.ApiClient('{% url 'bookmarks:api-root' %}') const wrapper = document.getElementById('search-input-wrap') const newWrapper = document.createElement('div') @@ -23,10 +31,11 @@ props: { name: 'q', placeholder: 'Search for words or #tags', - value: '{{ query }}', - tags: currentTags, + value: '{{ filters.query }}', + tags: uniqueTags, mode: '{{ mode }}', - apiClient + apiClient, + filters, } }) wrapper.parentElement.replaceChild(newWrapper, wrapper) diff --git a/bookmarks/templates/bookmarks/shared.html b/bookmarks/templates/bookmarks/shared.html new file mode 100644 index 0000000..578a827 --- /dev/null +++ b/bookmarks/templates/bookmarks/shared.html @@ -0,0 +1,48 @@ +{% extends "bookmarks/layout.html" %} +{% load static %} +{% load shared %} +{% load bookmarks %} + +{% block content %} + +
    + + {# Bookmark list #} +
    +
    +

    Shared bookmarks

    +
    + {% bookmark_search filters tags mode='shared' %} +
    + +
    + {% csrf_token %} + + {% if empty %} + {% include 'bookmarks/empty_bookmarks.html' %} + {% else %} + {% bookmark_list bookmarks return_url link_target %} + {% endif %} +
    +
    + + {# Filters #} +
    +
    +

    User

    +
    +
    + {% user_select filters users %} +
    +
    +
    +

    Tags

    +
    + {% tag_cloud tags %} +
    +
    + + + +{% endblock %} diff --git a/bookmarks/templates/bookmarks/user_select.html b/bookmarks/templates/bookmarks/user_select.html new file mode 100644 index 0000000..1d1a46b --- /dev/null +++ b/bookmarks/templates/bookmarks/user_select.html @@ -0,0 +1,29 @@ +
    + {% if filters.query %} + + {% endif %} +
    +
    + + +
    +
    +
    + diff --git a/bookmarks/templates/settings/general.html b/bookmarks/templates/settings/general.html index 06bd358..9728b1c 100644 --- a/bookmarks/templates/settings/general.html +++ b/bookmarks/templates/settings/general.html @@ -47,6 +47,16 @@ case it goes offline or its content is modified. +
    + +
    + Allows to share bookmarks with other users, and to view shared bookmarks. + Disabling this feature will hide all previously shared bookmarks from other users. +
    +
    diff --git a/bookmarks/templatetags/bookmarks.py b/bookmarks/templatetags/bookmarks.py index 49624e4..b5dc3c6 100644 --- a/bookmarks/templatetags/bookmarks.py +++ b/bookmarks/templatetags/bookmarks.py @@ -3,14 +3,16 @@ from typing import List from django import template from django.core.paginator import Page -from bookmarks.models import BookmarkForm, Tag, build_tag_string +from bookmarks.models import BookmarkForm, BookmarkFilters, Tag, build_tag_string, User +from bookmarks.utils import unique register = template.Library() -@register.inclusion_tag('bookmarks/form.html', name='bookmark_form') -def bookmark_form(form: BookmarkForm, cancel_url: str, bookmark_id: int = 0, auto_close: bool = False): +@register.inclusion_tag('bookmarks/form.html', name='bookmark_form', takes_context=True) +def bookmark_form(context, form: BookmarkForm, cancel_url: str, bookmark_id: int = 0, auto_close: bool = False): return { + 'request': context['request'], 'form': form, 'auto_close': auto_close, 'bookmark_id': bookmark_id, @@ -25,7 +27,13 @@ class TagGroup: def create_tag_groups(tags: List[Tag]): - sorted_tags = sorted(tags, key=lambda x: str.lower(x.name)) + # Only display each tag name once, ignoring casing + # This covers cases where the tag cloud contains shared tags with duplicate names + # Also means that the cloud can not make assumptions that it will necessarily contain + # all tags of the current user + unique_tags = unique(tags, key=lambda x: str.lower(x.name)) + # Ensure groups, as well as tags within groups, are ordered alphabetically + sorted_tags = sorted(unique_tags, key=lambda x: str.lower(x.name)) group = None groups = [] @@ -61,11 +69,20 @@ def bookmark_list(context, bookmarks: Page, return_url: str, link_target: str = @register.inclusion_tag('bookmarks/search.html', name='bookmark_search', takes_context=True) -def bookmark_search(context, query: str, tags: [Tag], mode: str = 'default'): +def bookmark_search(context, filters: BookmarkFilters, tags: [Tag], mode: str = ''): tag_names = [tag.name for tag in tags] tags_string = build_tag_string(tag_names, ' ') return { - 'query': query, + 'filters': filters, 'tags_string': tags_string, 'mode': mode, } + + +@register.inclusion_tag('bookmarks/user_select.html', name='user_select', takes_context=True) +def user_select(context, filters: BookmarkFilters, users: List[User]): + sorted_users = sorted(users, key=lambda x: str.lower(x.username)) + return { + 'filters': filters, + 'users': sorted_users, + } diff --git a/bookmarks/templatetags/shared.py b/bookmarks/templatetags/shared.py index 2eb13fd..65aea28 100644 --- a/bookmarks/templatetags/shared.py +++ b/bookmarks/templatetags/shared.py @@ -32,6 +32,18 @@ def append_query_param(context, **kwargs): return query.urlencode() +@register.simple_tag(takes_context=True) +def replace_query_param(context, **kwargs): + query = context.request.GET.copy() + + # Create query param or replace existing + for key in kwargs: + value = kwargs[key] + query.__setitem__(key, value) + + return query.urlencode() + + @register.filter(name='hash_tag') def hash_tag(tag_name): return '#' + tag_name diff --git a/bookmarks/tests/helpers.py b/bookmarks/tests/helpers.py index 2309dc1..058c2fc 100644 --- a/bookmarks/tests/helpers.py +++ b/bookmarks/tests/helpers.py @@ -23,6 +23,7 @@ class BookmarkFactoryMixin: def setup_bookmark(self, is_archived: bool = False, unread: bool = False, + shared: bool = False, tags=None, user: User = None, url: str = '', @@ -52,6 +53,7 @@ class BookmarkFactoryMixin: owner=user, is_archived=is_archived, unread=unread, + shared=shared, web_archive_snapshot_url=web_archive_snapshot_url, ) bookmark.save() @@ -69,6 +71,14 @@ class BookmarkFactoryMixin: tag.save() return tag + def setup_user(self, name: str = None, enable_sharing: bool = False): + if not name: + name = get_random_string(length=32) + user = User.objects.create_user(name, 'user@example.com', 'password123') + user.profile.enable_sharing = enable_sharing + user.profile.save() + return user + class LinkdingApiTestCase(APITestCase): def get(self, url, expected_status_code=status.HTTP_200_OK): diff --git a/bookmarks/tests/test_bookmark_archived_view_performance.py b/bookmarks/tests/test_bookmark_archived_view_performance.py new file mode 100644 index 0000000..72159ac --- /dev/null +++ b/bookmarks/tests/test_bookmark_archived_view_performance.py @@ -0,0 +1,42 @@ +from django.contrib.auth.models import User +from django.test import TransactionTestCase +from django.test.utils import CaptureQueriesContext +from django.urls import reverse +from django.db import connections +from django.db.utils import DEFAULT_DB_ALIAS + +from bookmarks.tests.helpers import BookmarkFactoryMixin + + +class BookmarkArchivedViewPerformanceTestCase(TransactionTestCase, BookmarkFactoryMixin): + + def setUp(self) -> None: + user = self.get_or_create_test_user() + self.client.force_login(user) + + def get_connection(self): + return connections[DEFAULT_DB_ALIAS] + + def test_should_not_increase_number_of_queries_per_bookmark(self): + # create initial bookmarks + num_initial_bookmarks = 10 + for index in range(num_initial_bookmarks): + self.setup_bookmark(user=self.user, is_archived=True) + + # capture number of queries + context = CaptureQueriesContext(self.get_connection()) + with context: + response = self.client.get(reverse('bookmarks:archived')) + self.assertContains(response, 'data-is-bookmark-item', num_initial_bookmarks) + + number_of_queries = context.final_queries + + # add more bookmarks + num_additional_bookmarks = 10 + for index in range(num_additional_bookmarks): + self.setup_bookmark(user=self.user, is_archived=True) + + # assert num queries doesn't increase + with self.assertNumQueries(number_of_queries): + response = self.client.get(reverse('bookmarks:archived')) + self.assertContains(response, 'data-is-bookmark-item', num_initial_bookmarks + num_additional_bookmarks) diff --git a/bookmarks/tests/test_bookmark_edit_view.py b/bookmarks/tests/test_bookmark_edit_view.py index f137895..4a44037 100644 --- a/bookmarks/tests/test_bookmark_edit_view.py +++ b/bookmarks/tests/test_bookmark_edit_view.py @@ -21,6 +21,7 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin): 'title': 'edited title', 'description': 'edited description', 'unread': False, + 'shared': False, } return {**form_data, **overrides} @@ -37,20 +38,37 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin): self.assertEqual(bookmark.title, form_data['title']) self.assertEqual(bookmark.description, form_data['description']) self.assertEqual(bookmark.unread, form_data['unread']) + self.assertEqual(bookmark.shared, form_data['shared']) self.assertEqual(bookmark.tags.count(), 2) self.assertEqual(bookmark.tags.all()[0].name, 'editedtag1') self.assertEqual(bookmark.tags.all()[1].name, 'editedtag2') - def test_should_mark_bookmark_as_unread(self): + def test_should_edit_unread_state(self): bookmark = self.setup_bookmark() + form_data = self.create_form_data({'id': bookmark.id, 'unread': True}) - self.client.post(reverse('bookmarks:edit', args=[bookmark.id]), form_data) - bookmark.refresh_from_db() - self.assertTrue(bookmark.unread) + form_data = self.create_form_data({'id': bookmark.id, 'unread': False}) + self.client.post(reverse('bookmarks:edit', args=[bookmark.id]), form_data) + bookmark.refresh_from_db() + self.assertFalse(bookmark.unread) + + def test_should_edit_shared_state(self): + bookmark = self.setup_bookmark() + + form_data = self.create_form_data({'id': bookmark.id, 'shared': True}) + self.client.post(reverse('bookmarks:edit', args=[bookmark.id]), form_data) + bookmark.refresh_from_db() + self.assertTrue(bookmark.shared) + + form_data = self.create_form_data({'id': bookmark.id, 'shared': False}) + self.client.post(reverse('bookmarks:edit', args=[bookmark.id]), form_data) + bookmark.refresh_from_db() + self.assertFalse(bookmark.shared) + def test_should_prefill_bookmark_form_fields(self): tag1 = self.setup_tag() tag2 = self.setup_tag() @@ -126,3 +144,32 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin): self.assertNotEqual(bookmark.url, form_data['url']) self.assertEqual(response.status_code, 404) + def test_should_respect_share_profile_setting(self): + bookmark = self.setup_bookmark() + + self.user.profile.enable_sharing = False + self.user.profile.save() + response = self.client.get(reverse('bookmarks:edit', args=[bookmark.id])) + html = response.content.decode() + + self.assertInHTML(''' + + ''', html, count=0) + + self.user.profile.enable_sharing = True + self.user.profile.save() + response = self.client.get(reverse('bookmarks:edit', args=[bookmark.id])) + html = response.content.decode() + + self.assertInHTML(''' + + ''', html, count=1) + diff --git a/bookmarks/tests/test_bookmark_index_view.py b/bookmarks/tests/test_bookmark_index_view.py index bfce3b6..b07bf4f 100644 --- a/bookmarks/tests/test_bookmark_index_view.py +++ b/bookmarks/tests/test_bookmark_index_view.py @@ -1,3 +1,5 @@ +import urllib.parse + from django.contrib.auth.models import User from django.test import TestCase from django.urls import reverse @@ -156,3 +158,30 @@ class BookmarkIndexViewTestCase(TestCase, BookmarkFactoryMixin): response = self.client.get(reverse('bookmarks:index')) self.assertVisibleBookmarks(response, visible_bookmarks, '_self') + + def test_edit_link_return_url_should_contain_query_params(self): + bookmark = self.setup_bookmark(title='foo') + + # without query params + url = reverse('bookmarks:index') + response = self.client.get(url) + html = response.content.decode() + edit_url = reverse('bookmarks:edit', args=[bookmark.id]) + return_url = urllib.parse.quote_plus(url) + + self.assertInHTML(f''' + Edit + ''', html) + + # with query params + url = reverse('bookmarks:index') + '?q=foo&user=user' + response = self.client.get(url) + html = response.content.decode() + edit_url = reverse('bookmarks:edit', args=[bookmark.id]) + return_url = urllib.parse.quote_plus(url) + + self.assertInHTML(f''' + Edit + ''', html) diff --git a/bookmarks/tests/test_bookmark_index_view_performance.py b/bookmarks/tests/test_bookmark_index_view_performance.py new file mode 100644 index 0000000..f13f532 --- /dev/null +++ b/bookmarks/tests/test_bookmark_index_view_performance.py @@ -0,0 +1,42 @@ +from django.contrib.auth.models import User +from django.test import TransactionTestCase +from django.test.utils import CaptureQueriesContext +from django.urls import reverse +from django.db import connections +from django.db.utils import DEFAULT_DB_ALIAS + +from bookmarks.tests.helpers import BookmarkFactoryMixin + + +class BookmarkIndexViewPerformanceTestCase(TransactionTestCase, BookmarkFactoryMixin): + + def setUp(self) -> None: + user = self.get_or_create_test_user() + self.client.force_login(user) + + def get_connection(self): + return connections[DEFAULT_DB_ALIAS] + + def test_should_not_increase_number_of_queries_per_bookmark(self): + # create initial bookmarks + num_initial_bookmarks = 10 + for index in range(num_initial_bookmarks): + self.setup_bookmark(user=self.user) + + # capture number of queries + context = CaptureQueriesContext(self.get_connection()) + with context: + response = self.client.get(reverse('bookmarks:index')) + self.assertContains(response, 'data-is-bookmark-item', num_initial_bookmarks) + + number_of_queries = context.final_queries + + # add more bookmarks + num_additional_bookmarks = 10 + for index in range(num_additional_bookmarks): + self.setup_bookmark(user=self.user) + + # assert num queries doesn't increase + with self.assertNumQueries(number_of_queries): + response = self.client.get(reverse('bookmarks:index')) + self.assertContains(response, 'data-is-bookmark-item', num_initial_bookmarks + num_additional_bookmarks) diff --git a/bookmarks/tests/test_bookmark_new_view.py b/bookmarks/tests/test_bookmark_new_view.py index ba2ce78..86b2596 100644 --- a/bookmarks/tests/test_bookmark_new_view.py +++ b/bookmarks/tests/test_bookmark_new_view.py @@ -20,6 +20,7 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin): 'title': 'test title', 'description': 'test description', 'unread': False, + 'shared': False, 'auto_close': '', } return {**form_data, **overrides} @@ -37,6 +38,7 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin): self.assertEqual(bookmark.title, form_data['title']) self.assertEqual(bookmark.description, form_data['description']) self.assertEqual(bookmark.unread, form_data['unread']) + self.assertEqual(bookmark.shared, form_data['shared']) self.assertEqual(bookmark.tags.count(), 2) self.assertEqual(bookmark.tags.all()[0].name, 'tag1') self.assertEqual(bookmark.tags.all()[1].name, 'tag2') @@ -51,6 +53,16 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin): bookmark = Bookmark.objects.first() self.assertTrue(bookmark.unread) + def test_should_create_new_shared_bookmark(self): + form_data = self.create_form_data({'shared': True}) + + self.client.post(reverse('bookmarks:new'), form_data) + + self.assertEqual(Bookmark.objects.count(), 1) + + bookmark = Bookmark.objects.first() + self.assertTrue(bookmark.shared) + def test_should_prefill_url_from_url_parameter(self): response = self.client.get(reverse('bookmarks:new') + '?url=http://example.com') html = response.content.decode() @@ -98,3 +110,30 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin): response = self.client.post(reverse('bookmarks:new'), form_data) self.assertRedirects(response, reverse('bookmarks:close')) + + def test_should_respect_share_profile_setting(self): + self.user.profile.enable_sharing = False + self.user.profile.save() + response = self.client.get(reverse('bookmarks:new')) + html = response.content.decode() + + self.assertInHTML(''' + + ''', html, count=0) + + self.user.profile.enable_sharing = True + self.user.profile.save() + response = self.client.get(reverse('bookmarks:new')) + html = response.content.decode() + + self.assertInHTML(''' + + ''', html, count=1) diff --git a/bookmarks/tests/test_bookmark_search_tag.py b/bookmarks/tests/test_bookmark_search_tag.py new file mode 100644 index 0000000..d63784b --- /dev/null +++ b/bookmarks/tests/test_bookmark_search_tag.py @@ -0,0 +1,40 @@ +from django.db.models import QuerySet +from django.template import Template, RequestContext +from django.test import TestCase, RequestFactory + +from bookmarks.models import BookmarkFilters, Tag +from bookmarks.tests.helpers import BookmarkFactoryMixin + + +class BookmarkSearchTagTest(TestCase, BookmarkFactoryMixin): + def render_template(self, url: str, tags: QuerySet[Tag] = Tag.objects.all()): + rf = RequestFactory() + request = rf.get(url) + filters = BookmarkFilters(request) + context = RequestContext(request, { + 'request': request, + 'filters': filters, + 'tags': tags, + }) + template_to_render = Template( + '{% load bookmarks %}' + '{% bookmark_search filters tags %}' + ) + return template_to_render.render(context) + + def test_render_hidden_inputs_for_filter_params(self): + # Should render hidden inputs if query param exists + url = '/test?q=foo&user=john' + rendered_template = self.render_template(url) + + self.assertInHTML(''' + + ''', rendered_template) + + # Should not render hidden inputs if query param does not exist + url = '/test?q=foo' + rendered_template = self.render_template(url) + + self.assertInHTML(''' + + ''', rendered_template, count=0) diff --git a/bookmarks/tests/test_bookmark_shared_view.py b/bookmarks/tests/test_bookmark_shared_view.py new file mode 100644 index 0000000..7881ec3 --- /dev/null +++ b/bookmarks/tests/test_bookmark_shared_view.py @@ -0,0 +1,255 @@ +from typing import List + +from django.contrib.auth.models import User +from django.test import TestCase +from django.urls import reverse + +from bookmarks.models import Bookmark, Tag, UserProfile +from bookmarks.tests.helpers import BookmarkFactoryMixin + + +class BookmarkSharedViewTestCase(TestCase, BookmarkFactoryMixin): + + def setUp(self) -> None: + user = self.get_or_create_test_user() + self.client.force_login(user) + + def assertBookmarkCount(self, html: str, bookmark: Bookmark, count: int, link_target: str = '_blank'): + self.assertInHTML( + f'{bookmark.resolved_title}', + html, count=count + ) + + def assertVisibleBookmarks(self, response, bookmarks: List[Bookmark], link_target: str = '_blank'): + html = response.content.decode() + self.assertContains(response, 'data-is-bookmark-item', count=len(bookmarks)) + + for bookmark in bookmarks: + self.assertBookmarkCount(html, bookmark, 1, link_target) + + def assertInvisibleBookmarks(self, response, bookmarks: List[Bookmark], link_target: str = '_blank'): + html = response.content.decode() + + for bookmark in bookmarks: + self.assertBookmarkCount(html, bookmark, 0, link_target) + + def assertVisibleTags(self, response, tags: [Tag]): + self.assertContains(response, 'data-is-tag-item', count=len(tags)) + + for tag in tags: + self.assertContains(response, tag.name) + + def assertInvisibleTags(self, response, tags: [Tag]): + for tag in tags: + self.assertNotContains(response, tag.name) + + def assertVisibleUserOptions(self, response, users: List[User]): + html = response.content.decode() + self.assertContains(response, 'data-is-user-option', count=len(users)) + + for user in users: + self.assertInHTML(f''' + + ''', html) + + def assertInvisibleUserOptions(self, response, users: List[User]): + html = response.content.decode() + + for user in users: + self.assertInHTML(f''' + + ''', html, count=0) + + def test_should_list_shared_bookmarks_from_all_users_that_have_sharing_enabled(self): + user1 = self.setup_user(enable_sharing=True) + user2 = self.setup_user(enable_sharing=True) + user3 = self.setup_user(enable_sharing=True) + user4 = self.setup_user(enable_sharing=False) + + visible_bookmarks = [ + self.setup_bookmark(shared=True, user=user1), + self.setup_bookmark(shared=True, user=user2), + self.setup_bookmark(shared=True, user=user3), + ] + invisible_bookmarks = [ + self.setup_bookmark(shared=False, user=user1), + self.setup_bookmark(shared=False, user=user2), + self.setup_bookmark(shared=False, user=user3), + self.setup_bookmark(shared=True, user=user4), + ] + + response = self.client.get(reverse('bookmarks:shared')) + + self.assertContains(response, '