diff --git a/kolibri/core/auth/sync_event_hook_utils.py b/kolibri/core/auth/sync_event_hook_utils.py index 2fddfc38982..83cb5db2cff 100644 --- a/kolibri/core/auth/sync_event_hook_utils.py +++ b/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) diff --git a/kolibri/core/auth/test/test_utils.py b/kolibri/core/auth/test/test_utils.py index 71bca53c3f2..52eb89c978d 100644 --- a/kolibri/core/auth/test/test_utils.py +++ b/kolibri/core/auth/test/test_utils.py @@ -11,6 +11,9 @@ 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 @@ -18,6 +21,7 @@ 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() diff --git a/kolibri/core/device/api.py b/kolibri/core/device/api.py index 3d6d4dcea69..79be3a9bfa8 100644 --- a/kolibri/core/device/api.py +++ b/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 = ( diff --git a/kolibri/core/device/test/test_api.py b/kolibri/core/device/test/test_api.py index 86d60eb0952..867b258e2b7 100644 --- a/kolibri/core/device/test/test_api.py +++ b/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,