Better error handling in change event handlers

contentcuration/contentcuration/migrations/0124_auto_20201204_1645.py

@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.29 on 2020-12-04 16:45
+from __future__ import unicode_literals
+
+import django.utils.timezone
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("contentcuration", "0123_auto_20200921_1536"),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name="contentnode",
+ name="created",
+ field=models.DateTimeField(
+ default=django.utils.timezone.now, verbose_name="created"
+ ),
+ ),
+ ]

contentcuration/contentcuration/models.py

@@ -1127,7 +1127,7 @@ class ContentNode(MPTTModel, models.Model):
 
  thumbnail_encoding = models.TextField(blank=True, null=True)
 
- created = models.DateTimeField(auto_now_add=True, verbose_name="created")
+ created = models.DateTimeField(default=timezone.now, verbose_name="created")
  modified = models.DateTimeField(auto_now=True, verbose_name="modified")
  published = models.BooleanField(default=False)
  publishing = models.BooleanField(default=False)

contentcuration/contentcuration/tests/test_contentnodes.py

@@ -9,6 +9,7 @@
 from builtins import zip
 
 import pytest
+from django.db import IntegrityError
 from django.db.utils import DataError
 from le_utils.constants import content_kinds
 from mixer.backend.django import mixer
@@ -273,13 +274,17 @@ def test_duplicate_nodes_position_right(self):
 
  # simulate a clean, right-after-publish state to ensure only new channel is marked as change
  self.channel.main_tree.changed = False
- self.channel.main_tree.title = 'Some other name'
+ self.channel.main_tree.title = "Some other name"
  self.channel.main_tree.save()
  self.channel.main_tree.refresh_from_db()
 
- self.channel.main_tree.copy_to(new_channel.main_tree.get_children()[0], position="right", batch_size=1000)
+ self.channel.main_tree.copy_to(
+ new_channel.main_tree.get_children()[0], position="right", batch_size=1000
+ )
 
- self.assertEqual(new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title)
+ self.assertEqual(
+ new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title
+ )
 
  def test_duplicate_nodes_mixed(self):
  """
@@ -811,3 +816,11 @@ def test_content_tag_creation(self):
  ContentTag,
  tag_name=random.sample(string.printable, random.randint(51, 80)),
  )
+
+ def test_create_node_that_already_exists_no_integrity_error(self):
+ obj = ContentNode.objects.first()
+ new_obj = ContentNode(id=obj.id, kind_id=content_kinds.TOPIC)
+ try:
+ new_obj.save()
+ except IntegrityError:
+ self.fail("Caused an IntegrityError")

contentcuration/contentcuration/tests/viewsets/test_contentnode.py

@@ -725,6 +725,23 @@ def test_cannot_update_some_contentnodes(self):
  models.ContentNode.objects.get(id=contentnode2.id).title, new_title
  )
 
+ def test_update_contentnode_updates_last_modified(self):
+ user = testdata.user()
+ contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)
+
+ last_modified = contentnode.modified
+ new_title = "This is not the old title"
+
+ self.client.force_authenticate(user=user)
+ response = self.client.post(
+ self.sync_url,
+ [generate_update_event(contentnode.id, CONTENTNODE, {"title": new_title})],
+ format="json",
+ )
+ self.assertEqual(response.status_code, 200, response.content)
+ updated_node = models.ContentNode.objects.get(id=contentnode.id)
+ self.assertNotEqual(last_modified, updated_node.modified)
+
  def test_delete_contentnode(self):
  user = testdata.user()
  contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)

contentcuration/contentcuration/viewsets/base.py

@@ -21,6 +21,7 @@
 from rest_framework.viewsets import ReadOnlyModelViewSet
 
 from contentcuration.viewsets.common import MissingRequiredParamsException
+from contentcuration.viewsets.sync.utils import log_sync_exception
 
 
 class SimpleReprMixin(object):
@@ -554,13 +555,18 @@ def create_from_changes(self, changes):
  changes_to_return = []
 
  for change in changes:
- serializer = self.get_serializer(data=self._map_create_change(change))
- if serializer.is_valid():
- self.perform_create(serializer)
- if serializer.changes:
- changes_to_return.extend(serializer.changes)
- else:
- change.update({"errors": serializer.errors})
+ try:
+ serializer = self.get_serializer(data=self._map_create_change(change))
+ if serializer.is_valid():
+ self.perform_create(serializer)
+ if serializer.changes:
+ changes_to_return.extend(serializer.changes)
+ else:
+ change.update({"errors": serializer.errors})
+ errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
  errors.append(change)
 
  return errors, changes_to_return
@@ -602,6 +608,10 @@ def delete_from_changes(self, changes):
  # If the object already doesn't exist, as far as the user is concerned
  # job done!
  pass
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
 
@@ -637,6 +647,10 @@ def update_from_changes(self, changes):
  # error if the user can view the object but not edit it?
  change.update({"errors": ValidationError("Not found").detail})
  errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
  def update(self, request, *args, **kwargs):

contentcuration/contentcuration/viewsets/contentnode.py

@@ -9,6 +9,7 @@
 from django.db.models import Q
 from django.db.models import Subquery
 from django.http import Http404
+from django.utils.timezone import now
 from django_filters.rest_framework import CharFilter
 from django_filters.rest_framework import DjangoFilterBackend
 from django_filters.rest_framework import UUIDFilter
@@ -101,6 +102,18 @@ def filter__node_id_channel_id(self, queryset, name, value):
  return queryset.filter(query)
 
 
+def bulk_create_tag_relations(tags_relations_to_create):
+ if tags_relations_to_create:
+ # In Django 2.2 add ignore_conflicts to make this fool proof
+ try:
+ ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ except IntegrityError:
+ # One of the relations already exists, so just save them one by one.
+ # Django's default upsert behaviour should mean we get no errors this way
+ for to_create in tags_relations_to_create:
+ to_create.save()
+
+
 def set_tags(tags_by_id):
  all_tag_names = set()
  tags_relations_to_create = []
@@ -142,8 +155,7 @@ def set_tags(tags_by_id):
  tags_relations_to_delete.append(
  Q(contentnode_id=target_node_id, contenttag__tag_name=tag_name)
  )
- if tags_relations_to_create:
- ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ bulk_create_tag_relations(tags_relations_to_create)
  if tags_relations_to_delete:
  ContentNode.tags.through.objects.filter(
  reduce(lambda x, y: x | y, tags_relations_to_delete)
@@ -166,6 +178,9 @@ def gather_tags(self, validated_data):
 
  def update(self, queryset, all_validated_data):
  tags = self.gather_tags(all_validated_data)
+ modified = now()
+ for data in all_validated_data:
+ data["modified"] = modified
  all_objects = super(ContentNodeListSerializer, self).update(
  queryset, all_validated_data
  )

contentcuration/contentcuration/viewsets/sync/endpoint.py

@@ -3,12 +3,10 @@
 and deals with processing all the changes to make appropriate
 bulk creates, updates, and deletes.
 """
-import logging
 import time
 from collections import OrderedDict
 from itertools import groupby
 
-from django.conf import settings
 from rest_framework.authentication import SessionAuthentication
 from rest_framework.authentication import TokenAuthentication
 from rest_framework.decorators import api_view
@@ -48,6 +46,7 @@
 from contentcuration.viewsets.sync.constants import USER
 from contentcuration.viewsets.sync.constants import VIEWER_M2M
 from contentcuration.viewsets.sync.utils import get_and_clear_user_events
+from contentcuration.viewsets.sync.utils import log_sync_exception
 from contentcuration.viewsets.task import TaskViewSet
 from contentcuration.viewsets.user import ChannelUserViewSet
 from contentcuration.viewsets.user import UserViewSet
@@ -186,15 +185,7 @@ def handle_changes(request, viewset_class, change_type, changes):
  report_exception(e)
  return result
  except Exception as e:
- # Capture exception and report, but allow sync
- # to complete properly.
- report_exception(e)
-
- if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
- raise
- else:
- # make sure we leave a record in the logs just in case.
- logging.error(e)
+ log_sync_exception(e)
  for change in changes:
  change["errors"] = [str(e)]
  return changes, None

contentcuration/contentcuration/viewsets/sync/utils.py

@@ -1,5 +1,9 @@
+import logging
+
+from django.conf import settings
 from django.core.cache import cache
 
+from contentcuration.utils.sentry import report_exception
 from contentcuration.viewsets.sync.constants import ALL_TABLES
 from contentcuration.viewsets.sync.constants import COPIED
 from contentcuration.viewsets.sync.constants import CREATED
@@ -82,3 +86,15 @@ def get_and_clear_user_events(user_id):
  user_events = cache.get(cache_key) or []
  cache.delete(cache_key)
  return user_events
+
+
+def log_sync_exception(e):
+ # Capture exception and report, but allow sync
+ # to complete properly.
+ report_exception(e)
+
+ if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
+ raise
+ else:
+ # make sure we leave a record in the logs just in case.
+ logging.error(e)

