Ensure changes get properly cleared as succeeded and make refresh blocking more targeted

contentcuration/contentcuration/frontend/shared/data/resources.js

@@ -694,10 +694,23 @@ class Resource extends mix(APIResource, IndexedDBResource) {
         // ContentNode tree operations are the troublemakers causing the logic below
         if (this.tableName === TABLE_NAMES.CONTENTNODE) {
           // Only fetch new updates if we don't have pending changes to ContentNode that
-          // affect tree structure
+          // affect local tree structure
           refresh = db[CHANGES_TABLE].where('table')
             .equals(TABLE_NAMES.CONTENTNODE)
-            .filter(c => TREE_CHANGE_TYPES.includes(c.type))
+            .filter(c => {
+              if (!TREE_CHANGE_TYPES.includes(c.type)) {
+                return false;
+              }
+              let parent = c.parent;
+              if (c.type === CHANGE_TYPES.CREATED) {
+                parent = c.obj.parent;
+              }
+              return (
+                params.parent === parent ||
+                params.parent === c.key ||
+                (params.id__in || []).includes(c.key)
+              );
+            })
             .count()
             .then(pendingCount => pendingCount === 0);
         }
@@ -1316,6 +1329,11 @@ export const ContentNode = new TreeResource({
               from_key: isCreate ? id : null,
               target,
               position,
+              // Regardless of the specific target/position
+              // always record the resolved parent for the tree insert
+              // so that we can avoid doing fetches while such changes
+              // are pending.
+              parent: parent.id,
               oldObj: isCreate ? null : node,
               source: CLIENTID,
               table: this.tableName,

contentcuration/contentcuration/frontend/shared/data/serverSync.js

@@ -181,9 +181,7 @@ function handleMaxRevs(response, userId) {
       .concat(get(response, ['data', 'successes'], [])),
     'server_rev',
     'desc'
-    // Ignore server generated changes, as they are applied out of order.
-    // This leads to a false sense of which changes we have actually seen.
-  ).filter(c => c.created_by_id);
+  );
   const channelIds = uniq(allChanges.map(c => c.channel_id)).filter(Boolean);
   const maxRevs = {};
   const promises = [];
@@ -255,10 +253,15 @@ async function syncChanges() {
     channel_revs[channelId] = get(user, [MAX_REV_KEY, channelId], 0);
   }
 
+  const unAppliedChanges = await db[CHANGES_TABLE].orderBy('server_rev')
+    .filter(c => c.synced && !c.errors && !c.disallowed)
+    .toArray();
+
   const requestPayload = {
     changes: [],
     channel_revs,
     user_rev: user.user_rev || 0,
+    unapplied_revs: unAppliedChanges.map(c => c.server_rev).filter(Boolean),
   };
 
   if (lastChange) {

contentcuration/contentcuration/views/base.py

@@ -90,9 +90,7 @@ def current_user_for_context(user):
 
     user_data = {field: getattr(user, field) for field in user_fields}
 
-    user_data["user_rev"] = Change.objects.filter(
-        applied=True, user=user, created_by__isnull=False
-    ).values_list("server_rev", flat=True).order_by("-server_rev").first() or 0
+    user_data["user_rev"] = Change.objects.filter(applied=True, user=user).values_list("server_rev", flat=True).order_by("-server_rev").first() or 0
 
     return json_for_parse_from_data(user_data)
 
@@ -303,9 +301,7 @@ def channel(request, channel_id):
         if channel.deleted:
             channel_error = 'CHANNEL_EDIT_ERROR_CHANNEL_DELETED'
         else:
-            channel_rev = Change.objects.filter(
-                applied=True, channel=channel, created_by__isnull=False
-            ).values_list("server_rev", flat=True).order_by("-server_rev").first() or 0
+            channel_rev = Change.objects.filter(applied=True, channel=channel).values_list("server_rev", flat=True).order_by("-server_rev").first() or 0
 
     return render(
         request,

contentcuration/contentcuration/viewsets/sync/endpoint.py

@@ -74,15 +74,18 @@ def get_channel_revs(self, request):
 
     def return_changes(self, request, channel_revs):
         user_rev = request.data.get("user_rev") or 0
+        unapplied_revs = request.data.get("unapplied_revs", [])
         session_key = request.session.session_key
 
+        unapplied_revs_filter = Q(server_rev__in=unapplied_revs)
+
         # Create a filter that returns all applied changes, and any errored changes made by this session
         relevant_to_session_filter = (Q(applied=True) | Q(errored=True, session_id=session_key))
 
-        change_filter = (Q(user=request.user, server_rev__gt=user_rev) & relevant_to_session_filter)
+        change_filter = (Q(user=request.user) & (unapplied_revs_filter | Q(server_rev__gt=user_rev)) & relevant_to_session_filter)
 
         for channel_id, rev in channel_revs.items():
-            change_filter |= (Q(channel_id=channel_id, server_rev__gt=rev) & relevant_to_session_filter)
+            change_filter |= (Q(channel_id=channel_id) & (unapplied_revs_filter | Q(server_rev__gt=rev)) & relevant_to_session_filter)
 
         changes_to_return = list(
             Change.objects.filter(