From 7c40ccde684b3167655bdfa5237954f3349a6ff4 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 29 Oct 2024 12:00:42 -0700 Subject: [PATCH 1/3] Include learner_needs field in content node api data. --- kolibri/core/content/api.py | 2 ++ kolibri/core/content/test/test_content_app.py | 3 +++ 2 files changed, 5 insertions(+) diff --git a/kolibri/core/content/api.py b/kolibri/core/content/api.py index 75ea57f6f4f..0196a98affe 100644 --- a/kolibri/core/content/api.py +++ b/kolibri/core/content/api.py @@ -648,6 +648,7 @@ class BaseContentNodeMixin(object): "grade_levels", "resource_types", "accessibility_labels", + "learner_needs", "categories", "duration", "ancestors", @@ -659,6 +660,7 @@ class BaseContentNodeMixin(object): "resource_types": lambda x: _split_text_field(x["resource_types"]), "accessibility_labels": lambda x: _split_text_field(x["accessibility_labels"]), "categories": lambda x: _split_text_field(x["categories"]), + "learner_needs": lambda x: _split_text_field(x["learner_needs"]), } def get_queryset(self): diff --git a/kolibri/core/content/test/test_content_app.py b/kolibri/core/content/test/test_content_app.py index 8154044443f..96250ed768b 100644 --- a/kolibri/core/content/test/test_content_app.py +++ b/kolibri/core/content/test/test_content_app.py @@ -334,6 +334,9 @@ def _assert_node(self, actual, expected): "learning_activities": expected.learning_activities.split(",") if expected.learning_activities else [], + "learner_needs": expected.learner_needs.split(",") + if expected.learner_needs + else [], "grade_levels": expected.grade_levels.split(",") if expected.grade_levels else [], From 3c1cb5cba5dd95b14fd62391b7162f6e6d82ae0d Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 29 Oct 2024 12:28:03 -0700 Subject: [PATCH 2/3] Add test for and remediate missing learner_needs key in public API data. Make remote view robust to missing ETag. --- kolibri/core/content/api.py | 10 +- kolibri/core/content/test/test_content_app.py | 110 ++++++++++++++++++ 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/kolibri/core/content/api.py b/kolibri/core/content/api.py index 0196a98affe..d4331f3c5d5 100644 --- a/kolibri/core/content/api.py +++ b/kolibri/core/content/api.py @@ -195,8 +195,9 @@ def _get_response_headers(self, response): return headers def _cache_etag(self, baseurl, headers): - cache_key = REMOTE_ETAG_CACHE_KEY.format(baseurl) - cache.set(cache_key, headers["Etag"], 3600) + if "Etag" in headers: + cache_key = REMOTE_ETAG_CACHE_KEY.format(baseurl) + cache.set(cache_key, headers["Etag"], 3600) def update_data(self, response_data, baseurl): return response_data @@ -792,6 +793,11 @@ def update_data(self, response_data, baseurl): response_data["admin_imported"] = ( response_data["id"] in self.locally_admin_imported_ids ) + if "learner_needs" not in response_data: + # We accidentally omitted learner_needs from previous versions + # of the public API, so we add it back in here, + # so that remote data and local data have consistent structure. + response_data["learner_needs"] = [] if "children" in response_data: response_data["children"] = self.update_data( response_data["children"], baseurl diff --git a/kolibri/core/content/test/test_content_app.py b/kolibri/core/content/test/test_content_app.py index 96250ed768b..232daa02951 100644 --- a/kolibri/core/content/test/test_content_app.py +++ b/kolibri/core/content/test/test_content_app.py @@ -1892,6 +1892,116 @@ def test_lesson_filter_multiple_assignment(self): self.assertEqual(len(response.data), 1) self.assertEqual(response.data[0]["content_id"], node.content_id) + def test_remote_content_node_missing_learner_needs(self): + with mock.patch("kolibri.core.content.api.NetworkClient") as nc: + mock_response = mock.Mock() + mock_response.headers = {} + mock_response.status_code = 200 + expected = content.ContentNode.objects.get(title="c2c2") + assessmentmetadata = ( + expected.assessmentmetadata.all() + .values( + "assessment_item_ids", + "number_of_assessments", + "mastery_model", + "randomize", + "is_manipulable", + "contentnode", + ) + .first() + ) + thumbnail = None + files = [] + for f in expected.files.all(): + "local_file__id", + "local_file__available", + "local_file__file_size", + "local_file__extension", + "lang_id", + file = {} + for field in [ + "id", + "priority", + "preset", + "supplementary", + "thumbnail", + ]: + file[field] = getattr(f, field) + file["checksum"] = f.local_file_id + for field in [ + "available", + "file_size", + "extension", + ]: + file[field] = getattr(f.local_file, field) + file["lang"] = self.map_language(f.lang) + file["storage_url"] = f.get_storage_url() + if self.baseurl and file["storage_url"]: + file["storage_url"] += "?baseurl={}".format(self.baseurl) + files.append(file) + if f.thumbnail: + thumbnail = f.get_storage_url() + if self.baseurl and thumbnail: + thumbnail += "?baseurl={}".format(self.baseurl) + expected_old_data = { + "id": expected.id, + "available": expected.available, + "author": expected.author, + "channel_id": expected.channel_id, + "coach_content": expected.coach_content, + "content_id": expected.content_id, + "description": expected.description, + "duration": expected.duration, + "learning_activities": expected.learning_activities.split(",") + if expected.learning_activities + else [], + "grade_levels": expected.grade_levels.split(",") + if expected.grade_levels + else [], + "resource_types": expected.resource_types.split(",") + if expected.resource_types + else [], + "accessibility_labels": expected.accessibility_labels.split(",") + if expected.accessibility_labels + else [], + "categories": expected.categories.split(",") + if expected.categories + else [], + "kind": expected.kind, + "lang": self.map_language(expected.lang), + "license_description": expected.license_description, + "license_name": expected.license_name, + "license_owner": expected.license_owner, + "num_coach_contents": expected.num_coach_contents, + "options": expected.options, + "parent": expected.parent_id, + "sort_order": expected.sort_order, + "title": expected.title, + "lft": expected.lft, + "rght": expected.rght, + "tree_id": expected.tree_id, + "ancestors": [], + "tags": list( + expected.tags.all() + .order_by("tag_name") + .values_list("tag_name", flat=True) + ), + "thumbnail": thumbnail, + "assessmentmetadata": assessmentmetadata, + "is_leaf": expected.kind != "topic", + "files": files, + "admin_imported": bool(expected.admin_imported), + } + mock_response.json.return_value = expected_old_data + mock_client = mock.MagicMock() + mock_client.get.return_value = mock_response + nc.build_for_address.return_value = mock_client + response = self.client.get( + reverse("kolibri:core:contentnode-detail", kwargs={"pk": expected.id}), + data={"baseurl": "http://example.com/"}, + ) + self.assertEqual(response.data["learner_needs"], []) + def tearDown(self): """ clean up files/folders created during the test From b48f3ac5e0a95a4c987e1412864ea4465b348ada Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 29 Oct 2024 12:47:08 -0700 Subject: [PATCH 3/3] Fix postgres specific test flake issue. --- kolibri/core/content/test/test_content_app.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kolibri/core/content/test/test_content_app.py b/kolibri/core/content/test/test_content_app.py index 232daa02951..3de78fd2451 100644 --- a/kolibri/core/content/test/test_content_app.py +++ b/kolibri/core/content/test/test_content_app.py @@ -320,6 +320,8 @@ def _assert_node(self, actual, expected): thumbnail = f.get_storage_url() if self.baseurl and thumbnail: thumbnail += "?baseurl={}".format(self.baseurl) + files = sorted(files, key=lambda x: x["id"]) + actual["files"] = sorted(actual["files"], key=lambda x: x["id"]) self.assertEqual( actual, {