contentcuration/contentcuration/migrations/0124_auto_20201204_1645.py

@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.29 on 2020-12-04 16:45
+from __future__ import unicode_literals
+
+import django.utils.timezone
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("contentcuration", "0123_auto_20200921_1536"),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name="contentnode",
+ name="created",
+ field=models.DateTimeField(
+ default=django.utils.timezone.now, verbose_name="created"
+ ),
+ ),
+ ]

contentcuration/contentcuration/models.py

@@ -1127,7 +1127,7 @@ class ContentNode(MPTTModel, models.Model):
 
  thumbnail_encoding = models.TextField(blank=True, null=True)
 
- created = models.DateTimeField(auto_now_add=True, verbose_name="created")
+ created = models.DateTimeField(default=timezone.now, verbose_name="created")
  modified = models.DateTimeField(auto_now=True, verbose_name="modified")
  published = models.BooleanField(default=False)
  publishing = models.BooleanField(default=False)

contentcuration/contentcuration/tests/test_contentnodes.py

@@ -9,6 +9,7 @@
 from builtins import zip
 
 import pytest
+from django.db import IntegrityError
 from django.db.utils import DataError
 from le_utils.constants import content_kinds
 from mixer.backend.django import mixer
@@ -273,13 +274,17 @@ def test_duplicate_nodes_position_right(self):
 
  # simulate a clean, right-after-publish state to ensure only new channel is marked as change
  self.channel.main_tree.changed = False
- self.channel.main_tree.title = 'Some other name'
+ self.channel.main_tree.title = "Some other name"
  self.channel.main_tree.save()
  self.channel.main_tree.refresh_from_db()
 
- self.channel.main_tree.copy_to(new_channel.main_tree.get_children()[0], position="right", batch_size=1000)
+ self.channel.main_tree.copy_to(
+ new_channel.main_tree.get_children()[0], position="right", batch_size=1000
+ )
 
- self.assertEqual(new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title)
+ self.assertEqual(
+ new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title
+ )
 
  def test_duplicate_nodes_mixed(self):
  """
@@ -811,3 +816,11 @@ def test_content_tag_creation(self):
  ContentTag,
  tag_name=random.sample(string.printable, random.randint(51, 80)),
  )
+
+ def test_create_node_that_already_exists_no_integrity_error(self):
+ obj = ContentNode.objects.first()
+ new_obj = ContentNode(id=obj.id, kind_id=content_kinds.TOPIC)
+ try:
+ new_obj.save()
+ except IntegrityError:
+ self.fail("Caused an IntegrityError")

contentcuration/contentcuration/tests/viewsets/test_contentnode.py

@@ -725,6 +725,23 @@ def test_cannot_update_some_contentnodes(self):
  models.ContentNode.objects.get(id=contentnode2.id).title, new_title
  )
 
+ def test_update_contentnode_updates_last_modified(self):
+ user = testdata.user()
+ contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)
+
+ last_modified = contentnode.modified
+ new_title = "This is not the old title"
+
+ self.client.force_authenticate(user=user)
+ response = self.client.post(
+ self.sync_url,
+ [generate_update_event(contentnode.id, CONTENTNODE, {"title": new_title})],
+ format="json",
+ )
+ self.assertEqual(response.status_code, 200, response.content)
+ updated_node = models.ContentNode.objects.get(id=contentnode.id)
+ self.assertNotEqual(last_modified, updated_node.modified)
+
  def test_delete_contentnode(self):
  user = testdata.user()
  contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)

contentcuration/contentcuration/viewsets/base.py

@@ -21,6 +21,7 @@
 from rest_framework.viewsets import ReadOnlyModelViewSet
 
 from contentcuration.viewsets.common import MissingRequiredParamsException
+from contentcuration.viewsets.sync.utils import log_sync_exception
 
 
 class SimpleReprMixin(object):
@@ -554,13 +555,18 @@ def create_from_changes(self, changes):
  changes_to_return = []
 
  for change in changes:
- serializer = self.get_serializer(data=self._map_create_change(change))
- if serializer.is_valid():
- self.perform_create(serializer)
- if serializer.changes:
- changes_to_return.extend(serializer.changes)
- else:
- change.update({"errors": serializer.errors})
+ try:
+ serializer = self.get_serializer(data=self._map_create_change(change))
+ if serializer.is_valid():
+ self.perform_create(serializer)
+ if serializer.changes:
+ changes_to_return.extend(serializer.changes)
+ else:
+ change.update({"errors": serializer.errors})
+ errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
  errors.append(change)
 
  return errors, changes_to_return
@@ -602,6 +608,10 @@ def delete_from_changes(self, changes):
  # If the object already doesn't exist, as far as the user is concerned
  # job done!
  pass
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
 
@@ -637,6 +647,10 @@ def update_from_changes(self, changes):
  # error if the user can view the object but not edit it?
  change.update({"errors": ValidationError("Not found").detail})
  errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
  def update(self, request, *args, **kwargs):

contentcuration/contentcuration/viewsets/contentnode.py

@@ -9,6 +9,7 @@
 from django.db.models import Q
 from django.db.models import Subquery
 from django.http import Http404
+from django.utils.timezone import now
 from django_filters.rest_framework import CharFilter
 from django_filters.rest_framework import DjangoFilterBackend
 from django_filters.rest_framework import UUIDFilter
@@ -101,6 +102,18 @@ def filter__node_id_channel_id(self, queryset, name, value):
  return queryset.filter(query)
 
 
+def bulk_create_tag_relations(tags_relations_to_create):
+ if tags_relations_to_create:
+ # In Django 2.2 add ignore_conflicts to make this fool proof
+ try:
+ ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ except IntegrityError:
+ # One of the relations already exists, so just save them one by one.
+ # Django's default upsert behaviour should mean we get no errors this way
+ for to_create in tags_relations_to_create:
+ to_create.save()
+
+
 def set_tags(tags_by_id):
  all_tag_names = set()
  tags_relations_to_create = []
@@ -142,8 +155,7 @@ def set_tags(tags_by_id):
  tags_relations_to_delete.append(
  Q(contentnode_id=target_node_id, contenttag__tag_name=tag_name)
  )
- if tags_relations_to_create:
- ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ bulk_create_tag_relations(tags_relations_to_create)
  if tags_relations_to_delete:
  ContentNode.tags.through.objects.filter(
  reduce(lambda x, y: x | y, tags_relations_to_delete)
@@ -166,6 +178,9 @@ def gather_tags(self, validated_data):
 
  def update(self, queryset, all_validated_data):
  tags = self.gather_tags(all_validated_data)
+ modified = now()
+ for data in all_validated_data:
+ data["modified"] = modified
  all_objects = super(ContentNodeListSerializer, self).update(
  queryset, all_validated_data
  )

contentcuration/contentcuration/viewsets/sync/endpoint.py

@@ -3,12 +3,10 @@
 and deals with processing all the changes to make appropriate
 bulk creates, updates, and deletes.
 """
-import logging
 import time
 from collections import OrderedDict
 from itertools import groupby
 
-from django.conf import settings
 from rest_framework.authentication import SessionAuthentication
 from rest_framework.authentication import TokenAuthentication
 from rest_framework.decorators import api_view
@@ -48,6 +46,7 @@
 from contentcuration.viewsets.sync.constants import USER
 from contentcuration.viewsets.sync.constants import VIEWER_M2M
 from contentcuration.viewsets.sync.utils import get_and_clear_user_events
+from contentcuration.viewsets.sync.utils import log_sync_exception
 from contentcuration.viewsets.task import TaskViewSet
 from contentcuration.viewsets.user import ChannelUserViewSet
 from contentcuration.viewsets.user import UserViewSet
@@ -186,15 +185,7 @@ def handle_changes(request, viewset_class, change_type, changes):
  report_exception(e)
  return result
  except Exception as e:
- # Capture exception and report, but allow sync
- # to complete properly.
- report_exception(e)
-
- if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
- raise
- else:
- # make sure we leave a record in the logs just in case.
- logging.error(e)
+ log_sync_exception(e)
  for change in changes:
  change["errors"] = [str(e)]
  return changes, None

contentcuration/contentcuration/viewsets/sync/utils.py

@@ -1,5 +1,9 @@
+import logging
+
+from django.conf import settings
 from django.core.cache import cache
 
+from contentcuration.utils.sentry import report_exception
 from contentcuration.viewsets.sync.constants import ALL_TABLES
 from contentcuration.viewsets.sync.constants import COPIED
 from contentcuration.viewsets.sync.constants import CREATED
@@ -82,3 +86,15 @@ def get_and_clear_user_events(user_id):
  user_events = cache.get(cache_key) or []
  cache.delete(cache_key)
  return user_events
+
+
+def log_sync_exception(e):
+ # Capture exception and report, but allow sync
+ # to complete properly.
+ report_exception(e)
+
+ if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
+ raise
+ else:
+ # make sure we leave a record in the logs just in case.
+ logging.error(e)

contentcuration/contentcuration/migrations/0124_auto_20201204_1645.py

