Merge pull request #5844 from AlexVelezLl/robust-solution-for-unpublishable-changes

Robust solution for filtering unpublishable changes on frontend

Author: rtibbles

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

@@ -1,9 +1,9 @@
 import { queueChange, debouncedSyncChanges } from '../serverSync';
 import { CreatedChange } from '../changes';
 import db from '../db';
-import { Session, Task } from 'shared/data/resources';
+import { Channel, Session, Task } from 'shared/data/resources';
 import client from 'shared/client';
-import { CHANGES_TABLE, CURRENT_USER, TABLE_NAMES } from 'shared/data/constants';
+import { CHANGE_TYPES, CHANGES_TABLE, CURRENT_USER, TABLE_NAMES } from 'shared/data/constants';
 import { mockChannelScope, resetMockChannelScope } from 'shared/utils/testing';
 
 async function makeChange(key, server_rev) {
@@ -225,4 +225,37 @@ describe('ServerSync tests', () => {
       expect(dbTask.status).toEqual(task.status);
     }
   });
+
+  it('should not set unpublished_changes when response contains only unpublishable changes', async () => {
+    const channelId = 'test-channel-unpublishable';
+    await Channel.table.put({ id: channelId });
+
+    client.post.mockResolvedValue({
+      data: {
+        disallowed: [],
+        allowed: [],
+        returned: [],
+        errors: [],
+        successes: [
+          {
+            channel_id: channelId,
+            server_rev: 100,
+            created_by_id: 'some-user-id',
+            type: CHANGE_TYPES.UPDATED,
+            unpublishable: true,
+          },
+        ],
+        maxRevs: [],
+        tasks: [],
+      },
+    });
+
+    await debouncedSyncChanges();
+
+    const channel = await Channel.table.get(channelId);
+    expect(channel.unpublished_changes).not.toBe(true);
+
+    // Manual clean up
+    await Channel.table.delete(channelId);
+  });
 });

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

@@ -175,7 +175,12 @@ function handleMaxRevs(response, userId) {
     maxRevs[`${MAX_REV_KEY}.${channelId}`] = channelChanges[0].server_rev;
     const lastChannelEditIndex = findLastIndex(
       channelChanges,
-      c => !c.errors && !c.user_id && c.created_by_id && c.type !== CHANGE_TYPES.PUBLISHED,
+      c =>
+        !c.errors &&
+        !c.user_id &&
+        c.created_by_id &&
+        c.type !== CHANGE_TYPES.PUBLISHED &&
+        !c.unpublishable,
     );
     const lastPublishIndex = findLastIndex(
       channelChanges,

contentcuration/contentcuration/models.py

@@ -3820,6 +3820,7 @@ def serialize(cls, change):
                 "channel_id": get_attribute(change, ["channel_id"]),
                 "user_id": get_attribute(change, ["user_id"]),
                 "created_by_id": get_attribute(change, ["created_by_id"]),
+                "unpublishable": get_attribute(change, ["unpublishable"]),
             }
         )
         return datum

contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py

@@ -720,12 +720,12 @@ def test_resolve_submission__accept_correct(self, apply_task_mock):
         self.assertEqual(resolved_submission.resolved_by, self.admin_user)
         self.assertEqual(resolved_submission.date_updated, self.resolved_time)
 
-        self.assertTrue(
-            Change.objects.filter(
-                channel=self.submission.channel,
-                change_type=ADDED_TO_COMMUNITY_LIBRARY,
-            ).exists()
+        change = Change.objects.get(
+            channel=self.submission.channel,
+            change_type=ADDED_TO_COMMUNITY_LIBRARY,
         )
+        self.assertEqual(change.created_by_id, self.admin_user.id)
+        self.assertTrue(change.unpublishable)
         apply_task_mock.fetch_or_enqueue.assert_called_once_with(
             self.admin_user,
             channel_id=self.submission.channel.id,

contentcuration/contentcuration/viewsets/community_library_submission.py

@@ -320,6 +320,7 @@ def _add_to_community_library(self, submission):
                 categories=submission.categories,
                 country_codes=country_codes,
             ),
+            created_by_id=submission.resolved_by_id,
             # This change is not publishable and should not trigger publish-related logic
             unpublishable=True,
         )

contentcuration/contentcuration/viewsets/sync/endpoint.py

@@ -155,6 +155,7 @@ def return_changes(self, request, channel_revs):
                 "table",
                 "change_type",
                 "kwargs",
+                "unpublishable",
             )
             .order_by("server_rev")[:CHANGE_RETURN_LIMIT]
         )