Fix masterylog end timestamp issues

kolibri/core/logger/api.py

@@ -628,6 +628,9 @@ def _update_and_return_mastery_log_id(
                 else:
                     self._check_quiz_log_permissions(masterylog)
                 if update_fields:
+                    if end_timestamp:
+                        masterylog.end_timestamp = end_timestamp
+                        update_fields += ("end_timestamp",)
                     masterylog.save(
                         update_fields=update_fields + ("_morango_dirty_bit",)
                     )

kolibri/core/logger/test/test_integrated_api.py

@@ -2145,6 +2145,7 @@ def setUp(self):
             mastery_criterion=mastery_model,
             summarylog=self.summary_log,
             start_timestamp=self.summary_log.start_timestamp,
+            end_timestamp=self.summary_log.start_timestamp,
             user=self.user,
             mastery_level=1,
         )
@@ -2175,6 +2176,9 @@ def test_update_assessment_session_update_time_delta_succeeds(self):
         self.assertEqual(response.status_code, 200)
         self.mastery_log.refresh_from_db()
         self.assertEqual(self.mastery_log.time_spent, 5)
+        self.assertNotEqual(
+            self.mastery_log.end_timestamp, self.mastery_log.start_timestamp
+        )
 
     def test_update_assessment_session_create_attempt_in_lesson_succeeds(self):
         lesson = create_assigned_lesson_for_user(self.user)

kolibri/core/logger/test/test_upgrades.py

@@ -0,0 +1,109 @@
+from uuid import uuid4
+
+from django.test import TestCase
+from django.utils import timezone
+
+from kolibri.core.auth.models import Facility
+from kolibri.core.auth.models import FacilityUser
+from kolibri.core.logger.models import AttemptLog
+from kolibri.core.logger.models import ContentSessionLog
+from kolibri.core.logger.models import ContentSummaryLog
+from kolibri.core.logger.models import MasteryLog
+from kolibri.core.logger.upgrade import fix_masterylog_end_timestamps
+
+
+class MasteryLogEndTimestampUpgradeTest(TestCase):
+    def setUp(self):
+        self.facility = Facility.objects.create()
+        self.user = FacilityUser.objects.create(
+            username="learner", facility=self.facility
+        )
+        now = timezone.now()
+
+        # Create base content summary log
+        self.summary_log = ContentSummaryLog.objects.create(
+            user=self.user,
+            content_id=uuid4().hex,
+            channel_id=uuid4().hex,
+            kind="exercise",
+            start_timestamp=now,
+            end_timestamp=now + timezone.timedelta(minutes=10),
+        )
+
+        # Case 1: MasteryLog with attempts
+        self.attempt_session = ContentSessionLog.objects.create(
+            user=self.user,
+            content_id=self.summary_log.content_id,
+            channel_id=self.summary_log.channel_id,
+            kind="exercise",
+            start_timestamp=now,
+            end_timestamp=now + timezone.timedelta(minutes=3),
+        )
+
+        self.attempt_mastery = MasteryLog.objects.create(
+            user=self.user,
+            summarylog=self.summary_log,
+            mastery_level=2,
+            start_timestamp=now,
+            end_timestamp=now,
+        )
+
+        AttemptLog.objects.create(
+            masterylog=self.attempt_mastery,
+            sessionlog=self.attempt_session,
+            start_timestamp=now,
+            end_timestamp=now - timezone.timedelta(minutes=3),
+            complete=True,
+            correct=1,
+        )
+
+        AttemptLog.objects.create(
+            masterylog=self.attempt_mastery,
+            sessionlog=self.attempt_session,
+            start_timestamp=now,
+            end_timestamp=now - timezone.timedelta(minutes=2),
+            complete=True,
+            correct=1,
+        )
+
+        self.last_attempt = AttemptLog.objects.create(
+            masterylog=self.attempt_mastery,
+            sessionlog=self.attempt_session,
+            start_timestamp=now,
+            end_timestamp=now + timezone.timedelta(minutes=3),
+            complete=True,
+            correct=1,
+        )
+
+        # Case 2: MasteryLog with only summary log
+        self.summary_session = ContentSessionLog.objects.create(
+            user=self.user,
+            content_id=self.summary_log.content_id,
+            channel_id=self.summary_log.channel_id,
+            kind="exercise",
+            start_timestamp=now,
+            end_timestamp=now,
+        )
+        self.summary_only_mastery = MasteryLog.objects.create(
+            user=self.user,
+            summarylog=self.summary_log,
+            mastery_level=3,
+            start_timestamp=now,
+            end_timestamp=now,
+        )
+
+        fix_masterylog_end_timestamps()
+
+    def test_attempt_logs_case(self):
+        """Test MasteryLog with attempt logs gets end_timestamp from last attempt"""
+        self.attempt_mastery.refresh_from_db()
+        self.assertEqual(
+            self.attempt_mastery.end_timestamp, self.last_attempt.end_timestamp
+        )
+
+    def test_summary_log_case(self):
+        """Test MasteryLog with only summary log gets end_timestamp from summary"""
+        self.summary_only_mastery.refresh_from_db()
+        self.assertEqual(
+            self.summary_only_mastery.end_timestamp, self.summary_log.end_timestamp
+        )

kolibri/core/logger/upgrade.py

@@ -1,9 +1,15 @@
 """
 A file to contain specific logic to handle version upgrades in Kolibri.
 """
+from django.db.models import F
+from django.db.models import Max
+from django.db.models import OuterRef
+from django.db.models import Subquery
+
 from kolibri.core.logger.models import AttemptLog
 from kolibri.core.logger.models import ContentSummaryLog
 from kolibri.core.logger.models import ExamLog