@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.29 on 2020-12-04 16:45
+from __future__ import unicode_literals
+
+import django.utils.timezone
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("contentcuration", "0123_auto_20200921_1536"),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name="contentnode",
+ name="created",
+ field=models.DateTimeField(
+ default=django.utils.timezone.now, verbose_name="created"
+ ),
+ ),
+ ]

contentcuration/contentcuration/models.py

@@ -1127,7 +1127,7 @@ class ContentNode(MPTTModel, models.Model):
 
  thumbnail_encoding = models.TextField(blank=True, null=True)
 
- created = models.DateTimeField(auto_now_add=True, verbose_name="created")
+ created = models.DateTimeField(default=timezone.now, verbose_name="created")
  modified = models.DateTimeField(auto_now=True, verbose_name="modified")
  published = models.BooleanField(default=False)
  publishing = models.BooleanField(default=False)

contentcuration/contentcuration/tests/test_contentnodes.py

@@ -9,6 +9,7 @@
 from builtins import zip
 
 import pytest
+from django.db import IntegrityError
 from django.db.utils import DataError
 from le_utils.constants import content_kinds
 from mixer.backend.django import mixer
@@ -273,13 +274,17 @@ def test_duplicate_nodes_position_right(self):
 
  # simulate a clean, right-after-publish state to ensure only new channel is marked as change
  self.channel.main_tree.changed = False
- self.channel.main_tree.title = 'Some other name'
+ self.channel.main_tree.title = "Some other name"
  self.channel.main_tree.save()
  self.channel.main_tree.refresh_from_db()
 
- self.channel.main_tree.copy_to(new_channel.main_tree.get_children()[0], position="right", batch_size=1000)
+ self.channel.main_tree.copy_to(
+ new_channel.main_tree.get_children()[0], position="right", batch_size=1000
+ )
 
- self.assertEqual(new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title)
+ self.assertEqual(
+ new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title
+ )
 
  def test_duplicate_nodes_mixed(self):
  """
@@ -811,3 +816,11 @@ def test_content_tag_creation(self):
  ContentTag,
  tag_name=random.sample(string.printable, random.randint(51, 80)),
  )
+
+ def test_create_node_that_already_exists_no_integrity_error(self):
+ obj = ContentNode.objects.first()
+ new_obj = ContentNode(id=obj.id, kind_id=content_kinds.TOPIC)
+ try:
+ new_obj.save()
+ except IntegrityError:
+ self.fail("Caused an IntegrityError")

contentcuration/contentcuration/tests/viewsets/test_contentnode.py

@@ -725,6 +725,23 @@ def test_cannot_update_some_contentnodes(self):
  models.ContentNode.objects.get(id=contentnode2.id).title, new_title
  )
 
+ def test_update_contentnode_updates_last_modified(self):
+ user = testdata.user()
+ contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)
+
+ last_modified = contentnode.modified
+ new_title = "This is not the old title"
+
+ self.client.force_authenticate(user=user)
+ response = self.client.post(
+ self.sync_url,
+ [generate_update_event(contentnode.id, CONTENTNODE, {"title": new_title})],
+ format="json",
+ )
+ self.assertEqual(response.status_code, 200, response.content)
+ updated_node = models.ContentNode.objects.get(id=contentnode.id)
+ self.assertNotEqual(last_modified, updated_node.modified)
+
  def test_delete_contentnode(self):
  user = testdata.user()
  contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)

contentcuration/contentcuration/viewsets/base.py

@@ -21,6 +21,7 @@
 from rest_framework.viewsets import ReadOnlyModelViewSet
 
 from contentcuration.viewsets.common import MissingRequiredParamsException
+from contentcuration.viewsets.sync.utils import log_sync_exception
 
 
 class SimpleReprMixin(object):
@@ -554,13 +555,18 @@ def create_from_changes(self, changes):
  changes_to_return = []
 
  for change in changes:
- serializer = self.get_serializer(data=self._map_create_change(change))
- if serializer.is_valid():
- self.perform_create(serializer)
- if serializer.changes:
- changes_to_return.extend(serializer.changes)
- else:
- change.update({"errors": serializer.errors})
+ try:
+ serializer = self.get_serializer(data=self._map_create_change(change))
+ if serializer.is_valid():
+ self.perform_create(serializer)
+ if serializer.changes:
+ changes_to_return.extend(serializer.changes)
+ else:
+ change.update({"errors": serializer.errors})
+ errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
  errors.append(change)
 
  return errors, changes_to_return
@@ -602,6 +608,10 @@ def delete_from_changes(self, changes):
  # If the object already doesn't exist, as far as the user is concerned
  # job done!
  pass
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
 
@@ -637,6 +647,10 @@ def update_from_changes(self, changes):
  # error if the user can view the object but not edit it?
  change.update({"errors": ValidationError("Not found").detail})
  errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
  def update(self, request, *args, **kwargs):

contentcuration/contentcuration/viewsets/contentnode.py

@@ -9,6 +9,7 @@
 from django.db.models import Q
 from django.db.models import Subquery
 from django.http import Http404
+from django.utils.timezone import now
 from django_filters.rest_framework import CharFilter
 from django_filters.rest_framework import DjangoFilterBackend
 from django_filters.rest_framework import UUIDFilter
@@ -101,6 +102,18 @@ def filter__node_id_channel_id(self, queryset, name, value):
  return queryset.filter(query)
 
 
+def bulk_create_tag_relations(tags_relations_to_create):
+ if tags_relations_to_create:
+ # In Django 2.2 add ignore_conflicts to make this fool proof
+ try:
+ ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ except IntegrityError:
+ # One of the relations already exists, so just save them one by one.
+ # Django's default upsert behaviour should mean we get no errors this way
+ for to_create in tags_relations_to_create:
+ to_create.save()
+
+
 def set_tags(tags_by_id):
  all_tag_names = set()
  tags_relations_to_create = []
@@ -142,8 +155,7 @@ def set_tags(tags_by_id):
  tags_relations_to_delete.append(
  Q(contentnode_id=target_node_id, contenttag__tag_name=tag_name)
  )
- if tags_relations_to_create:
- ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ bulk_create_tag_relations(tags_relations_to_create)
  if tags_relations_to_delete:
  ContentNode.tags.through.objects.filter(
  reduce(lambda x, y: x | y, tags_relations_to_delete)
@@ -166,6 +178,9 @@ def gather_tags(self, validated_data):
 
  def update(self, queryset, all_validated_data):
  tags = self.gather_tags(all_validated_data)
+ modified = now()
+ for data in all_validated_data:
+ data["modified"] = modified
  all_objects = super(ContentNodeListSerializer, self).update(
  queryset, all_validated_data
  )

contentcuration/contentcuration/viewsets/sync/endpoint.py

@@ -3,12 +3,10 @@
 and deals with processing all the changes to make appropriate
 bulk creates, updates, and deletes.
 """
-import logging
 import time
 from collections import OrderedDict
 from itertools import groupby
 
-from django.conf import settings
 from rest_framework.authentication import SessionAuthentication
 from rest_framework.authentication import TokenAuthentication
 from rest_framework.decorators import api_view
@@ -48,6 +46,7 @@
 from contentcuration.viewsets.sync.constants import USER
 from contentcuration.viewsets.sync.constants import VIEWER_M2M
 from contentcuration.viewsets.sync.utils import get_and_clear_user_events
+from contentcuration.viewsets.sync.utils import log_sync_exception
 from contentcuration.viewsets.task import TaskViewSet
 from contentcuration.viewsets.user import ChannelUserViewSet
 from contentcuration.viewsets.user import UserViewSet
@@ -186,15 +185,7 @@ def handle_changes(request, viewset_class, change_type, changes):
  report_exception(e)
  return result
  except Exception as e:
- # Capture exception and report, but allow sync
- # to complete properly.
- report_exception(e)
-
- if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
- raise
- else:
- # make sure we leave a record in the logs just in case.
- logging.error(e)
+ log_sync_exception(e)
  for change in changes:
  change["errors"] = [str(e)]
  return changes, None

contentcuration/contentcuration/viewsets/sync/utils.py

@@ -1,5 +1,9 @@
+import logging
+
+from django.conf import settings
 from django.core.cache import cache
 
+from contentcuration.utils.sentry import report_exception
 from contentcuration.viewsets.sync.constants import ALL_TABLES
 from contentcuration.viewsets.sync.constants import COPIED
 from contentcuration.viewsets.sync.constants import CREATED
@@ -82,3 +86,15 @@ def get_and_clear_user_events(user_id):
  user_events = cache.get(cache_key) or []
  cache.delete(cache_key)
  return user_events
+
+
+def log_sync_exception(e):
+ # Capture exception and report, but allow sync
+ # to complete properly.
+ report_exception(e)
+
+ if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
+ raise
+ else:
+ # make sure we leave a record in the logs just in case.
+ logging.error(e)

contentcuration/contentcuration/migrations/0124_auto_20201204_1645.py

