From 67d6b9754f6a1da1307e42a6b9c4ed25b80977c2 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 19 Aug 2024 20:18:40 +0530 Subject: [PATCH 1/4] feat: add get_collections api --- openedx_learning/__init__.py | 2 +- openedx_learning/apps/authoring/collections/api.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index 354ad3724..cf87e3bf2 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -1,4 +1,4 @@ """ Open edX Learning ("Learning Core"). """ -__version__ = "0.11.1" +__version__ = "0.11.2" diff --git a/openedx_learning/apps/authoring/collections/api.py b/openedx_learning/apps/authoring/collections/api.py index 0539dc336..b8699d411 100644 --- a/openedx_learning/apps/authoring/collections/api.py +++ b/openedx_learning/apps/authoring/collections/api.py @@ -15,6 +15,7 @@ __all__ = [ "create_collection", "get_collection", + "get_collections", "get_learning_package_collections", "update_collection", ] @@ -78,3 +79,14 @@ def get_learning_package_collections(learning_package_id: int) -> QuerySet[Colle return Collection.objects \ .filter(learning_package_id=learning_package_id, enabled=True) \ .select_related("learning_package") + + +def get_collections(enabled: bool = True) -> QuerySet[Collection]: + """ + Get all collections, optionally caller can filter by enabled flag + """ + return ( + Collection.objects + .filter(enabled=enabled) + .select_related("learning_package") + ) From 005f616132687e8e12cca8bffb7daa34fa188e89 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 22 Aug 2024 11:10:45 +0530 Subject: [PATCH 2/4] refactor: get all collections by default allow users to get enabled & disabled collections together --- .../apps/authoring/collections/api.py | 11 +++-- .../apps/authoring/collections/test_api.py | 42 +++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/openedx_learning/apps/authoring/collections/api.py b/openedx_learning/apps/authoring/collections/api.py index b8699d411..48cb24d37 100644 --- a/openedx_learning/apps/authoring/collections/api.py +++ b/openedx_learning/apps/authoring/collections/api.py @@ -81,12 +81,11 @@ def get_learning_package_collections(learning_package_id: int) -> QuerySet[Colle .select_related("learning_package") -def get_collections(enabled: bool = True) -> QuerySet[Collection]: +def get_collections(enabled: bool | None = None) -> QuerySet[Collection]: """ Get all collections, optionally caller can filter by enabled flag """ - return ( - Collection.objects - .filter(enabled=enabled) - .select_related("learning_package") - ) + qs = Collection.objects.all() + if enabled is not None: + qs = qs.filter(enabled=enabled) + return qs.select_related("learning_package") diff --git a/tests/openedx_learning/apps/authoring/collections/test_api.py b/tests/openedx_learning/apps/authoring/collections/test_api.py index cf3555412..885f891cf 100644 --- a/tests/openedx_learning/apps/authoring/collections/test_api.py +++ b/tests/openedx_learning/apps/authoring/collections/test_api.py @@ -21,6 +21,7 @@ class CollectionTestCase(TestCase): Base-class for setting up commonly used test data. """ learning_package: LearningPackage + learning_package_2: LearningPackage now: datetime @classmethod @@ -29,6 +30,10 @@ def setUpTestData(cls) -> None: key="ComponentTestCase-test-key", title="Components Test Case Learning Package", ) + cls.learning_package_2 = publishing_api.create_learning_package( + key="ComponentTestCase-test-key-2", + title="Components Test Case another Learning Package", + ) cls.now = datetime(2024, 8, 5, tzinfo=timezone.utc) @@ -38,6 +43,7 @@ class GetCollectionTestCase(CollectionTestCase): """ collection1: Collection collection2: Collection + collection3: Collection disabled_collection: Collection @classmethod @@ -58,6 +64,12 @@ def setUpTestData(cls) -> None: title="Collection 2", description="Description of Collection 2", ) + cls.collection3 = collection_api.create_collection( + cls.learning_package_2.id, + created_by=None, + title="Collection 3", + description="Description of Collection 3", + ) cls.disabled_collection = collection_api.create_collection( cls.learning_package.id, created_by=None, @@ -98,6 +110,36 @@ def test_get_invalid_learning_package_collections(self): collections = collection_api.get_learning_package_collections(12345) assert not list(collections) + def test_get_all_collections(self): + """ + Test getting all collections. + """ + collections = collection_api.get_collections() + assert list(collections) == [ + self.collection1, + self.collection2, + self.collection3, + self.disabled_collection, + ] + + def test_get_all_enabled_collections(self): + """ + Test getting all ENABLED collections. + """ + collections = collection_api.get_collections(enabled=True) + assert list(collections) == [ + self.collection1, + self.collection2, + self.collection3, + ] + + def test_get_all_disabled_collections(self): + """ + Test getting all DISABLED collections. + """ + collections = collection_api.get_collections(enabled=False) + assert list(collections) == [self.disabled_collection] + class CollectionCreateTestCase(CollectionTestCase): """ From 2a0bc53bd09fe255df592cbb7bfe6253a88e18a6 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 22 Aug 2024 11:57:16 +0530 Subject: [PATCH 3/4] test: fix list assertion --- .../apps/authoring/collections/test_api.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/openedx_learning/apps/authoring/collections/test_api.py b/tests/openedx_learning/apps/authoring/collections/test_api.py index 885f891cf..a44a7d070 100644 --- a/tests/openedx_learning/apps/authoring/collections/test_api.py +++ b/tests/openedx_learning/apps/authoring/collections/test_api.py @@ -115,23 +115,23 @@ def test_get_all_collections(self): Test getting all collections. """ collections = collection_api.get_collections() - assert list(collections) == [ + self.assertQuerySetEqual(collections, [ self.collection1, self.collection2, self.collection3, self.disabled_collection, - ] + ], ordered=False) def test_get_all_enabled_collections(self): """ Test getting all ENABLED collections. """ collections = collection_api.get_collections(enabled=True) - assert list(collections) == [ + self.assertQuerySetEqual(collections, [ self.collection1, self.collection2, self.collection3, - ] + ], ordered=False) def test_get_all_disabled_collections(self): """ From cdc7dd213246df4b8896348d188e2d052c6b0fda Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 23 Aug 2024 15:55:03 +0530 Subject: [PATCH 4/4] refactor: order collections by primary key --- openedx_learning/apps/authoring/collections/api.py | 5 +++-- .../openedx_learning/apps/authoring/collections/test_api.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/openedx_learning/apps/authoring/collections/api.py b/openedx_learning/apps/authoring/collections/api.py index 48cb24d37..942903b73 100644 --- a/openedx_learning/apps/authoring/collections/api.py +++ b/openedx_learning/apps/authoring/collections/api.py @@ -78,7 +78,8 @@ def get_learning_package_collections(learning_package_id: int) -> QuerySet[Colle """ return Collection.objects \ .filter(learning_package_id=learning_package_id, enabled=True) \ - .select_related("learning_package") + .select_related("learning_package") \ + .order_by('pk') def get_collections(enabled: bool | None = None) -> QuerySet[Collection]: @@ -88,4 +89,4 @@ def get_collections(enabled: bool | None = None) -> QuerySet[Collection]: qs = Collection.objects.all() if enabled is not None: qs = qs.filter(enabled=enabled) - return qs.select_related("learning_package") + return qs.select_related("learning_package").order_by('pk') diff --git a/tests/openedx_learning/apps/authoring/collections/test_api.py b/tests/openedx_learning/apps/authoring/collections/test_api.py index a44a7d070..9f4eef70f 100644 --- a/tests/openedx_learning/apps/authoring/collections/test_api.py +++ b/tests/openedx_learning/apps/authoring/collections/test_api.py @@ -120,7 +120,7 @@ def test_get_all_collections(self): self.collection2, self.collection3, self.disabled_collection, - ], ordered=False) + ], ordered=True) def test_get_all_enabled_collections(self): """ @@ -131,7 +131,7 @@ def test_get_all_enabled_collections(self): self.collection1, self.collection2, self.collection3, - ], ordered=False) + ], ordered=True) def test_get_all_disabled_collections(self): """