From 9df270557f03d6219c9123b7ffac5c192768cc3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Sat, 2 Jan 2021 11:30:20 +0100 Subject: [PATCH] Make tag search and assignment case insensitive (#56) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Make tag assignment and search case-insensitive (#45) * Add tests for tag case-sensitivity and deduplication (#45) Co-authored-by: Sascha Ißbrücker --- bookmarks/models.py | 5 ++- bookmarks/queries.py | 6 ++- bookmarks/services/tags.py | 7 +++- bookmarks/tests.py | 3 -- bookmarks/tests/__init__.py | 0 bookmarks/tests/test_tags_model.py | 27 +++++++++++++ bookmarks/tests/test_tags_service.py | 60 ++++++++++++++++++++++++++++ bookmarks/utils.py | 2 + 8 files changed, 102 insertions(+), 8 deletions(-) delete mode 100644 bookmarks/tests.py create mode 100644 bookmarks/tests/__init__.py create mode 100644 bookmarks/tests/test_tags_model.py create mode 100644 bookmarks/tests/test_tags_service.py create mode 100644 bookmarks/utils.py diff --git a/bookmarks/models.py b/bookmarks/models.py index 7e00435..613f139 100644 --- a/bookmarks/models.py +++ b/bookmarks/models.py @@ -4,6 +4,8 @@ from django import forms from django.contrib.auth import get_user_model from django.db import models +from bookmarks.utils import unique + class Tag(models.Model): name = models.CharField(max_length=64) @@ -18,7 +20,8 @@ def parse_tag_string(tag_string: str, delimiter: str = ','): if not tag_string: return [] names = tag_string.strip().split(delimiter) - names = [name for name in names if name] + names = [name.strip() for name in names if name] + names = unique(names, str.lower) names.sort(key=str.lower) return names diff --git a/bookmarks/queries.py b/bookmarks/queries.py index 0026086..bcce913 100644 --- a/bookmarks/queries.py +++ b/bookmarks/queries.py @@ -2,6 +2,7 @@ from django.contrib.auth.models import User from django.db.models import Q, Count, Aggregate, CharField, Value, BooleanField from bookmarks.models import Bookmark, Tag +from bookmarks.utils import unique class Concat(Aggregate): @@ -41,7 +42,7 @@ def query_bookmarks(user: User, query_string: str): for tag_name in query['tag_names']: query_set = query_set.filter( - tags__name=tag_name + tags__name__iexact=tag_name ) # Sort by modification date @@ -74,7 +75,7 @@ def query_tags(user: User, query_string: str): for tag_name in query['tag_names']: query_set = query_set.filter( - bookmark__tags__name=tag_name + bookmark__tags__name__iexact=tag_name ) return query_set.distinct() @@ -95,6 +96,7 @@ def _parse_query_string(query_string): search_terms = [word for word in keywords if word[0] != '#'] tag_names = [word[1:] for word in keywords if word[0] == '#'] + tag_names = unique(tag_names, str.lower) return { 'search_terms': search_terms, diff --git a/bookmarks/services/tags.py b/bookmarks/services/tags.py index 1c339e8..3c13f9e 100644 --- a/bookmarks/services/tags.py +++ b/bookmarks/services/tags.py @@ -1,18 +1,21 @@ +import operator from typing import List from django.contrib.auth.models import User from django.utils import timezone from bookmarks.models import Tag +from bookmarks.utils import unique def get_or_create_tags(tag_names: List[str], user: User): - return [get_or_create_tag(tag_name, user) for tag_name in tag_names] + tags = [get_or_create_tag(tag_name, user) for tag_name in tag_names] + return unique(tags, operator.attrgetter('id')) def get_or_create_tag(name: str, user: User): try: - return Tag.objects.get(name=name, owner=user) + return Tag.objects.get(name__iexact=name, owner=user) except Tag.DoesNotExist: tag = Tag(name=name, owner=user) tag.date_added = timezone.now() diff --git a/bookmarks/tests.py b/bookmarks/tests.py deleted file mode 100644 index 7ce503c..0000000 --- a/bookmarks/tests.py +++ /dev/null @@ -1,3 +0,0 @@ -from django.test import TestCase - -# Create your tests here. diff --git a/bookmarks/tests/__init__.py b/bookmarks/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/bookmarks/tests/test_tags_model.py b/bookmarks/tests/test_tags_model.py new file mode 100644 index 0000000..a41e70f --- /dev/null +++ b/bookmarks/tests/test_tags_model.py @@ -0,0 +1,27 @@ +from django.test import TestCase + +from bookmarks.models import parse_tag_string + + +class TagTestCase(TestCase): + + def test_parse_tag_string_returns_list_of_tag_names(self): + self.assertCountEqual(parse_tag_string('book, movie, album'), ['book', 'movie', 'album']) + + def test_parse_tag_string_respects_separator(self): + self.assertCountEqual(parse_tag_string('book movie album', ' '), ['book', 'movie', 'album']) + + def test_parse_tag_string_orders_tag_names_alphabetically(self): + self.assertListEqual(parse_tag_string('book,movie,album'), ['album', 'book', 'movie']) + self.assertListEqual(parse_tag_string('Book,movie,album'), ['album', 'Book', 'movie']) + + def test_parse_tag_string_handles_whitespace(self): + self.assertCountEqual(parse_tag_string('\t book, movie \t, album, \n\r'), ['album', 'book', 'movie']) + + def test_parse_tag_string_handles_invalid_input(self): + self.assertListEqual(parse_tag_string(None), []) + self.assertListEqual(parse_tag_string(''), []) + + def test_parse_tag_string_deduplicates_tag_names(self): + self.assertEqual(len(parse_tag_string('book,book,Book,BOOK')), 1) + diff --git a/bookmarks/tests/test_tags_service.py b/bookmarks/tests/test_tags_service.py new file mode 100644 index 0000000..297385d --- /dev/null +++ b/bookmarks/tests/test_tags_service.py @@ -0,0 +1,60 @@ +import datetime +from django.contrib.auth import get_user_model +from django.test import TestCase +from django.utils import timezone + +from bookmarks.models import Tag +from bookmarks.services.tags import get_or_create_tag, get_or_create_tags + +User = get_user_model() + + +class TagTestCase(TestCase): + + def setUp(self) -> None: + self.user = User.objects.create_user('testuser', 'test@example.com', 'password123') + + def test_get_or_create_tag_should_create_new_tag(self): + get_or_create_tag('Book', self.user) + + tags = Tag.objects.all() + + self.assertEqual(len(tags), 1) + self.assertEqual(tags[0].name, 'Book') + self.assertEqual(tags[0].owner, self.user) + self.assertTrue(abs(tags[0].date_added - timezone.now()) < datetime.timedelta(seconds=10)) + + def test_get_or_create_tag_should_return_existing_tag(self): + first_tag = get_or_create_tag('Book', self.user) + second_tag = get_or_create_tag('Book', self.user) + + tags = Tag.objects.all() + + self.assertEqual(len(tags), 1) + self.assertEqual(first_tag.id, second_tag.id) + + def test_get_or_create_tag_should_ignore_casing_when_looking_for_existing_tag(self): + first_tag = get_or_create_tag('Book', self.user) + second_tag = get_or_create_tag('book', self.user) + + tags = Tag.objects.all() + + self.assertEqual(len(tags), 1) + self.assertEqual(first_tag.id, second_tag.id) + + def test_get_or_create_tags_should_return_tags(self): + books_tag = get_or_create_tag('Book', self.user) + movies_tag = get_or_create_tag('Movie', self.user) + + tags = get_or_create_tags(['book', 'movie'], self.user) + + self.assertEqual(len(tags), 2) + self.assertListEqual(tags, [books_tag, movies_tag]) + + def test_get_or_create_tags_should_deduplicate_tags(self): + books_tag = get_or_create_tag('Book', self.user) + + tags = get_or_create_tags(['book', 'Book', 'BOOK'], self.user) + + self.assertEqual(len(tags), 1) + self.assertListEqual(tags, [books_tag]) diff --git a/bookmarks/utils.py b/bookmarks/utils.py new file mode 100644 index 0000000..ac87766 --- /dev/null +++ b/bookmarks/utils.py @@ -0,0 +1,2 @@ +def unique(elements, key): + return list({key(element): element for element in elements}.values())