@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.29 on 2020-12-04 16:45
+from __future__ import unicode_literals
+
+import django.utils.timezone
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("contentcuration", "0123_auto_20200921_1536"),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name="contentnode",
+ name="created",
+ field=models.DateTimeField(
+ default=django.utils.timezone.now, verbose_name="created"
+ ),
+ ),
+ ]

contentcuration/contentcuration/models.py

@@ -1127,7 +1127,7 @@ class ContentNode(MPTTModel, models.Model):
 
  thumbnail_encoding = models.TextField(blank=True, null=True)
 
- created = models.DateTimeField(auto_now_add=True, verbose_name="created")
+ created = models.DateTimeField(default=timezone.now, verbose_name="created")
  modified = models.DateTimeField(auto_now=True, verbose_name="modified")
  published = models.BooleanField(default=False)
  publishing = models.BooleanField(default=False)

contentcuration/contentcuration/tests/test_contentnodes.py

@@ -9,6 +9,7 @@
 from builtins import zip
 
 import pytest
+from django.db import IntegrityError
 from django.db.utils import DataError
 from le_utils.constants import content_kinds
 from mixer.backend.django import mixer
@@ -273,13 +274,17 @@ def test_duplicate_nodes_position_right(self):
 
  # simulate a clean, right-after-publish state to ensure only new channel is marked as change
  self.channel.main_tree.changed = False
- self.channel.main_tree.title = 'Some other name'
+ self.channel.main_tree.title = "Some other name"
  self.channel.main_tree.save()
  self.channel.main_tree.refresh_from_db()
 
- self.channel.main_tree.copy_to(new_channel.main_tree.get_children()[0], position="right", batch_size=1000)
+ self.channel.main_tree.copy_to(
+ new_channel.main_tree.get_children()[0], position="right", batch_size=1000
+ )
 
- self.assertEqual(new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title)
+ self.assertEqual(
+ new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title
+ )
 
  def test_duplicate_nodes_mixed(self):
  """
@@ -811,3 +816,11 @@ def test_content_tag_creation(self):
  ContentTag,
  tag_name=random.sample(string.printable, random.randint(51, 80)),
  )
+
+ def test_create_node_that_already_exists_no_integrity_error(self):
+ obj = ContentNode.objects.first()
+ new_obj = ContentNode(id=obj.id, kind_id=content_kinds.TOPIC)
+ try:
+ new_obj.save()
+ except IntegrityError:
+ self.fail("Caused an IntegrityError")

contentcuration/contentcuration/tests/viewsets/test_contentnode.py

@@ -725,6 +725,23 @@ def test_cannot_update_some_contentnodes(self):
  models.ContentNode.objects.get(id=contentnode2.id).title, new_title
  )
 
+ def test_update_contentnode_updates_last_modified(self):
+ user = testdata.user()
+ contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)
+
+ last_modified = contentnode.modified
+ new_title = "This is not the old title"
+
+ self.client.force_authenticate(user=user)
+ response = self.client.post(
+ self.sync_url,
+ [generate_update_event(contentnode.id, CONTENTNODE, {"title": new_title})],
+ format="json",
+ )
+ self.assertEqual(response.status_code, 200, response.content)
+ updated_node = models.ContentNode.objects.get(id=contentnode.id)
+ self.assertNotEqual(last_modified, updated_node.modified)
+
  def test_delete_contentnode(self):
  user = testdata.user()
  contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)

contentcuration/contentcuration/viewsets/base.py

@@ -21,6 +21,7 @@
 from rest_framework.viewsets import ReadOnlyModelViewSet
 
 from contentcuration.viewsets.common import MissingRequiredParamsException
+from contentcuration.viewsets.sync.utils import log_sync_exception
 
 
 class SimpleReprMixin(object):
@@ -554,13 +555,18 @@ def create_from_changes(self, changes):
  changes_to_return = []
 
  for change in changes:
- serializer = self.get_serializer(data=self._map_create_change(change))
- if serializer.is_valid():
- self.perform_create(serializer)
- if serializer.changes:
- changes_to_return.extend(serializer.changes)
- else:
- change.update({"errors": serializer.errors})
+ try:
+ serializer = self.get_serializer(data=self._map_create_change(change))
+ if serializer.is_valid():
+ self.perform_create(serializer)
+ if serializer.changes:
+ changes_to_return.extend(serializer.changes)
+ else:
+ change.update({"errors": serializer.errors})
+ errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
  errors.append(change)
 
  return errors, changes_to_return
@@ -602,6 +608,10 @@ def delete_from_changes(self, changes):
  # If the object already doesn't exist, as far as the user is concerned
  # job done!
  pass
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
 
@@ -637,6 +647,10 @@ def update_from_changes(self, changes):
  # error if the user can view the object but not edit it?
  change.update({"errors": ValidationError("Not found").detail})
  errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
  def update(self, request, *args, **kwargs):

contentcuration/contentcuration/viewsets/contentnode.py

@@ -9,6 +9,7 @@
 from django.db.models import Q
 from django.db.models import Subquery
 from django.http import Http404
+from django.utils.timezone import now
 from django_filters.rest_framework import CharFilter
 from django_filters.rest_framework import DjangoFilterBackend
 from django_filters.rest_framework import UUIDFilter
@@ -101,6 +102,18 @@ def filter__node_id_channel_id(self, queryset, name, value):
  return queryset.filter(query)
 
 
+def bulk_create_tag_relations(tags_relations_to_create):
+ if tags_relations_to_create:
+ # In Django 2.2 add ignore_conflicts to make this fool proof
+ try:
+ ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ except IntegrityError:
+ # One of the relations already exists, so just save them one by one.
+ # Django's default upsert behaviour should mean we get no errors this way
+ for to_create in tags_relations_to_create:
+ to_create.save()
+
+
 def set_tags(tags_by_id):
  all_tag_names = set()
  tags_relations_to_create = []
@@ -142,8 +155,7 @@ def set_tags(tags_by_id):
  tags_relations_to_delete.append(
  Q(contentnode_id=target_node_id, contenttag__tag_name=tag_name)
  )
- if tags_relations_to_create:
- ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ bulk_create_tag_relations(tags_relations_to_create)
  if tags_relations_to_delete:
  ContentNode.tags.through.objects.filter(
  reduce(lambda x, y: x | y, tags_relations_to_delete)
@@ -166,6 +178,9 @@ def gather_tags(self, validated_data):
 
  def update(self, queryset, all_validated_data):
  tags = self.gather_tags(all_validated_data)
+ modified = now()
+ for data in all_validated_data:
+ data["modified"] = modified
  all_objects = super(ContentNodeListSerializer, self).update(
  queryset, all_validated_data
  )

contentcuration/contentcuration/viewsets/sync/endpoint.py

@@ -3,12 +3,10 @@
 and deals with processing all the changes to make appropriate
 bulk creates, updates, and deletes.
 """
-import logging
 import time
 from collections import OrderedDict
 from itertools import groupby
 
-from django.conf import settings
 from rest_framework.authentication import SessionAuthentication
 from rest_framework.authentication import TokenAuthentication
 from rest_framework.decorators import api_view
@@ -48,6 +46,7 @@
 from contentcuration.viewsets.sync.constants import USER
 from contentcuration.viewsets.sync.constants import VIEWER_M2M
 from contentcuration.viewsets.sync.utils import get_and_clear_user_events
+from contentcuration.viewsets.sync.utils import log_sync_exception
 from contentcuration.viewsets.task import TaskViewSet
 from contentcuration.viewsets.user import ChannelUserViewSet
 from contentcuration.viewsets.user import UserViewSet
@@ -186,15 +185,7 @@ def handle_changes(request, viewset_class, change_type, changes):
  report_exception(e)
  return result
  except Exception as e:
- # Capture exception and report, but allow sync
- # to complete properly.
- report_exception(e)
-
- if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
- raise
- else:
- # make sure we leave a record in the logs just in case.
- logging.error(e)
+ log_sync_exception(e)
  for change in changes:
  change["errors"] = [str(e)]
  return changes, None

contentcuration/contentcuration/viewsets/sync/utils.py

@@ -1,5 +1,9 @@
+import logging
+
+from django.conf import settings
 from django.core.cache import cache
 
+from contentcuration.utils.sentry import report_exception
 from contentcuration.viewsets.sync.constants import ALL_TABLES
 from contentcuration.viewsets.sync.constants import COPIED
 from contentcuration.viewsets.sync.constants import CREATED
@@ -82,3 +86,15 @@ def get_and_clear_user_events(user_id):
  user_events = cache.get(cache_key) or []
  cache.delete(cache_key)
  return user_events
+
+
+def log_sync_exception(e):
+ # Capture exception and report, but allow sync
+ # to complete properly.
+ report_exception(e)
+
+ if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
+ raise
+ else:
+ # make sure we leave a record in the logs just in case.
+ logging.error(e)

contentcuration/contentcuration/migrations/0124_auto_20201204_1645.py

