diff --git a/kolibri/core/logger/api.py b/kolibri/core/logger/api.py index 948983256d9..de0550d811b 100644 --- a/kolibri/core/logger/api.py +++ b/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",) ) diff --git a/kolibri/core/logger/test/test_integrated_api.py b/kolibri/core/logger/test/test_integrated_api.py index d292faac1df..69243d30263 100644 --- a/kolibri/core/logger/test/test_integrated_api.py +++ b/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) diff --git a/kolibri/core/logger/test/test_upgrades.py b/kolibri/core/logger/test/test_upgrades.py new file mode 100644 index 00000000000..07bf6c769fd --- /dev/null +++ b/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 + ) diff --git a/kolibri/core/logger/upgrade.py b/kolibri/core/logger/upgrade.py index 7698129c584..f049546bc9e 100644 --- a/kolibri/core/logger/upgrade.py +++ b/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)) diff --git a/kolibri/core/test/test_upgrade.py b/kolibri/core/test/test_upgrade.py index dcec46f7538..8b96adb4c57 100644 --- a/kolibri/core/test/test_upgrade.py +++ b/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() diff --git a/kolibri/core/upgrade.py b/kolibri/core/upgrade.py index 5e593444b0e..2dd370b91ec 100644 --- a/kolibri/core/upgrade.py +++ b/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): diff --git a/kolibri/utils/tests/test_version.py b/kolibri/utils/tests/test_version.py index 2f3087da773..c8ab0769727 100755 --- a/kolibri/utils/tests/test_version.py +++ b/kolibri/utils/tests/test_version.py @@ -4,6 +4,7 @@ import unittest import mock +from parameterized import parameterized import kolibri from kolibri.utils import version @@ -14,6 +15,25 @@ get_version = version.get_version.__wrapped__ # @UndefinedVariable +def _sanitize(name): + name = name.replace(" ", "_") + name = name.replace(">=", "gte") + name = name.replace(">", "gt") + name = name.replace("<=", "lte") + name = name.replace("<", "lt") + name = name.replace("==", "eq") + name = name.replace("!=", "ne") + name = name.replace(".", "_") + name = name.replace("+", "_") + name = name.replace("-", "_") + name = name.replace(",", "_") + return name + + +def _name_func(test_func, param_num, params): + return f"{test_func.__name__}_{param_num}_{_sanitize(params.args[0])}_{_sanitize(params.args[1])}_{params.args[2]}" + + class TestKolibriVersion(unittest.TestCase): def test_version(self): """ @@ -342,7 +362,7 @@ def test_normalize_version_to_semver_tripartite(self): version.normalize_version_to_semver( "0.15.0", ), - "0.15.0-c", + "0.15.0", ) def test_normalize_version_to_semver_bipartite(self): @@ -350,7 +370,7 @@ def test_normalize_version_to_semver_bipartite(self): version.normalize_version_to_semver( "1.10", ), - "1.10-c", + "1.10", ) def test_normalize_version_to_semver_alpa(self): @@ -358,7 +378,7 @@ def test_normalize_version_to_semver_alpa(self): version.normalize_version_to_semver( "0.14a1", ), - "0.14-a.1.c", + "0.14-a.1", ) def test_normalize_version_to_semver_beta(self): @@ -366,7 +386,7 @@ def test_normalize_version_to_semver_beta(self): version.normalize_version_to_semver( "0.16b1", ), - "0.16-b.1.c", + "0.16-b.1", ) @mock.patch("kolibri.utils.version.get_git_describe", return_value="v0.15.8") @@ -385,3 +405,83 @@ def test_get_version_from_file(self, describe_mock): "0.15.8", ) assert describe_mock.call_count == 1 + + @parameterized.expand( + [ + ("0.15.8", ">=0.15.8", True), + ("0.15.7", ">=0.15.8", False), + ("0.15.8a5", ">=0.15.8", False), + ("0.15.9a5", ">=0.15.8", True), + ("0.15.8a5.dev0+git.682.g0be46de2", ">=0.15.8", False), + ("0.15.9a5.dev0+git.682.g0be46de2", ">=0.15.8", True), + ("0.15.9", ">0.15.8", True), + ("0.15.8", ">0.15.8", False), + ("0.15.9a5.dev0+git.682.g0be46de2", ">0.15.8", True), + ("0.15.8a5.dev0+git.682.g0be46de2", ">0.15.8", False), + ("0.15.9a5", ">0.15.8", True), + ("0.15.8a5", ">0.15.8", False), + ("0.15.9b5", ">0.15.8", True), + ("0.15.8b5", ">0.15.8", False), + ("0.15.9rc5", ">0.15.8", True), + ("0.15.8rc5", ">0.15.8", False), + ("0.15.8", "<=0.15.8", True), + ("0.15.9", "<=0.15.8", False), + ("0.15.9a5.dev0+git.682.g0be46de2", "<=0.15.8", False), + ("0.15.8a5.dev0+git.682.g0be46de2", "<=0.15.8", True), + ("0.15.9a5", "<=0.15.8", False), + ("0.15.8a5", "<=0.15.8", True), + ("0.15.9b5", "<=0.15.8", False), + ("0.15.8b5", "<=0.15.8", True), + ("0.15.9rc5", "<=0.15.8", False), + ("0.15.8rc5", "<=0.15.8", True), + ("0.15.7", "<0.15.8", True), + ("0.15.8", "<0.15.8", False), + ("0.15.8a5.dev0+git.682.g0be46de2", "<0.15.8", True), + ("0.15.9a5.dev0+git.682.g0be46de2", "<0.15.8", False), + ("0.15.8a5", "<0.15.8", True), + ("0.15.9a5", "<0.15.8", False), + ("0.15.8b5", "<0.15.8", True), + ("0.15.9b5", "<0.15.8", False), + ("0.15.8rc5", "<0.15.8", True), + ("0.15.9rc5", "<0.15.8", False), + ("0.15.8", "==0.15.8", True), + ("0.15.9", "==0.15.8", False), + ("0.15.7", "!=0.15.8", True), + ("0.15.8", "!=0.15.8", False), + ], + name_func=_name_func, + ) + def test_version_matches_simple_range(self, version_string, version_range, matches): + self.assertEqual( + version.version_matches_range(version_string, version_range), matches + ) + + @parameterized.expand( + [ + ("0.15.8", ">0.15.8,<0.16.0", False), + ("0.15.7", ">0.15.8,<0.16.0", False), + ("0.15.8a5.dev+git.682.g0be46de2", ">0.15.8,<0.16.0", False), + ("0.15.9a5.dev+git.682.g0be46de2", ">0.15.8,<0.16.0", True), + ("0.15.8a5", ">0.15.8,<0.16.0", False), + ("0.15.9a5", ">0.15.8,<0.16.0", True), + ("0.15.8b5", ">0.15.8,<0.16.0", False), + ("0.15.9b5", ">0.15.8,<0.16.0", True), + ("0.15.8rc5", ">0.15.8,<0.16.0", False), + ("0.15.9rc5", ">0.15.8,<0.16.0", True), + ("0.15.9", ">0.15.8,<0.16.0", True), + ("0.16.0", ">0.15.8,<0.16.0", False), + ("0.15.9", ">0.15.8,<0.16.0", True), + ("0.15.8", ">=0.15.8,<0.16.0", True), + ("0.15.7", ">=0.15.8,<0.16.0", False), + ("0.16.0", ">=0.15.8,<0.16.0", False), + ("0.16.0", ">=0.15.8,<=0.16.0", True), + ("0.16.1", ">=0.15.8,<=0.16.0", False), + ], + name_func=_name_func, + ) + def test_version_matches_compound_range( + self, version_string, compound_range, matches + ): + self.assertEqual( + version.version_matches_range(version_string, compound_range), matches + ) diff --git a/kolibri/utils/version.py b/kolibri/utils/version.py index c74f3849683..6dde034f981 100644 --- a/kolibri/utils/version.py +++ b/kolibri/utils/version.py @@ -379,10 +379,10 @@ def version_matches_range(version, version_range): # extract and normalize version strings operator, range_version = get_version_and_operator_from_range(version_range) - version = normalize_version_to_semver(version) + version = semver.VersionInfo.parse(normalize_version_to_semver(version)) # check whether the version is in the range - return semver.match(version, operator + range_version) + return version.match(operator + range_version) def normalize_version_to_semver(version): @@ -405,10 +405,6 @@ def normalize_version_to_semver(version): if after_pieces: after = ".".join([piece for piece in after_pieces.group() if piece]) - # position final releases between alphas, betas, and further dev - if not dev: - after = (after + ".c").strip(".") - # make sure dev versions are sorted nicely relative to one another dev = (dev or "").replace("+", ".").replace("-", ".") @@ -430,19 +426,21 @@ def truncate_version(version, truncation_level=PATCH_VERSION): """ import semver - v = semver.parse_version_info( + v = semver.VersionInfo.parse( normalize_version_to_semver(version).replace(".dev", "+dev") ) if truncation_level == MAJOR_VERSION: - return semver.format_version(v.major, 0, 0) + return str(semver.VersionInfo(major=v.major, minor=0, patch=0)) if truncation_level == MINOR_VERSION: - return semver.format_version(v.major, v.minor, 0) + return str(semver.VersionInfo(major=v.major, minor=v.minor, patch=0)) if truncation_level == PATCH_VERSION: - return semver.format_version(v.major, v.minor, v.patch) + return str(semver.VersionInfo(major=v.major, minor=v.minor, patch=v.patch)) if truncation_level == PRERELEASE_VERSION: - truncated_version = semver.format_version( - v.major, v.minor, v.patch, prerelease=v.prerelease + truncated_version = str( + semver.VersionInfo( + major=v.major, minor=v.minor, patch=v.patch, prerelease=v.prerelease + ) ) # ensure prerelease formatting matches our convention truncated_version, prerelease_version = truncated_version.split("-")