diff --git a/completion/services.py b/completion/services.py index 3effbd7..cf20bbc 100644 --- a/completion/services.py +++ b/completion/services.py @@ -103,6 +103,24 @@ def get_completable_children(self, node): user_children = [node] return user_children + @staticmethod + def matches_vertical_optional_completion(optional_vertical, optional_child): + """ + Checks if child should count towards a vertical's completion. + + There are only four combinations: + 1. Optional Vertical and Optional Child -> Include Child + 2. Optional Vertical and Required Child -> Include Child + 3. Required Vertical and Required Child -> Include Child + 4. Required Vertical and Optional Child -> Exclude Child + (case 2 shouldn't happen but this is how we can handle it gracefully) + + This means that the only case in which we want to exclude the child + is when the parent is required and the child isn't. + """ + required_vertical = not optional_vertical + return not (required_vertical and optional_child) # exclude case 4 from docstring + def vertical_is_complete(self, item): """ Calculates and returns whether a particular vertical is complete. @@ -122,9 +140,9 @@ def vertical_is_complete(self, item): # this is temporary local logic and will be removed when the whole course tree is included in completion child_locations = [ child.scope_ids.usage_id for child in self.get_completable_children(item) - # if vertical is optional, include all children - # else only include non-optional children - if optional_vertical or not getattr(child, "optional_completion", False) + if self.matches_vertical_optional_completion( + optional_vertical, getattr(child, "optional_completion", False) + ) ] completions = self.get_completions(child_locations) for child_location in child_locations: diff --git a/completion/tests/test_services.py b/completion/tests/test_services.py index 8eb5d2c..893334c 100644 --- a/completion/tests/test_services.py +++ b/completion/tests/test_services.py @@ -205,6 +205,20 @@ def test_blocks_to_mark_complete_on_view(self): [] ) + def test_matches_vertical_optional_completion(self): + for optional_vertical, optional_child, should_include_child in ( + (True, True, True), + (True, False, True), + (False, False, True), + (False, True, False), # do not count optional children for non-optional verticals + ): + self.assertEqual( + self.completion_service.matches_vertical_optional_completion( + optional_vertical, optional_child + ), + should_include_child, + ) + @ddt.ddt class CompletionDelayTestCase(CompletionSetUpMixin, TestCase):