@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.29 on 2020-12-04 16:45
+from __future__ import unicode_literals
+
+import django.utils.timezone
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("contentcuration", "0123_auto_20200921_1536"),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name="contentnode",
+ name="created",
+ field=models.DateTimeField(
+ default=django.utils.timezone.now, verbose_name="created"
+ ),
+ ),
+ ]

contentcuration/contentcuration/models.py

@@ -1127,7 +1127,7 @@ class ContentNode(MPTTModel, models.Model):
 
  thumbnail_encoding = models.TextField(blank=True, null=True)
 
- created = models.DateTimeField(auto_now_add=True, verbose_name="created")
+ created = models.DateTimeField(default=timezone.now, verbose_name="created")
  modified = models.DateTimeField(auto_now=True, verbose_name="modified")
  published = models.BooleanField(default=False)
  publishing = models.BooleanField(default=False)

contentcuration/contentcuration/tests/test_contentnodes.py

@@ -9,6 +9,7 @@
 from builtins import zip
 
 import pytest
+from django.db import IntegrityError
 from django.db.utils import DataError
 from le_utils.constants import content_kinds
 from mixer.backend.django import mixer
@@ -273,13 +274,17 @@ def test_duplicate_nodes_position_right(self):
 
  # simulate a clean, right-after-publish state to ensure only new channel is marked as change
  self.channel.main_tree.changed = False
- self.channel.main_tree.title = 'Some other name'
+ self.channel.main_tree.title = "Some other name"
  self.channel.main_tree.save()
  self.channel.main_tree.refresh_from_db()
 
- self.channel.main_tree.copy_to(new_channel.main_tree.get_children()[0], position="right", batch_size=1000)
+ self.channel.main_tree.copy_to(
+ new_channel.main_tree.get_children()[0], position="right", batch_size=1000
+ )
 
- self.assertEqual(new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title)
+ self.assertEqual(
+ new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title
+ )
 
  def test_duplicate_nodes_mixed(self):
  """
@@ -811,3 +816,11 @@ def test_content_tag_creation(self):
  ContentTag,
  tag_name=random.sample(string.printable, random.randint(51, 80)),
  )
+
+ def test_create_node_that_already_exists_no_integrity_error(self):
+ obj = ContentNode.objects.first()
+ new_obj = ContentNode(id=obj.id, kind_id=content_kinds.TOPIC)
+ try:
+ new_obj.save()
+ except IntegrityError:
+ self.fail("Caused an IntegrityError")

contentcuration/contentcuration/tests/viewsets/test_contentnode.py

@@ -725,6 +725,23 @@ def test_cannot_update_some_contentnodes(self):
  models.ContentNode.objects.get(id=contentnode2.id).title, new_title
  )
 
+ def test_update_contentnode_updates_last_modified(self):
+ user = testdata.user()
+ contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)
+
+ last_modified = contentnode.modified
+ new_title = "This is not the old title"
+
+ self.client.force_authenticate(user=user)
+ response = self.client.post(
+ self.sync_url,
+ [generate_update_event(contentnode.id, CONTENTNODE, {"title": new_title})],
+ format="json",
+ )
+ self.assertEqual(response.status_code, 200, response.content)
+ updated_node = models.ContentNode.objects.get(id=contentnode.id)
+ self.assertNotEqual(last_modified, updated_node.modified)
+
  def test_delete_contentnode(self):
  user = testdata.user()
  contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)

contentcuration/contentcuration/viewsets/base.py

@@ -21,6 +21,7 @@
 from rest_framework.viewsets import ReadOnlyModelViewSet
 
 from contentcuration.viewsets.common import MissingRequiredParamsException
+from contentcuration.viewsets.sync.utils import log_sync_exception
 
 
 class SimpleReprMixin(object):
@@ -554,13 +555,18 @@ def create_from_changes(self, changes):
  changes_to_return = []
 
  for change in changes:
- serializer = self.get_serializer(data=self._map_create_change(change))
- if serializer.is_valid():
- self.perform_create(serializer)
- if serializer.changes:
- changes_to_return.extend(serializer.changes)
- else:
- change.update({"errors": serializer.errors})
+ try:
+ serializer = self.get_serializer(data=self._map_create_change(change))
+ if serializer.is_valid():
+ self.perform_create(serializer)
+ if serializer.changes:
+ changes_to_return.extend(serializer.changes)
+ else:
+ change.update({"errors": serializer.errors})
+ errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
  errors.append(change)
 
  return errors, changes_to_return
@@ -602,6 +608,10 @@ def delete_from_changes(self, changes):
  # If the object already doesn't exist, as far as the user is concerned
  # job done!
  pass
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
 
@@ -637,6 +647,10 @@ def update_from_changes(self, changes):
  # error if the user can view the object but not edit it?
  change.update({"errors": ValidationError("Not found").detail})
  errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
  def update(self, request, *args, **kwargs):

contentcuration/contentcuration/viewsets/contentnode.py

@@ -9,6 +9,7 @@
 from django.db.models import Q
 from django.db.models import Subquery
 from django.http import Http404
+from django.utils.timezone import now
 from django_filters.rest_framework import CharFilter
 from django_filters.rest_framework import DjangoFilterBackend
 from django_filters.rest_framework import UUIDFilter
@@ -101,6 +102,18 @@ def filter__node_id_channel_id(self, queryset, name, value):
  return queryset.filter(query)
 
 
+def bulk_create_tag_relations(tags_relations_to_create):
+ if tags_relations_to_create:
+ # In Django 2.2 add ignore_conflicts to make this fool proof
+ try:
+ ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ except IntegrityError:
+ # One of the relations already exists, so just save them one by one.
+ # Django's default upsert behaviour should mean we get no errors this way
+ for to_create in tags_relations_to_create:
+ to_create.save()
+
+
 def set_tags(tags_by_id):
  all_tag_names = set()
  tags_relations_to_create = []
@@ -142,8 +155,7 @@ def set_tags(tags_by_id):
  tags_relations_to_delete.append(
  Q(contentnode_id=target_node_id, contenttag__tag_name=tag_name)
  )
- if tags_relations_to_create:
- ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ bulk_create_tag_relations(tags_relations_to_create)
  if tags_relations_to_delete:
  ContentNode.tags.through.objects.filter(
  reduce(lambda x, y: x | y, tags_relations_to_delete)
@@ -166,6 +178,9 @@ def gather_tags(self, validated_data):
 
  def update(self, queryset, all_validated_data):
  tags = self.gather_tags(all_validated_data)
+ modified = now()
+ for data in all_validated_data:
+ data["modified"] = modified
  all_objects = super(ContentNodeListSerializer, self).update(
  queryset, all_validated_data
  )

contentcuration/contentcuration/viewsets/sync/endpoint.py

@@ -3,12 +3,10 @@
 and deals with processing all the changes to make appropriate
 bulk creates, updates, and deletes.
 """
-import logging
 import time
 from collections import OrderedDict
 from itertools import groupby
 
-from django.conf import settings
 from rest_framework.authentication import SessionAuthentication
 from rest_framework.authentication import TokenAuthentication
 from rest_framework.decorators import api_view
@@ -48,6 +46,7 @@
 from contentcuration.viewsets.sync.constants import USER
 from contentcuration.viewsets.sync.constants import VIEWER_M2M
 from contentcuration.viewsets.sync.utils import get_and_clear_user_events
+from contentcuration.viewsets.sync.utils import log_sync_exception
 from contentcuration.viewsets.task import TaskViewSet
 from contentcuration.viewsets.user import ChannelUserViewSet
 from contentcuration.viewsets.user import UserViewSet
@@ -186,15 +185,7 @@ def handle_changes(request, viewset_class, change_type, changes):
  report_exception(e)
  return result
  except Exception as e:
- # Capture exception and report, but allow sync
- # to complete properly.
- report_exception(e)
-
- if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
- raise
- else:
- # make sure we leave a record in the logs just in case.
- logging.error(e)
+ log_sync_exception(e)
  for change in changes:
  change["errors"] = [str(e)]
  return changes, None

contentcuration/contentcuration/viewsets/sync/utils.py

@@ -1,5 +1,9 @@
+import logging
+
+from django.conf import settings
 from django.core.cache import cache
 
+from contentcuration.utils.sentry import report_exception
 from contentcuration.viewsets.sync.constants import ALL_TABLES
 from contentcuration.viewsets.sync.constants import COPIED
 from contentcuration.viewsets.sync.constants import CREATED
@@ -82,3 +86,15 @@ def get_and_clear_user_events(user_id):
  user_events = cache.get(cache_key) or []
  cache.delete(cache_key)
  return user_events
+
+
+def log_sync_exception(e):
+ # Capture exception and report, but allow sync
+ # to complete properly.
+ report_exception(e)
+
+ if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
+ raise
+ else:
+ # make sure we leave a record in the logs just in case.
+ logging.error(e)

contentcuration/contentcuration/migrations/0124_auto_20201204_1645.py

