diff --git a/server/notes/migrations/0004_auto_20210322_1514.py b/server/notes/migrations/0004_auto_20210322_1514.py new file mode 100644 index 00000000..f0ccc3e7 --- /dev/null +++ b/server/notes/migrations/0004_auto_20210322_1514.py @@ -0,0 +1,31 @@ +# Generated by Django 2.2.19 on 2021-03-22 15:14 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('notes', '0003_instrumentbookmark'), + ] + + operations = [ + migrations.AlterField( + model_name='contentblockbookmark', + name='uuid', + field=models.UUIDField(), + ), + migrations.AlterField( + model_name='instrumentbookmark', + name='uuid', + field=models.UUIDField(), + ), + migrations.AddConstraint( + model_name='contentblockbookmark', + constraint=models.UniqueConstraint(fields=('uuid', 'content_block', 'user'), name='unique_content_bookmark_per_user'), + ), + migrations.AddConstraint( + model_name='instrumentbookmark', + constraint=models.UniqueConstraint(fields=('uuid', 'instrument', 'user'), name='unique_instrument_bookmark_per_user'), + ), + ] diff --git a/server/notes/models.py b/server/notes/models.py index 1d7794ed..9fc60ce6 100644 --- a/server/notes/models.py +++ b/server/notes/models.py @@ -18,9 +18,14 @@ class Bookmark(models.Model): class ContentBlockBookmark(Bookmark): - uuid = models.UUIDField(unique=True) + uuid = models.UUIDField(unique=False) content_block = models.ForeignKey('books.ContentBlock', on_delete=models.CASCADE) + class Meta: + constraints = [ + models.UniqueConstraint(fields=['uuid', 'content_block', 'user'], name='unique_content_bookmark_per_user') + ] + class ModuleBookmark(Bookmark): module = models.ForeignKey('books.Module', on_delete=models.CASCADE) @@ -30,5 +35,10 @@ class ChapterBookmark(Bookmark): chapter = models.ForeignKey('books.Chapter', on_delete=models.CASCADE) class InstrumentBookmark(Bookmark): - uuid = models.UUIDField(unique=True) + uuid = models.UUIDField(unique=False) instrument = models.ForeignKey('basicknowledge.BasicKnowledge', on_delete=models.CASCADE) + + class Meta: + constraints = [ + models.UniqueConstraint(fields=['uuid', 'instrument', 'user'], name='unique_instrument_bookmark_per_user') + ] diff --git a/server/notes/mutations.py b/server/notes/mutations.py index dd48fe23..f6ec80ef 100644 --- a/server/notes/mutations.py +++ b/server/notes/mutations.py @@ -2,6 +2,8 @@ from builtins import PermissionError import graphene import json + +from django.db import IntegrityError from graphene import relay from graphql_relay import from_global_id @@ -32,11 +34,15 @@ class UpdateContentBookmark(relay.ClientIDMutation): content_block = get_object(ContentBlock, content_block_id) if bookmarked: - ContentBlockBookmark.objects.create( - content_block=content_block, - uuid=uuid, - user=user - ) + try: + ContentBlockBookmark.objects.create( + content_block=content_block, + uuid=uuid, + user=user + ) + except IntegrityError: + # already exists, probably fine + return cls(success=True) else: ContentBlockBookmark.objects.get( content_block=content_block, @@ -200,11 +206,15 @@ class UpdateInstrumentBookmark(relay.ClientIDMutation): instrument = BasicKnowledge.objects.get(slug=instrument_slug) if bookmarked: - InstrumentBookmark.objects.create( - instrument=instrument, - uuid=uuid, - user=user - ) + try: + InstrumentBookmark.objects.create( + instrument=instrument, + uuid=uuid, + user=user + ) + except IntegrityError: + # already exists, that's probably ok + return cls(success=True) else: InstrumentBookmark.objects.get( instrument=instrument, diff --git a/server/notes/tests/__init__.py b/server/notes/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/server/notes/tests/test_duplicate_bookmarks.py b/server/notes/tests/test_duplicate_bookmarks.py new file mode 100644 index 00000000..cad63a8a --- /dev/null +++ b/server/notes/tests/test_duplicate_bookmarks.py @@ -0,0 +1,141 @@ +from django.db import transaction +from django.test import TestCase, RequestFactory +from graphene.test import Client +from graphql_relay import to_global_id + +from api.schema import schema +from books.factories import InstrumentFactory, ContentBlockFactory, ModuleFactory +from notes.models import InstrumentBookmark, ContentBlockBookmark +from users.models import User +from users.services import create_users + +UPDATE_INSTRUMENT_BOOKMARK_MUTATION = """ +mutation UpdateInstrumentBookmark($input: UpdateInstrumentBookmarkInput!) { + updateInstrumentBookmark(input: $input) { + success + __typename + } +} +""" + +UPDATE_CONTENT_BLOCK_BOOKMARK_MUTATION = """ +mutation UpdateContentBookmark($input: UpdateContentBookmarkInput!) { + updateContentBookmark(input: $input) { + success + } +} +""" + + +class DuplicateBookmarksTestCase(TestCase): + def setUp(self): + create_users() + + self.student1 = User.objects.get(username='student1') + self.student2 = User.objects.get(username='student2') + self.instrument = InstrumentFactory(slug='instrument-slug') + module = ModuleFactory() + self.content_block = ContentBlockFactory(contents=[ + { + 'type': 'text_block', + 'value': { + 'text': 'Ein Text' + } + } + ], module=module) + self.uuid = '56e45c9b-e0b2-4103-82e0-ad04ba05ac5b' + self.other_uuid = '56e45c9b-e0b2-4103-82e0-ad04ba05ac5c' + self.content_block_id = to_global_id('ContentBlockNode', self.content_block.id) + + request = RequestFactory().get('/') + request.user = self.student1 + self.client = Client(schema=schema, context_value=request) + + request = RequestFactory().get('/') + request.user = self.student2 + self.client2 = Client(schema=schema, context_value=request) + + def _check_success(self, result, key): + self.assertIsNone(result.get('errors')) + self.assertTrue( + result.get('data').get(key).get('success')) + + def test_two_students_bookmark_the_same_instrument_content(self): + variables = { + "input": { + "bookmarked": True, + "instrument": self.instrument.slug, + "uuid": self.uuid + } + } + + result1 = self.client.execute(UPDATE_INSTRUMENT_BOOKMARK_MUTATION, variables=variables) + self._check_success(result1, 'updateInstrumentBookmark') + self.assertEqual(InstrumentBookmark.objects.count(), 1) + + result2 = self.client2.execute(UPDATE_INSTRUMENT_BOOKMARK_MUTATION, variables=variables) + self._check_success(result2, 'updateInstrumentBookmark') + self.assertEqual(InstrumentBookmark.objects.count(), 2) + + def test_two_students_bookmark_the_same_content_block_content(self): + variables = { + "input": { + "bookmarked": True, + "contentBlock": self.content_block_id, + "uuid": self.other_uuid + } + } + + result1 = self.client.execute(UPDATE_CONTENT_BLOCK_BOOKMARK_MUTATION, variables=variables) + self._check_success(result1, 'updateContentBookmark') + self.assertEqual(ContentBlockBookmark.objects.count(), 1) + + result2 = self.client2.execute(UPDATE_CONTENT_BLOCK_BOOKMARK_MUTATION, variables=variables) + self._check_success(result2, 'updateContentBookmark') + self.assertEqual(ContentBlockBookmark.objects.count(), 2) + + def test_returns_one_instrument_bookmark(self): + # create 2 bookmarks for same user and content + InstrumentBookmark.objects.create( + instrument=self.instrument, + uuid=self.uuid, + user=self.student1 + ) + + variables = { + "input": { + "bookmarked": True, + "instrument": self.instrument.slug, + "uuid": self.uuid + } + } + + # need to do this because of a django test quirk, see: + # https://stackoverflow.com/questions/21458387/transactionmanagementerror-you-cant-execute-queries-until-the-end-of-the-atom + with transaction.atomic(): + result = self.client.execute(UPDATE_INSTRUMENT_BOOKMARK_MUTATION, variables=variables) + self._check_success(result, 'updateInstrumentBookmark') + self.assertEqual(InstrumentBookmark.objects.count(), 1) + + def test_returns_one_content_block_bookmark(self): + # create 2 bookmarks for same user and content + ContentBlockBookmark.objects.create( + content_block=self.content_block, + uuid=self.other_uuid, + user=self.student1 + ) + + variables = { + "input": { + "bookmarked": True, + "contentBlock": self.content_block_id, + "uuid": self.other_uuid + } + } + + # need to do this because of a django test quirk, see: + # https://stackoverflow.com/questions/21458387/transactionmanagementerror-you-cant-execute-queries-until-the-end-of-the-atom + with transaction.atomic(): + result = self.client.execute(UPDATE_CONTENT_BLOCK_BOOKMARK_MUTATION, variables=variables) + self._check_success(result, 'updateContentBookmark') + self.assertEqual(ContentBlockBookmark.objects.count(), 1)