+from kolibri.core.logger.models import MasteryLog
 from kolibri.core.logger.utils.attempt_log_consolidation import (
     consolidate_quiz_attempt_logs,
 )
@@ -57,3 +63,32 @@ def fix_duplicated_attempt_logs():
     item and non-null masterylog_id.
     """
     consolidate_quiz_attempt_logs(AttemptLog.objects.all())
+
+
+@version_upgrade(old_version=">0.15.0,<0.18.0")
+def fix_masterylog_end_timestamps():
+    """
+    Fix any MasteryLogs that have an end_timestamp that was not updated after creation due to a bug in the
+    integrated logging API endpoint.
+    """
+    # Fix the MasteryLogs that that have attempts - infer from the end_timestamp of the last attempt.
+    attempt_subquery = (
+        AttemptLog.objects.filter(masterylog=OuterRef("pk"))
+        .values("masterylog")
+        .annotate(max_end=Max("end_timestamp"))
+        .values("max_end")
+    )
+
+    MasteryLog.objects.filter(
+        end_timestamp=F("start_timestamp"), attemptlogs__isnull=False
+    ).update(end_timestamp=Subquery(attempt_subquery))
+    # Fix the MasteryLogs that don't have any attempts - just set the end_timestamp to the end_timestamp of the summary log.
+    summary_subquery = ContentSummaryLog.objects.filter(
+        masterylogs=OuterRef("pk")
+    ).values("end_timestamp")
+
+    MasteryLog.objects.filter(
+        end_timestamp=F("start_timestamp"),
+        completion_timestamp__isnull=True,
+        attemptlogs__isnull=True,
+    ).update(end_timestamp=Subquery(summary_subquery))

kolibri/core/test/test_upgrade.py

@@ -21,6 +21,20 @@ def test_filter_old_version():
         unfiltered_mock.assert_called_once()
 
 
+def test_filter_compound_old_version():
+    filtered_mock = Mock()
+    unfiltered_mock = Mock()
+
+    filtered = VersionUpgrade(old_version=">1.0.1,<1.1.1", upgrade=filtered_mock)
+    not_filtered = VersionUpgrade(upgrade=unfiltered_mock)
+    with patch(
+        "kolibri.core.upgrade.get_upgrades", return_value=[not_filtered, filtered]
+    ):
+        run_upgrades("1.0.0", "1.1.2")
+        filtered_mock.assert_not_called()
+        unfiltered_mock.assert_called_once()
+
+
 def test_not_filter_alpha_version():
     unfiltered_mock = Mock()
 
@@ -59,6 +73,17 @@ def test_order_old_version():
         function.assert_has_calls([call(0), call(1)])
 
 
+def test_order_compound_old_version():
+    function = Mock()
+
+    first = VersionUpgrade(old_version=">0.9.1,<0.10.1", upgrade=lambda: function(0))
+    second = VersionUpgrade(upgrade=lambda: function(1))
+
+    with patch("kolibri.core.upgrade.get_upgrades", return_value=[second, first]):
+        run_upgrades("0.10.0", "1.1.2")
+        function.assert_has_calls([call(0), call(1)])
+
+
 def test_order_new_version():
     function = Mock()
 
@@ -70,6 +95,17 @@ def test_order_new_version():
         function.assert_has_calls([call(0), call(1)])
 
 
+def test_order_compound_new_version():
+    function = Mock()
+
+    first = VersionUpgrade(new_version=">0.10.1,<1.1.3", upgrade=lambda: function(0))
+    second = VersionUpgrade(upgrade=lambda: function(1))
+
+    with patch("kolibri.core.upgrade.get_upgrades", return_value=[second, first]):
+        run_upgrades("0.10.0", "1.1.2")
+        function.assert_has_calls([call(0), call(1)])
+
+
 def test_order_old_and_new_version():
     function = Mock()
 
@@ -100,6 +136,21 @@ def test_filter_new_version():
         unfiltered_mock.assert_called_once()
 
 
+def test_filter_compound_new_version():
+    filtered_mock = Mock()
+    unfiltered_mock = Mock()
+
+    filtered = VersionUpgrade(new_version=">1.1.1,<1.1.3", upgrade=filtered_mock)
+    not_filtered = VersionUpgrade(upgrade=unfiltered_mock)
+
+    with patch(
+        "kolibri.core.upgrade.get_upgrades", return_value=[not_filtered, filtered]
+    ):
+        run_upgrades("1.0.1", "1.1.0")
+        filtered_mock.assert_not_called()
+        unfiltered_mock.assert_called_once()
+
+
 def test_blank_old_version():
     function = Mock()
 

kolibri/core/upgrade.py

@@ -1,12 +1,12 @@
 from importlib import import_module
 
 from django.apps import apps
-from semver import match
 from semver import VersionInfo
 
 import kolibri
 from kolibri.utils.version import get_version_and_operator_from_range
 from kolibri.utils.version import normalize_version_to_semver
+from kolibri.utils.version import version_matches_range
 
 
 CURRENT_VERSION = VersionInfo.parse(normalize_version_to_semver(kolibri.__version__))
@@ -110,10 +110,7 @@ def wrapper(upgrade):
 def matches_version(version, version_range):
     if version_range is None or not version:
         return True
-    # For the purposes of upgrade comparison, treat dev versions as alphas
-    version = normalize_version_to_semver(version).replace("dev", "a")
-    version_range = "".join(get_version_and_operator_from_range(version_range))
-    return match(version, version_range)
+    return version_matches_range(version, version_range)
 
 
 def get_upgrades(app_configs=None):