@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.29 on 2020-12-04 16:45
+from __future__ import unicode_literals
+
+import django.utils.timezone
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("contentcuration", "0123_auto_20200921_1536"),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name="contentnode",
+ name="created",
+ field=models.DateTimeField(
+ default=django.utils.timezone.now, verbose_name="created"
+ ),
+ ),
+ ]

contentcuration/contentcuration/models.py

@@ -1127,7 +1127,7 @@ class ContentNode(MPTTModel, models.Model):
 
  thumbnail_encoding = models.TextField(blank=True, null=True)
 
- created = models.DateTimeField(auto_now_add=True, verbose_name="created")
+ created = models.DateTimeField(default=timezone.now, verbose_name="created")
  modified = models.DateTimeField(auto_now=True, verbose_name="modified")
  published = models.BooleanField(default=False)
  publishing = models.BooleanField(default=False)

contentcuration/contentcuration/tests/test_contentnodes.py

@@ -9,6 +9,7 @@
 from builtins import zip
 
 import pytest
+from django.db import IntegrityError
 from django.db.utils import DataError
 from le_utils.constants import content_kinds
 from mixer.backend.django import mixer
@@ -273,13 +274,17 @@ def test_duplicate_nodes_position_right(self):
 
  # simulate a clean, right-after-publish state to ensure only new channel is marked as change
  self.channel.main_tree.changed = False
- self.channel.main_tree.title = 'Some other name'
+ self.channel.main_tree.title = "Some other name"
  self.channel.main_tree.save()
  self.channel.main_tree.refresh_from_db()
 
- self.channel.main_tree.copy_to(new_channel.main_tree.get_children()[0], position="right", batch_size=1000)
+ self.channel.main_tree.copy_to(
+ new_channel.main_tree.get_children()[0], position="right", batch_size=1000
+ )
 
- self.assertEqual(new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title)
+ self.assertEqual(
+ new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title
+ )
 
  def test_duplicate_nodes_mixed(self):
  """
@@ -811,3 +816,11 @@ def test_content_tag_creation(self):
  ContentTag,
  tag_name=random.sample(string.printable, random.randint(51, 80)),
  )
+
+ def test_create_node_that_already_exists_no_integrity_error(self):
+ obj = ContentNode.objects.first()
+ new_obj = ContentNode(id=obj.id, kind_id=content_kinds.TOPIC)
+ try:
+ new_obj.save()
+ except IntegrityError:
+ self.fail("Caused an IntegrityError")

contentcuration/contentcuration/tests/viewsets/test_contentnode.py

@@ -725,6 +725,23 @@ def test_cannot_update_some_contentnodes(self):
  models.ContentNode.objects.get(id=contentnode2.id).title, new_title
  )
 
+ def test_update_contentnode_updates_last_modified(self):
+ user = testdata.user()
+ contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)
+
+ last_modified = contentnode.modified
+ new_title = "This is not the old title"
+
+ self.client.force_authenticate(user=user)
+ response = self.client.post(
+ self.sync_url,
+ [generate_update_event(contentnode.id, CONTENTNODE, {"title": new_title})],
+ format="json",
+ )
+ self.assertEqual(response.status_code, 200, response.content)
+ updated_node = models.ContentNode.objects.get(id=contentnode.id)
+ self.assertNotEqual(last_modified, updated_node.modified)
+
  def test_delete_contentnode(self):
  user = testdata.user()
  contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)

contentcuration/contentcuration/viewsets/base.py

@@ -21,6 +21,7 @@
 from rest_framework.viewsets import ReadOnlyModelViewSet
 
 from contentcuration.viewsets.common import MissingRequiredParamsException
+from contentcuration.viewsets.sync.utils import log_sync_exception
 
 
 class SimpleReprMixin(object):
@@ -554,13 +555,18 @@ def create_from_changes(self, changes):
  changes_to_return = []
 
  for change in changes:
- serializer = self.get_serializer(data=self._map_create_change(change))
- if serializer.is_valid():
- self.perform_create(serializer)
- if serializer.changes:
- changes_to_return.extend(serializer.changes)
- else:
- change.update({"errors": serializer.errors})
+ try:
+ serializer = self.get_serializer(data=self._map_create_change(change))
+ if serializer.is_valid():
+ self.perform_create(serializer)
+ if serializer.changes:
+ changes_to_return.extend(serializer.changes)
+ else:
+ change.update({"errors": serializer.errors})
+ errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
  errors.append(change)
 
  return errors, changes_to_return
@@ -602,6 +608,10 @@ def delete_from_changes(self, changes):
  # If the object already doesn't exist, as far as the user is concerned
  # job done!
  pass
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
 
@@ -637,6 +647,10 @@ def update_from_changes(self, changes):
  # error if the user can view the object but not edit it?
  change.update({"errors": ValidationError("Not found").detail})
  errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
  def update(self, request, *args, **kwargs):

contentcuration/contentcuration/viewsets/contentnode.py

@@ -9,6 +9,7 @@
 from django.db.models import Q
 from django.db.models import Subquery
 from django.http import Http404
+from django.utils.timezone import now
 from django_filters.rest_framework import CharFilter
 from django_filters.rest_framework import DjangoFilterBackend
 from django_filters.rest_framework import UUIDFilter
@@ -101,6 +102,18 @@ def filter__node_id_channel_id(self, queryset, name, value):
  return queryset.filter(query)
 
 
+def bulk_create_tag_relations(tags_relations_to_create):
+ if tags_relations_to_create:
+ # In Django 2.2 add ignore_conflicts to make this fool proof
+ try:
+ ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ except IntegrityError:
+ # One of the relations already exists, so just save them one by one.
+ # Django's default upsert behaviour should mean we get no errors this way
+ for to_create in tags_relations_to_create:
+ to_create.save()
+
+
 def set_tags(tags_by_id):
  all_tag_names = set()
  tags_relations_to_create = []
@@ -142,8 +155,7 @@ def set_tags(tags_by_id):
  tags_relations_to_delete.append(
  Q(contentnode_id=target_node_id, contenttag__tag_name=tag_name)
  )
- if tags_relations_to_create:
- ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ bulk_create_tag_relations(tags_relations_to_create)
  if tags_relations_to_delete:
  ContentNode.tags.through.objects.filter(
  reduce(lambda x, y: x | y, tags_relations_to_delete)
@@ -166,6 +178,9 @@ def gather_tags(self, validated_data):
 
  def update(self, queryset, all_validated_data):
  tags = self.gather_tags(all_validated_data)
+ modified = now()
+ for data in all_validated_data:
+ data["modified"] = modified
  all_objects = super(ContentNodeListSerializer, self).update(
  queryset, all_validated_data
  )

contentcuration/contentcuration/viewsets/sync/endpoint.py

@@ -3,12 +3,10 @@
 and deals with processing all the changes to make appropriate
 bulk creates, updates, and deletes.
 """
-import logging
 import time
 from collections import OrderedDict
 from itertools import groupby
 
-from django.conf import settings
 from rest_framework.authentication import SessionAuthentication
 from rest_framework.authentication import TokenAuthentication
 from rest_framework.decorators import api_view
@@ -48,6 +46,7 @@
 from contentcuration.viewsets.sync.constants import USER
 from contentcuration.viewsets.sync.constants import VIEWER_M2M
 from contentcuration.viewsets.sync.utils import get_and_clear_user_events
+from contentcuration.viewsets.sync.utils import log_sync_exception
 from contentcuration.viewsets.task import TaskViewSet
 from contentcuration.viewsets.user import ChannelUserViewSet
 from contentcuration.viewsets.user import UserViewSet
@@ -186,15 +185,7 @@ def handle_changes(request, viewset_class, change_type, changes):
  report_exception(e)
  return result
  except Exception as e:
- # Capture exception and report, but allow sync
- # to complete properly.
- report_exception(e)
-
- if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
- raise
- else:
- # make sure we leave a record in the logs just in case.
- logging.error(e)
+ log_sync_exception(e)
  for change in changes:
  change["errors"] = [str(e)]
  return changes, None

contentcuration/contentcuration/viewsets/sync/utils.py

@@ -1,5 +1,9 @@
+import logging
+
+from django.conf import settings
 from django.core.cache import cache
 
+from contentcuration.utils.sentry import report_exception
 from contentcuration.viewsets.sync.constants import ALL_TABLES
 from contentcuration.viewsets.sync.constants import COPIED
 from contentcuration.viewsets.sync.constants import CREATED
@@ -82,3 +86,15 @@ def get_and_clear_user_events(user_id):
  user_events = cache.get(cache_key) or []
  cache.delete(cache_key)
  return user_events
+
+
+def log_sync_exception(e):
+ # Capture exception and report, but allow sync
+ # to complete properly.
+ report_exception(e)
+
+ if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
+ raise
+ else:
+ # make sure we leave a record in the logs just in case.
+ logging.error(e)

contentcuration/contentcuration/migrations/0124_auto_20201204_1645.py

@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.29 on 2020-12-04 16:45
+from __future__ import unicode_literals
+
+import django.utils.timezone
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("contentcuration", "0123_auto_20200921_1536"),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name="contentnode",
+ name="created",
+ field=models.DateTimeField(
+ default=django.utils.timezone.now, verbose_name="created"
+ ),
+ ),
+ ]

