Make tag search and assignment case insensitive (#56)
* Make tag assignment and search case-insensitive (#45) * Add tests for tag case-sensitivity and deduplication (#45) Co-authored-by: Sascha Ißbrücker <sissbruecker@lyska.io>
This commit is contained in:
parent
f98c89e99d
commit
9df270557f
|
@ -4,6 +4,8 @@ from django import forms
|
||||||
from django.contrib.auth import get_user_model
|
from django.contrib.auth import get_user_model
|
||||||
from django.db import models
|
from django.db import models
|
||||||
|
|
||||||
|
from bookmarks.utils import unique
|
||||||
|
|
||||||
|
|
||||||
class Tag(models.Model):
|
class Tag(models.Model):
|
||||||
name = models.CharField(max_length=64)
|
name = models.CharField(max_length=64)
|
||||||
|
@ -18,7 +20,8 @@ def parse_tag_string(tag_string: str, delimiter: str = ','):
|
||||||
if not tag_string:
|
if not tag_string:
|
||||||
return []
|
return []
|
||||||
names = tag_string.strip().split(delimiter)
|
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)
|
names.sort(key=str.lower)
|
||||||
|
|
||||||
return names
|
return names
|
||||||
|
|
|
@ -2,6 +2,7 @@ from django.contrib.auth.models import User
|
||||||
from django.db.models import Q, Count, Aggregate, CharField, Value, BooleanField
|
from django.db.models import Q, Count, Aggregate, CharField, Value, BooleanField
|
||||||
|
|
||||||
from bookmarks.models import Bookmark, Tag
|
from bookmarks.models import Bookmark, Tag
|
||||||
|
from bookmarks.utils import unique
|
||||||
|
|
||||||
|
|
||||||
class Concat(Aggregate):
|
class Concat(Aggregate):
|
||||||
|
@ -41,7 +42,7 @@ def query_bookmarks(user: User, query_string: str):
|
||||||
|
|
||||||
for tag_name in query['tag_names']:
|
for tag_name in query['tag_names']:
|
||||||
query_set = query_set.filter(
|
query_set = query_set.filter(
|
||||||
tags__name=tag_name
|
tags__name__iexact=tag_name
|
||||||
)
|
)
|
||||||
|
|
||||||
# Sort by modification date
|
# Sort by modification date
|
||||||
|
@ -74,7 +75,7 @@ def query_tags(user: User, query_string: str):
|
||||||
|
|
||||||
for tag_name in query['tag_names']:
|
for tag_name in query['tag_names']:
|
||||||
query_set = query_set.filter(
|
query_set = query_set.filter(
|
||||||
bookmark__tags__name=tag_name
|
bookmark__tags__name__iexact=tag_name
|
||||||
)
|
)
|
||||||
|
|
||||||
return query_set.distinct()
|
return query_set.distinct()
|
||||||
|
@ -95,6 +96,7 @@ def _parse_query_string(query_string):
|
||||||
|
|
||||||
search_terms = [word for word in keywords if word[0] != '#']
|
search_terms = [word for word in keywords if word[0] != '#']
|
||||||
tag_names = [word[1:] 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 {
|
return {
|
||||||
'search_terms': search_terms,
|
'search_terms': search_terms,
|
||||||
|
|
|
@ -1,18 +1,21 @@
|
||||||
|
import operator
|
||||||
from typing import List
|
from typing import List
|
||||||
|
|
||||||
from django.contrib.auth.models import User
|
from django.contrib.auth.models import User
|
||||||
from django.utils import timezone
|
from django.utils import timezone
|
||||||
|
|
||||||
from bookmarks.models import Tag
|
from bookmarks.models import Tag
|
||||||
|
from bookmarks.utils import unique
|
||||||
|
|
||||||
|
|
||||||
def get_or_create_tags(tag_names: List[str], user: User):
|
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):
|
def get_or_create_tag(name: str, user: User):
|
||||||
try:
|
try:
|
||||||
return Tag.objects.get(name=name, owner=user)
|
return Tag.objects.get(name__iexact=name, owner=user)
|
||||||
except Tag.DoesNotExist:
|
except Tag.DoesNotExist:
|
||||||
tag = Tag(name=name, owner=user)
|
tag = Tag(name=name, owner=user)
|
||||||
tag.date_added = timezone.now()
|
tag.date_added = timezone.now()
|
||||||
|
|
|
@ -1,3 +0,0 @@
|
||||||
from django.test import TestCase
|
|
||||||
|
|
||||||
# Create your tests here.
|
|
|
@ -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)
|
||||||
|
|
|
@ -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])
|
|
@ -0,0 +1,2 @@
|
||||||
|
def unique(elements, key):
|
||||||
|
return list({key(element): element for element in elements}.values())
|
Loading…
Reference in New Issue