Fix unique content uuid bug and duplicate entry bug

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'),
+        ),
+    ]

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')
+        ]

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:
+            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:
+            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,

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)