contentcuration/contentcuration/models.py

@@ -1127,7 +1127,7 @@ class ContentNode(MPTTModel, models.Model):
 
  thumbnail_encoding = models.TextField(blank=True, null=True)
 
- created = models.DateTimeField(auto_now_add=True, verbose_name="created")
+ created = models.DateTimeField(default=timezone.now, verbose_name="created")
  modified = models.DateTimeField(auto_now=True, verbose_name="modified")
  published = models.BooleanField(default=False)
  publishing = models.BooleanField(default=False)

contentcuration/contentcuration/tests/test_contentnodes.py

@@ -9,6 +9,7 @@
 from builtins import zip
 
 import pytest
+from django.db import IntegrityError
 from django.db.utils import DataError
 from le_utils.constants import content_kinds
 from mixer.backend.django import mixer
@@ -273,13 +274,17 @@ def test_duplicate_nodes_position_right(self):
 
  # simulate a clean, right-after-publish state to ensure only new channel is marked as change
  self.channel.main_tree.changed = False
- self.channel.main_tree.title = 'Some other name'
+ self.channel.main_tree.title = "Some other name"
  self.channel.main_tree.save()
  self.channel.main_tree.refresh_from_db()
 
- self.channel.main_tree.copy_to(new_channel.main_tree.get_children()[0], position="right", batch_size=1000)
+ self.channel.main_tree.copy_to(
+ new_channel.main_tree.get_children()[0], position="right", batch_size=1000
+ )
 
- self.assertEqual(new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title)
+ self.assertEqual(
+ new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title
+ )
 
  def test_duplicate_nodes_mixed(self):
  """
@@ -811,3 +816,11 @@ def test_content_tag_creation(self):
  ContentTag,
  tag_name=random.sample(string.printable, random.randint(51, 80)),
  )
+
+ def test_create_node_that_already_exists_no_integrity_error(self):
+ obj = ContentNode.objects.first()
+ new_obj = ContentNode(id=obj.id, kind_id=content_kinds.TOPIC)
+ try:
+ new_obj.save()
+ except IntegrityError:
+ self.fail("Caused an IntegrityError")

contentcuration/contentcuration/tests/viewsets/test_contentnode.py

@@ -725,6 +725,23 @@ def test_cannot_update_some_contentnodes(self):
  models.ContentNode.objects.get(id=contentnode2.id).title, new_title
  )
 
+ def test_update_contentnode_updates_last_modified(self):
+ user = testdata.user()
+ contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)
+
+ last_modified = contentnode.modified
+ new_title = "This is not the old title"
+
+ self.client.force_authenticate(user=user)
+ response = self.client.post(
+ self.sync_url,
+ [generate_update_event(contentnode.id, CONTENTNODE, {"title": new_title})],
+ format="json",
+ )
+ self.assertEqual(response.status_code, 200, response.content)
+ updated_node = models.ContentNode.objects.get(id=contentnode.id)
+ self.assertNotEqual(last_modified, updated_node.modified)
+
  def test_delete_contentnode(self):
  user = testdata.user()
  contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)

contentcuration/contentcuration/viewsets/base.py

@@ -21,6 +21,7 @@
 from rest_framework.viewsets import ReadOnlyModelViewSet
 
 from contentcuration.viewsets.common import MissingRequiredParamsException
+from contentcuration.viewsets.sync.utils import log_sync_exception
 
 
 class SimpleReprMixin(object):
@@ -554,13 +555,18 @@ def create_from_changes(self, changes):
  changes_to_return = []
 
  for change in changes:
- serializer = self.get_serializer(data=self._map_create_change(change))
- if serializer.is_valid():
- self.perform_create(serializer)
- if serializer.changes:
- changes_to_return.extend(serializer.changes)
- else:
- change.update({"errors": serializer.errors})
+ try:
+ serializer = self.get_serializer(data=self._map_create_change(change))
+ if serializer.is_valid():
+ self.perform_create(serializer)
+ if serializer.changes:
+ changes_to_return.extend(serializer.changes)
+ else:
+ change.update({"errors": serializer.errors})
+ errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
  errors.append(change)
 
  return errors, changes_to_return
@@ -602,6 +608,10 @@ def delete_from_changes(self, changes):
  # If the object already doesn't exist, as far as the user is concerned
  # job done!
  pass
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
 
@@ -637,6 +647,10 @@ def update_from_changes(self, changes):
  # error if the user can view the object but not edit it?
  change.update({"errors": ValidationError("Not found").detail})
  errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
  def update(self, request, *args, **kwargs):

contentcuration/contentcuration/viewsets/contentnode.py

@@ -9,6 +9,7 @@
 from django.db.models import Q
 from django.db.models import Subquery
 from django.http import Http404
+from django.utils.timezone import now
 from django_filters.rest_framework import CharFilter
 from django_filters.rest_framework import DjangoFilterBackend
 from django_filters.rest_framework import UUIDFilter
@@ -101,6 +102,18 @@ def filter__node_id_channel_id(self, queryset, name, value):
  return queryset.filter(query)
 
 
+def bulk_create_tag_relations(tags_relations_to_create):
+ if tags_relations_to_create:
+ # In Django 2.2 add ignore_conflicts to make this fool proof
+ try:
+ ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ except IntegrityError:
+ # One of the relations already exists, so just save them one by one.
+ # Django's default upsert behaviour should mean we get no errors this way
+ for to_create in tags_relations_to_create:
+ to_create.save()
+
+
 def set_tags(tags_by_id):
  all_tag_names = set()
  tags_relations_to_create = []
@@ -142,8 +155,7 @@ def set_tags(tags_by_id):
  tags_relations_to_delete.append(
  Q(contentnode_id=target_node_id, contenttag__tag_name=tag_name)
  )
- if tags_relations_to_create:
- ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ bulk_create_tag_relations(tags_relations_to_create)
  if tags_relations_to_delete:
  ContentNode.tags.through.objects.filter(
  reduce(lambda x, y: x | y, tags_relations_to_delete)
@@ -166,6 +178,9 @@ def gather_tags(self, validated_data):
 
  def update(self, queryset, all_validated_data):
  tags = self.gather_tags(all_validated_data)
+ modified = now()
+ for data in all_validated_data:
+ data["modified"] = modified
  all_objects = super(ContentNodeListSerializer, self).update(
  queryset, all_validated_data
  )

contentcuration/contentcuration/viewsets/sync/endpoint.py

@@ -3,12 +3,10 @@
 and deals with processing all the changes to make appropriate
 bulk creates, updates, and deletes.
 """
-import logging
 import time
 from collections import OrderedDict
 from itertools import groupby
 
-from django.conf import settings
 from rest_framework.authentication import SessionAuthentication
 from rest_framework.authentication import TokenAuthentication
 from rest_framework.decorators import api_view
@@ -48,6 +46,7 @@
 from contentcuration.viewsets.sync.constants import USER
 from contentcuration.viewsets.sync.constants import VIEWER_M2M
 from contentcuration.viewsets.sync.utils import get_and_clear_user_events
+from contentcuration.viewsets.sync.utils import log_sync_exception
 from contentcuration.viewsets.task import TaskViewSet
 from contentcuration.viewsets.user import ChannelUserViewSet
 from contentcuration.viewsets.user import UserViewSet
@@ -186,15 +185,7 @@ def handle_changes(request, viewset_class, change_type, changes):
  report_exception(e)
  return result
  except Exception as e:
- # Capture exception and report, but allow sync
- # to complete properly.
- report_exception(e)
-
- if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
- raise
- else:
- # make sure we leave a record in the logs just in case.
- logging.error(e)
+ log_sync_exception(e)
  for change in changes:
  change["errors"] = [str(e)]
  return changes, None

contentcuration/contentcuration/viewsets/sync/utils.py

@@ -1,5 +1,9 @@
+import logging
+
+from django.conf import settings
 from django.core.cache import cache
 
+from contentcuration.utils.sentry import report_exception
 from contentcuration.viewsets.sync.constants import ALL_TABLES
 from contentcuration.viewsets.sync.constants import COPIED
 from contentcuration.viewsets.sync.constants import CREATED
@@ -82,3 +86,15 @@ def get_and_clear_user_events(user_id):
  user_events = cache.get(cache_key) or []
  cache.delete(cache_key)
  return user_events
+
+
+def log_sync_exception(e):
+ # Capture exception and report, but allow sync
+ # to complete properly.
+ report_exception(e)
+
+ if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
+ raise
+ else:
+ # make sure we leave a record in the logs just in case.
+ logging.error(e)

contentcuration/contentcuration/migrations/0124_auto_20201204_1645.py

@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.29 on 2020-12-04 16:45
+from __future__ import unicode_literals
+
+import django.utils.timezone
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("contentcuration", "0123_auto_20200921_1536"),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name="contentnode",
+ name="created",
+ field=models.DateTimeField(
+ default=django.utils.timezone.now, verbose_name="created"
+ ),
+ ),
+ ]

