Merge pull request #11244 from rtibbles/ive_lost_that_syncing_feeling

Fix learner sync status reporting on server device

kolibri/core/auth/sync_event_hook_utils.py

@@ -76,17 +76,19 @@ def wrapper(context):
         :type context: CompositeSessionContext|LocalSessionContext
         """
         local_context = context if isinstance(context, LocalSessionContext) else None
-
-        try:
-            if not local_context and isinstance(context, CompositeSessionContext):
-                local_context = next(
-                    c for c in context.children if isinstance(c, LocalSessionContext)
-                )
-            else:
-                raise StopIteration("No local context found")
-        except StopIteration:
-            # no local context, so we can't do anything
-            return
+        if local_context is None:
+            try:
+                if isinstance(context, CompositeSessionContext):
+                    local_context = next(
+                        c
+                        for c in context.children
+                        if isinstance(c, LocalSessionContext)
+                    )
+                else:
+                    raise StopIteration("No local context found")
+            except StopIteration:
+                # no local context, so we can't do anything
+                return
 
         kwargs = _extract_kwargs_from_context(local_context)
         return func(**kwargs)

kolibri/core/auth/test/test_utils.py

@@ -11,13 +11,17 @@
 from django.core.management.base import CommandError
 from django.test import TestCase
 from morango.registry import syncable_models
+from morango.sync.context import CompositeSessionContext
+from morango.sync.context import LocalSessionContext
+from morango.sync.context import NetworkSessionContext
 
 from ..models import Facility
 from kolibri.core.auth.management import utils
 from kolibri.core.auth.models import AdHocGroup
 from kolibri.core.auth.models import Classroom
 from kolibri.core.auth.models import FacilityUser
 from kolibri.core.auth.models import LearnerGroup
+from kolibri.core.auth.sync_event_hook_utils import _local_event_handler
 from kolibri.core.auth.test.test_api import ClassroomFactory
 from kolibri.core.auth.test.test_api import FacilityFactory
 from kolibri.core.auth.test.test_api import FacilityUserFactory
@@ -673,3 +677,45 @@ def test_deletion_inclusion(self):
         delete_group = get_delete_group_for_facility(facility)
         all_deleted_models = set(qs.model for qs in delete_group.get_querysets())
         self.assertTrue(all_deleted_models.issuperset(all_facility_models))
+
+
+class TestLocalEventHandler(TestCase):
+    def setUp(self):
+        self.mock_method = mock.Mock()
+
+        # Wrap the mock method in a regular function to avoid functools complaining about
+        # trying to wrap a mock object
+        def method(*args, **kwargs):
+            self.mock_method(*args, **kwargs)
+
+        self.method = method
+
+    def test_local_event_handler_local_context(self):
+        """
+        Test that the local event handler calls the wrapped method when there is a local session context
+        """
+        context = LocalSessionContext()
+        context.sync_session = mock.Mock()
+        _local_event_handler(self.method)(context)
+        self.mock_method.assert_called_once()
+
+    def test_local_event_handler_composite_context(self):
+        """
+        Test that the local event handler calls the wrapped method when there is a local session context
+        """
+        local_context = LocalSessionContext()
+        network_context = NetworkSessionContext(mock.Mock())
+        context = CompositeSessionContext([network_context, local_context])
+        local_context.sync_session = mock.Mock()
+        _local_event_handler(self.method)(context)
+        self.mock_method.assert_called_once()
+
+    def test_local_event_handler_composite_context_no_local_child(self):
+        """
+        Test that the local event handler calls the wrapped method when there is a local session context
+        """
+        network_context1 = NetworkSessionContext(mock.Mock())
+        network_context2 = NetworkSessionContext(mock.Mock())
+        context = CompositeSessionContext([network_context1, network_context2])
+        _local_event_handler(self.method)(context)
+        self.mock_method.assert_not_called()

kolibri/core/device/api.py

@@ -5,6 +5,7 @@
 from django.conf import settings
 from django.contrib.auth import login
 from django.db.models import Exists
+from django.db.models import F
 from django.db.models import Max
 from django.db.models import OuterRef
 from django.db.models.expressions import Subquery
@@ -283,8 +284,7 @@ def map_status(record):
         return RECENTLY_SYNCED
     elif queued:
         return QUEUED
-    elif record["last_synced"] and not recent:
-        return NOT_RECENTLY_SYNCED
+    return NOT_RECENTLY_SYNCED
 
 
 class UserSyncStatusViewSet(ReadOnlyValuesViewset):
@@ -323,7 +323,7 @@ def get_queryset(self):
     def annotate_queryset(self, queryset):
 
         queryset = queryset.annotate(
-            last_synced=Max("sync_session__last_activity_timestamp")
+            last_synced=F("sync_session__last_activity_timestamp")
         )
 
         most_recent_sync_status = (

kolibri/core/device/test/test_api.py

@@ -836,6 +836,29 @@ def test_usersyncstatus_list_learner_not_recent_success(self):
             response.data[0]["status"], user_sync_statuses.NOT_RECENTLY_SYNCED
         )
 
+    def test_usersyncstatus_list_learner_no_sync_session(self):
+        self.client.login(
+            username=self.user1.username,
+            password=DUMMY_PASSWORD,
+            facility=self.facility,
+        )
+        self.syncstatus1.queued = False
+        previous_sync_session = self.syncstatus1.sync_session
+        self.syncstatus1.sync_session = None
+        self.syncstatus1.save()
+
+        try:
+            response = self.client.get(reverse("kolibri:core:usersyncstatus-list"))
+            self.assertEqual(len(response.data), 1)
+            self.assertEqual(response.data[0]["user"], self.user1.id)
+            self.assertEqual(
+                response.data[0]["status"], user_sync_statuses.NOT_RECENTLY_SYNCED
+            )
+        finally:
+            # Not doing this leads to weird unexpected test contagion.
+            self.syncstatus1.sync_session = previous_sync_session
+            self.syncstatus1.save()
+
     def test_usersyncstatus_list__insufficient_storage(self):
         self.client.login(
             username=self.user1.username,