Skip to content

Commit

Permalink
refactor: address PR reviews and implement unittests
Browse files Browse the repository at this point in the history
  • Loading branch information
mariajgrimaldi committed Nov 20, 2024
1 parent 1ad12f5 commit 5d0083b
Show file tree
Hide file tree
Showing 6 changed files with 222 additions and 18 deletions.
20 changes: 10 additions & 10 deletions openedx_learning/apps/authoring/containers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,9 @@ def create_next_defined_list(
)
)
EntityListRow.objects.bulk_create(new_rows)
return new_entity_list

def create_defined_list(
entity_list: EntityList,
def create_defined_list_with_rows(
entity_pks: list[int],
draft_version_pks: list[int | None],
published_version_pks: list[int | None],
Expand All @@ -140,6 +140,7 @@ def create_defined_list(
"""
order_nums = range(len(entity_pks))
with atomic():
entity_list = create_entity_list()
EntityListRow.objects.bulk_create(
[
EntityListRow(
Expand Down Expand Up @@ -178,8 +179,8 @@ def get_entity_list_with_pinned_versions(
entity_list=entity_list,
entity_id=row.entity.id,
order_num=row.order_num,
draft_version_id=row.entity.draft.version.pk,
published_version_id=row.entity.published.version.pk,
draft_version_id=None,
published_version_id=None, # For simplicity, we are not copying the pinned versions
)
for row in rows
]
Expand Down Expand Up @@ -222,10 +223,8 @@ def check_new_changes_in_defined_list(
True if there are new changes in the defined list, False otherwise.
"""
# Is there a way to short-circuit this? Using queryset operations
return any(
row.entity.pk not in publishable_entities_pk
for row in entity_list.entitylistrow_set.all()
)
# For simplicity, return True
return True


def create_container_version(
Expand Down Expand Up @@ -262,7 +261,7 @@ def create_container_version(
created=created,
created_by=created_by,
)
defined_list = create_defined_list(
defined_list = create_defined_list_with_rows(
entity_pks=publishable_entities_pk,
draft_version_pks=draft_version_pks,
published_version_pks=published_version_pks,
Expand All @@ -274,6 +273,7 @@ def create_container_version(
initial_list=defined_list,
# TODO: Check for unpinned versions in defined_list to know whether to point this to the defined_list
# point to None.
# If this is the first version ever created for this ContainerEntity, then start as None.
frozen_list=None,
)
return container_version
Expand Down Expand Up @@ -346,7 +346,7 @@ def create_next_container_version(
published_version_pks=published_version_pks,
)
next_initial_list = get_entity_list_with_pinned_versions(
rows=next_defined_list.frozen_list.entitylistrow_set.all()
rows=next_defined_list.entitylistrow_set.all()
)
if check_unpinned_versions_in_defined_list(next_defined_list):
next_frozen_list = None
Expand Down
18 changes: 10 additions & 8 deletions openedx_learning/apps/authoring/units/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def create_unit_version(
draft_version_pks: list[int | None],
published_version_pks: list[int | None],
created: datetime,
created_by: int | None,
created_by: int | None = None,
) -> Unit:
"""Create a new unit version.
Expand Down Expand Up @@ -98,7 +98,7 @@ def create_next_unit_version(
draft_version_pks: list[int | None],
published_version_pks: list[int | None],
created: datetime,
created_by: int | None,
created_by: int | None = None,
) -> Unit:
"""Create the next unit version.
Expand Down Expand Up @@ -134,11 +134,11 @@ def create_next_unit_version(
def create_unit_and_version(
learning_package_id: int,
key: str,
created: datetime,
created_by: int | None,
title: str,
created: datetime,
created_by: int | None = None,
) -> tuple[Unit, UnitVersion]:
"""Create a new unit and version.
"""Create a new unit and its version.
Args:
learning_package_id: The learning package ID.
Expand All @@ -153,6 +153,8 @@ def create_unit_and_version(
1,
title,
[],
[],
[],
created,
created_by,
)
Expand Down Expand Up @@ -193,7 +195,7 @@ def get_user_defined_list_in_unit_version(unit_version_pk: int) -> QuerySet[Enti
unit_version_pk: The unit version ID.
"""
unit_version = UnitVersion.objects.get(pk=unit_version_pk)
return container_api.get_defined_list_for_container_version(unit_version.container_entity_version)
return container_api.get_defined_list_rows_for_container_version(unit_version.container_entity_version)


def get_initial_list_in_unit_version(unit_version_pk: int) -> list[int]:
Expand All @@ -203,7 +205,7 @@ def get_initial_list_in_unit_version(unit_version_pk: int) -> list[int]:
unit_version_pk: The unit version ID.
"""
unit_version = UnitVersion.objects.get(pk=unit_version_pk)
return container_api.get_initial_list_for_container_version(unit_version.container_entity_version)
return container_api.get_initial_list_rows_for_container_version(unit_version.container_entity_version)


def get_frozen_list_in_unit_version(unit_version_pk: int) -> list[int]:
Expand All @@ -213,4 +215,4 @@ def get_frozen_list_in_unit_version(unit_version_pk: int) -> list[int]:
unit_version_pk: The unit version ID.
"""
unit_version = UnitVersion.objects.get(pk=unit_version_pk)
return container_api.get_frozen_list_for_container_version(unit_version.container_entity_version)
return container_api.get_frozen_list_rows_for_container_version(unit_version.container_entity_version)
2 changes: 2 additions & 0 deletions projects/dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
"openedx_learning.apps.authoring.components.apps.ComponentsConfig",
"openedx_learning.apps.authoring.contents.apps.ContentsConfig",
"openedx_learning.apps.authoring.publishing.apps.PublishingConfig",
"openedx_learning.apps.authoring.containers.apps.ContainersConfig",
"openedx_learning.apps.authoring.units.apps.UnitsConfig",
# Learning Contrib Apps
"openedx_learning.contrib.media_server.apps.MediaServerConfig",
# Apps that don't belong in this repo in the long term, but are here to make
Expand Down
2 changes: 2 additions & 0 deletions test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ def root(*args):
"openedx_learning.apps.authoring.contents.apps.ContentsConfig",
"openedx_learning.apps.authoring.publishing.apps.PublishingConfig",
"openedx_tagging.core.tagging.apps.TaggingConfig",
"openedx_learning.apps.authoring.containers.apps.ContainersConfig",
"openedx_learning.apps.authoring.units.apps.UnitsConfig",
]

AUTHENTICATION_BACKENDS = [
Expand Down
Empty file.
198 changes: 198 additions & 0 deletions tests/openedx_learning/apps/authoring/units/test_api.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
"""
Basic tests for the units API.
"""
from ..components.test_api import ComponentTestCase
from openedx_learning.api import authoring as authoring_api


class UnitTestCase(ComponentTestCase):

def setUp(self) -> None:
self.component_1, self.component_version_1 = authoring_api.create_component_and_version(
self.learning_package.id,
component_type=self.problem_type,
local_key="Query Counting",
title="Querying Counting Problem",
created=self.now,
created_by=None,
)
self.component_2, self.component_version_2 = authoring_api.create_component_and_version(
self.learning_package.id,
component_type=self.problem_type,
local_key="Query Counting (2)",
title="Querying Counting Problem (2)",
created=self.now,
created_by=None,
)

def test_create_unit_with_content_instead_of_components(self):
"""Test creating a unit with content instead of components.
Expected results:
1. An error is raised indicating the content restriction for units.
2. The unit is not created.
"""

def test_create_empty_first_unit_and_version(self):
"""Test creating a unit with no components.
Expected results:
1. A unit and unit version are created.
2. The unit version number is 1.
3. The unit version is in the unit's versions.
"""
unit, unit_version = authoring_api.create_unit_and_version(
learning_package_id=self.learning_package.id,
key=f"unit:key",
title="Unit",
created=self.now,
created_by=None,
)
assert unit, unit_version
assert unit_version.version_num == 1
assert unit_version in unit.versioning.versions.all()

def test_create_next_unit_version_with_two_components(self):
"""Test creating a unit version with two components.
Expected results:
1. A new unit version is created.
2. The unit version number is 2.
3. The unit version is in the unit's versions.
4. The components are in the unit version's user defined list.
5. Initial list contains the pinned versions of the defined list.
6. Frozen list is empty.
"""
unit, unit_version = authoring_api.create_unit_and_version(
learning_package_id=self.learning_package.id,
key=f"unit:key",
title="Unit",
created=self.now,
created_by=None,
)
unit_version_v2 = authoring_api.create_next_unit_version(
unit=unit,
title="Unit",
publishable_entities_pks=[
self.component_1.publishable_entity.id,
self.component_2.publishable_entity.id,
],
draft_version_pks=[None, None],
published_version_pks=[None, None],
created=self.now,
created_by=None,
)
assert unit_version_v2.version_num == 2
assert unit_version_v2 in unit.versioning.versions.all()
publishable_entities_in_list = [
row.entity for row in authoring_api.get_user_defined_list_in_unit_version(unit_version_v2.pk)
]
assert self.component_1.publishable_entity in publishable_entities_in_list
assert self.component_2.publishable_entity in publishable_entities_in_list


def test_next_version_with_different_different_title(self):
"""Test creating a unit version with a different title.
Expected results:
1. A new unit version is created.
2. The unit version number is 2.
3. The unit version is in the unit's versions.
4. The unit version's title is different from the previous version.
5. The user defined is the same as the previous version.
6. The frozen list is empty.
"""

def test_create_two_units_with_same_components(self):
"""Test creating two units with the same components.
Expected results:
1. Two different units are created.
2. The units have the same components.
"""

def test_check_author_defined_list_matches_components(self):
"""Test checking the author defined list matches the components.
Expected results:
1. The author defined list matches the components used to create the unit version.
"""

def test_check_initial_list_matches_components(self):
"""Test checking the initial list matches the components.
Expected results:
1. The initial list matches the components (pinned) used to create the unit version.
"""

def test_check_frozen_list_is_none_floating_versions(self):
"""Test checking the frozen list is None when floating versions are used in the author defined list.
Expected results:
1. The frozen list is None.
"""

def test_check_frozen_list_when_next_version_is_created(self):
"""Test checking the frozen list when a new version is created.
Expected results:
1. The frozen list has pinned versions of the user defined list from the previous version.
"""

def test_check_lists_equal_when_pinned_versions(self):
"""Test checking the lists are equal when pinned versions are used.
Expected results:
1. The author defined list == initial list == frozen list.
"""

def test_publish_unit_version(self):
"""Test publish unpublished unit version.
Expected results:
1. The newly created unit version has unpublished changes.
2. The published version matches the unit version.
3. The draft version matches the unit version.
"""

def test_publish_unit_with_unpublished_component(self):
"""Test publishing a unit with an unpublished component.
Expected results:
1. The unit version is published.
2. The component is published.
"""

def test_next_version_with_different_order(self):
"""Test creating a unit version with different order of components.
Expected results:
1. A new unit version is created.
2. The unit version number is 2.
3. The unit version is in the unit's versions.
4. The user defined list is different from the previous version.
5. The initial list contains the pinned versions of the defined list.
6. The frozen list is empty.
"""

def test_soft_delete_component_from_units(self):
"""Soft-delete a component from a unit.
Expected result:
After soft-deleting the component (draft), a new unit version (draft) is created for the unit.
"""

def test_soft_delete_component_from_units_and_publish(self):
"""Soft-delete a component from a unit and publish the unit.
Expected result:
After soft-deleting the component (draft), a new unit version (draft) is created for the unit.
Then, if the unit is published all units referencing the component are published as well.
"""

def test_unit_version_becomes_draft_again(self):
"""Test a unit version becomes a draft again.
Expected results:
1. The frozen list is None after the unit version becomes a draft again.
"""

0 comments on commit 5d0083b

Please sign in to comment.