contentcuration/contentcuration/models.py

@@ -1127,7 +1127,7 @@ class ContentNode(MPTTModel, models.Model):
 
  thumbnail_encoding = models.TextField(blank=True, null=True)
 
- created = models.DateTimeField(auto_now_add=True, verbose_name="created")
+ created = models.DateTimeField(default=timezone.now, verbose_name="created")
  modified = models.DateTimeField(auto_now=True, verbose_name="modified")
  published = models.BooleanField(default=False)
  publishing = models.BooleanField(default=False)

contentcuration/contentcuration/tests/test_contentnodes.py

@@ -9,6 +9,7 @@
 from builtins import zip
 
 import pytest
+from django.db import IntegrityError
 from django.db.utils import DataError
 from le_utils.constants import content_kinds
 from mixer.backend.django import mixer
@@ -273,13 +274,17 @@ def test_duplicate_nodes_position_right(self):
 
  # simulate a clean, right-after-publish state to ensure only new channel is marked as change
  self.channel.main_tree.changed = False
- self.channel.main_tree.title = 'Some other name'
+ self.channel.main_tree.title = "Some other name"
  self.channel.main_tree.save()
  self.channel.main_tree.refresh_from_db()
 
- self.channel.main_tree.copy_to(new_channel.main_tree.get_children()[0], position="right", batch_size=1000)
+ self.channel.main_tree.copy_to(
+ new_channel.main_tree.get_children()[0], position="right", batch_size=1000
+ )
 
- self.assertEqual(new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title)
+ self.assertEqual(
+ new_channel.main_tree.get_children()[1].title, self.channel.main_tree.title
+ )
 
  def test_duplicate_nodes_mixed(self):
  """
@@ -811,3 +816,11 @@ def test_content_tag_creation(self):
  ContentTag,
  tag_name=random.sample(string.printable, random.randint(51, 80)),
  )
+
+ def test_create_node_that_already_exists_no_integrity_error(self):
+ obj = ContentNode.objects.first()
+ new_obj = ContentNode(id=obj.id, kind_id=content_kinds.TOPIC)
+ try:
+ new_obj.save()
+ except IntegrityError:
+ self.fail("Caused an IntegrityError")

contentcuration/contentcuration/tests/viewsets/test_contentnode.py

@@ -725,6 +725,23 @@ def test_cannot_update_some_contentnodes(self):
  models.ContentNode.objects.get(id=contentnode2.id).title, new_title
  )
 
+ def test_update_contentnode_updates_last_modified(self):
+ user = testdata.user()
+ contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)
+
+ last_modified = contentnode.modified
+ new_title = "This is not the old title"
+
+ self.client.force_authenticate(user=user)
+ response = self.client.post(
+ self.sync_url,
+ [generate_update_event(contentnode.id, CONTENTNODE, {"title": new_title})],
+ format="json",
+ )
+ self.assertEqual(response.status_code, 200, response.content)
+ updated_node = models.ContentNode.objects.get(id=contentnode.id)
+ self.assertNotEqual(last_modified, updated_node.modified)
+
  def test_delete_contentnode(self):
  user = testdata.user()
  contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)

contentcuration/contentcuration/viewsets/base.py

@@ -21,6 +21,7 @@
 from rest_framework.viewsets import ReadOnlyModelViewSet
 
 from contentcuration.viewsets.common import MissingRequiredParamsException
+from contentcuration.viewsets.sync.utils import log_sync_exception
 
 
 class SimpleReprMixin(object):
@@ -554,13 +555,18 @@ def create_from_changes(self, changes):
  changes_to_return = []
 
  for change in changes:
- serializer = self.get_serializer(data=self._map_create_change(change))
- if serializer.is_valid():
- self.perform_create(serializer)
- if serializer.changes:
- changes_to_return.extend(serializer.changes)
- else:
- change.update({"errors": serializer.errors})
+ try:
+ serializer = self.get_serializer(data=self._map_create_change(change))
+ if serializer.is_valid():
+ self.perform_create(serializer)
+ if serializer.changes:
+ changes_to_return.extend(serializer.changes)
+ else:
+ change.update({"errors": serializer.errors})
+ errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
  errors.append(change)
 
  return errors, changes_to_return
@@ -602,6 +608,10 @@ def delete_from_changes(self, changes):
  # If the object already doesn't exist, as far as the user is concerned
  # job done!
  pass
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
 
@@ -637,6 +647,10 @@ def update_from_changes(self, changes):
  # error if the user can view the object but not edit it?
  change.update({"errors": ValidationError("Not found").detail})
  errors.append(change)
+ except Exception as e:
+ log_sync_exception(e)
+ change["errors"] = [str(e)]
+ errors.append(change)
  return errors, changes_to_return
 
  def update(self, request, *args, **kwargs):

contentcuration/contentcuration/viewsets/contentnode.py

@@ -9,6 +9,7 @@
 from django.db.models import Q
 from django.db.models import Subquery
 from django.http import Http404
+from django.utils.timezone import now
 from django_filters.rest_framework import CharFilter
 from django_filters.rest_framework import DjangoFilterBackend
 from django_filters.rest_framework import UUIDFilter
@@ -101,6 +102,18 @@ def filter__node_id_channel_id(self, queryset, name, value):
  return queryset.filter(query)
 
 
+def bulk_create_tag_relations(tags_relations_to_create):
+ if tags_relations_to_create:
+ # In Django 2.2 add ignore_conflicts to make this fool proof
+ try:
+ ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ except IntegrityError:
+ # One of the relations already exists, so just save them one by one.
+ # Django's default upsert behaviour should mean we get no errors this way
+ for to_create in tags_relations_to_create:
+ to_create.save()
+
+
 def set_tags(tags_by_id):
  all_tag_names = set()
  tags_relations_to_create = []
@@ -142,8 +155,7 @@ def set_tags(tags_by_id):
  tags_relations_to_delete.append(
  Q(contentnode_id=target_node_id, contenttag__tag_name=tag_name)
  )
- if tags_relations_to_create:
- ContentNode.tags.through.objects.bulk_create(tags_relations_to_create)
+ bulk_create_tag_relations(tags_relations_to_create)
  if tags_relations_to_delete:
  ContentNode.tags.through.objects.filter(
  reduce(lambda x, y: x | y, tags_relations_to_delete)
@@ -166,6 +178,9 @@ def gather_tags(self, validated_data):
 
  def update(self, queryset, all_validated_data):
  tags = self.gather_tags(all_validated_data)
+ modified = now()
+ for data in all_validated_data:
+ data["modified"] = modified
  all_objects = super(ContentNodeListSerializer, self).update(
  queryset, all_validated_data
  )

contentcuration/contentcuration/viewsets/sync/endpoint.py

@@ -3,12 +3,10 @@
 and deals with processing all the changes to make appropriate
 bulk creates, updates, and deletes.
 """
-import logging
 import time
 from collections import OrderedDict
 from itertools import groupby
 
-from django.conf import settings
 from rest_framework.authentication import SessionAuthentication
 from rest_framework.authentication import TokenAuthentication
 from rest_framework.decorators import api_view
@@ -48,6 +46,7 @@
 from contentcuration.viewsets.sync.constants import USER
 from contentcuration.viewsets.sync.constants import VIEWER_M2M
 from contentcuration.viewsets.sync.utils import get_and_clear_user_events
+from contentcuration.viewsets.sync.utils import log_sync_exception
 from contentcuration.viewsets.task import TaskViewSet
 from contentcuration.viewsets.user import ChannelUserViewSet
 from contentcuration.viewsets.user import UserViewSet
@@ -186,15 +185,7 @@ def handle_changes(request, viewset_class, change_type, changes):
  report_exception(e)
  return result
  except Exception as e:
- # Capture exception and report, but allow sync
- # to complete properly.
- report_exception(e)
-
- if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
- raise
- else:
- # make sure we leave a record in the logs just in case.
- logging.error(e)
+ log_sync_exception(e)
  for change in changes:
  change["errors"] = [str(e)]
  return changes, None

contentcuration/contentcuration/viewsets/sync/utils.py

@@ -1,5 +1,9 @@
+import logging
+
+from django.conf import settings
 from django.core.cache import cache
 
+from contentcuration.utils.sentry import report_exception
 from contentcuration.viewsets.sync.constants import ALL_TABLES
 from contentcuration.viewsets.sync.constants import COPIED
 from contentcuration.viewsets.sync.constants import CREATED
@@ -82,3 +86,15 @@ def get_and_clear_user_events(user_id):
  user_events = cache.get(cache_key) or []
  cache.delete(cache_key)
  return user_events
+
+
+def log_sync_exception(e):
+ # Capture exception and report, but allow sync
+ # to complete properly.
+ report_exception(e)
+
+ if getattr(settings, "DEBUG", False) or getattr(settings, "TEST_ENV", False):
+ raise
+ else:
+ # make sure we leave a record in the logs just in case.
+ logging.error(e)