Skip to content

Commit

Permalink
feat: mark optional completions
Browse files Browse the repository at this point in the history
  • Loading branch information
DanielVZ96 committed Feb 17, 2024
1 parent 9cb9860 commit b7ce759
Show file tree
Hide file tree
Showing 11 changed files with 164 additions and 26 deletions.
12 changes: 8 additions & 4 deletions completion_aggregator/api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,8 @@ class CompletionDetailView(CompletionViewMixin, APIView):
"completion": {
"earned": 12.0,
"possible": 24.0,
"ratio": 0.5
"ratio": 0.5,
"optional": false
},
"mean": 0.25,
"chapter": [
Expand All @@ -325,7 +326,8 @@ class CompletionDetailView(CompletionViewMixin, APIView):
"completion": {
"earned: 12.0,
"possible": 24.0,
"ratio": 0.5
"ratio": 0.5,
"optional": false
}
}
],
Expand All @@ -337,7 +339,8 @@ class CompletionDetailView(CompletionViewMixin, APIView):
"completion": {
"earned: 12.0,
"possible": 12.0,
"ratio": 1.0
"ratio": 1.0,
"optional": false
}
},
{
Expand All @@ -347,7 +350,8 @@ class CompletionDetailView(CompletionViewMixin, APIView):
"completion": {
"earned: 0.0,
"possible": 12.0,
"ratio": 0.0
"ratio": 0.0,
"optional": false
}
},
},
Expand Down
11 changes: 11 additions & 0 deletions completion_aggregator/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,17 @@ def get_block_aggregators(course_blocks, block):
) or []


def is_block_optional(course_blocks, block):
"""
Returns whether a block is optional or not.
"""
return course_blocks.get_transformer_block_field(
block,
AggregatorAnnotationTransformer,
AggregatorAnnotationTransformer.OPTIONAL_CONTENT
)


def get_mobile_only_courses(enrollments):
"""
Return list of courses with mobile available given a list of enrollments.
Expand Down
34 changes: 21 additions & 13 deletions completion_aggregator/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
UPDATER_CACHE_TIMEOUT = 600 # 10 minutes

CacheEntry = namedtuple('CacheEntry', ['course_blocks', 'root_block'])
CompletionStats = namedtuple('CompletionStats', ['earned', 'possible', 'last_modified'])
CompletionStats = namedtuple('CompletionStats', ['earned', 'possible', 'last_modified', 'optional'])

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -84,7 +84,7 @@ def cache_key(self):
)


CourseBlocksEntry = namedtuple('CourseBlocksEntry', ['children', 'aggregators'])
CourseBlocksEntry = namedtuple('CourseBlocksEntry', ['children', 'aggregators', 'optional'])


class AggregationUpdater:
Expand Down Expand Up @@ -139,7 +139,8 @@ def format_course_blocks(self, course_blocks, root_block):
{
block_key: CourseBlocksEntry(
children=block_key[],
aggregators=block_key[]
aggregators=block_key[],
optional=boolean,
)
}
Expand All @@ -151,6 +152,7 @@ def populate(structure, block):
structure[block] = CourseBlocksEntry(
children=compat.get_children(course_blocks, block),
aggregators=compat.get_block_aggregators(course_blocks, block),
optional=compat.is_block_optional(course_blocks, block),
)
for child in structure[block].children:
populate(structure, child)
Expand Down Expand Up @@ -215,7 +217,7 @@ def update(self, changed_blocks=frozenset(), force=False):
Aggregator.objects.bulk_create_or_update(updated_aggregators)
self.resolve_stale_completions(changed_blocks, start)

def update_for_block(self, block, affected_aggregators, force=False):
def update_for_block(self, block, affected_aggregators, force=False, optional_branch=False):
"""
Recursive function to perform updates for a given block.
Expand All @@ -229,26 +231,29 @@ def update_for_block(self, block, affected_aggregators, force=False):
if mode == XBlockCompletionMode.EXCLUDED:
return self.update_for_excluded()
elif mode == XBlockCompletionMode.COMPLETABLE:
return self.update_for_completable(block)
return self.update_for_completable(block, optional_branch)
elif mode == XBlockCompletionMode.AGGREGATOR:
return self.update_for_aggregator(block, affected_aggregators, force)
return self.update_for_aggregator(block, affected_aggregators, force, optional_branch)
else:
raise ValueError(f"Invalid completion mode {mode}")

def update_for_aggregator(self, block, affected_aggregators, force):
def update_for_aggregator(self, block, affected_aggregators, force, optional_branch=False):
"""
Calculate the new completion values for an aggregator.
"""
total_earned = 0.0
total_possible = 0.0
last_modified = OLD_DATETIME

optional = self.course_blocks[block].optional or optional_branch
if block not in affected_aggregators:
obj = self.aggregators.get(block)
if obj:
return CompletionStats(earned=obj.earned, possible=obj.possible, last_modified=obj.last_modified)
return CompletionStats(earned=obj.earned, possible=obj.possible, last_modified=obj.last_modified, optional=optional)
for child in self.course_blocks[block].children:
(earned, possible, modified) = self.update_for_block(child, affected_aggregators, force)
(earned, possible, modified, is_optional_child) = self.update_for_block(child, affected_aggregators, force, optional_branch=optional)
if is_optional_child and not optional:
continue # Optional children shouldn't count towards non-optional parents aggregations
total_earned += earned
total_possible += possible
if modified is not None:
Expand All @@ -270,6 +275,7 @@ def update_for_aggregator(self, block, affected_aggregators, force):
percent=percent,
last_modified=last_modified,
modified=timezone.now(),
optional=optional,
)
self.aggregators[block] = aggregator
else:
Expand All @@ -279,27 +285,29 @@ def update_for_aggregator(self, block, affected_aggregators, force):
aggregator.percent = percent
aggregator.last_modified = last_modified
aggregator.modified = timezone.now()
aggregator.optional = optional
self.updated_aggregators.append(aggregator)
return CompletionStats(earned=total_earned, possible=total_possible, last_modified=last_modified)
return CompletionStats(earned=total_earned, possible=total_possible, last_modified=last_modified, optional=optional)

def update_for_excluded(self):
"""
Return a sentinel empty completion value for excluded blocks.
"""
return CompletionStats(earned=0.0, possible=0.0, last_modified=OLD_DATETIME)
return CompletionStats(earned=0.0, possible=0.0, last_modified=OLD_DATETIME, optional=False)

def update_for_completable(self, block):
def update_for_completable(self, block, optional_branch=False):
"""
Return the block completion value for a given completable block.
"""
completion = self.block_completions.get(block)
optional = self.course_blocks[block].optional
if completion:
earned = completion.completion
last_modified = completion.modified
else:
earned = 0.0
last_modified = OLD_DATETIME
return CompletionStats(earned=earned, possible=1.0, last_modified=last_modified)
return CompletionStats(earned=earned, possible=1.0, last_modified=last_modified, optional=optional or optional_branch)

def _aggregator_needs_update(self, block, modified, force):
"""
Expand Down
18 changes: 18 additions & 0 deletions completion_aggregator/migrations/0006_aggregator_optional.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 3.2.24 on 2024-02-16 00:11

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('completion_aggregator', '0005_cachegroupinvalidation'),
]

operations = [
migrations.AddField(
model_name='aggregator',
name='optional',
field=models.BooleanField(default=False),
),
]
5 changes: 4 additions & 1 deletion completion_aggregator/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def pre_save(sender, instance, **kwargs): # pylint: disable=unused-argument
instance.full_clean()

def submit_completion(self, user, course_key, block_key, aggregation_name, earned, possible,
last_modified):
last_modified, optional=False):
"""
Insert and Update the completion Aggregator for the specified record.
Expand Down Expand Up @@ -169,6 +169,7 @@ def submit_completion(self, user, course_key, block_key, aggregation_name, earne
'possible': possible,
'earned': earned,
'last_modified': last_modified,
'optional': optional,
},
)
return obj, is_new
Expand All @@ -190,6 +191,7 @@ def bulk_create_or_update(self, updated_aggregators):
possible=aggregator.possible,
earned=aggregator.earned,
last_modified=aggregator.last_modified,
optional=aggregator.optional,
)
else:
aggregation_data = [obj.get_values() for obj in updated_aggregators]
Expand All @@ -211,6 +213,7 @@ class Aggregator(TimeStampedModel):
possible = models.FloatField(validators=[validate_positive_float])
percent = models.FloatField(validators=[validate_percent])
last_modified = models.DateTimeField()
optional = models.BooleanField(default=False)

objects = AggregatorManager()

Expand Down
2 changes: 2 additions & 0 deletions completion_aggregator/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ def course(self):
earned=0.0,
possible=None,
percent=0.0,
optional=False,
)

@property
Expand Down Expand Up @@ -210,6 +211,7 @@ class _CompletionSerializer(serializers.Serializer):
earned = serializers.FloatField()
possible = serializers.FloatField()
percent = serializers.FloatField()
optional = serializers.BooleanField(default=False)


class _CompletionSerializerV0(_CompletionSerializer):
Expand Down
12 changes: 9 additions & 3 deletions completion_aggregator/transformers.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class AggregatorAnnotationTransformer(BlockStructureTransformer):
READ_VERSION = 1
WRITE_VERSION = 1
AGGREGATORS = "aggregators"
OPTIONAL_CONTENT = "optional_content"

@classmethod
def name(cls):
Expand Down Expand Up @@ -48,23 +49,27 @@ def collect(cls, block_structure):
"""
Collect the data required to perform this calculation.
"""
block_structure.request_xblock_fields("completion_mode")
block_structure.request_xblock_fields("completion_mode", "optional_content")

def calculate_aggregators(self, block_structure, block_key):
"""
Calculate the set of aggregators for the specified block.
"""
aggregators = set()
optional = False
parents = block_structure.get_parents(block_key)
for parent in parents:
parent_block = block_structure[parent]
completion_mode = getattr(parent_block, 'completion_mode', XBlockCompletionMode.COMPLETABLE)
optional_parent = getattr(parent_block, 'optional_content', False)
if completion_mode == XBlockCompletionMode.EXCLUDED:
continue
if completion_mode == XBlockCompletionMode.AGGREGATOR:
aggregators.add(parent)
if optional_parent:
optional = True
aggregators.update(self.get_block_aggregators(block_structure, parent))
return aggregators
return aggregators, optional

def transform(self, usage_info, block_structure): # pylint: disable=unused-argument
"""
Expand All @@ -77,5 +82,6 @@ def transform(self, usage_info, block_structure): # pylint: disable=unused-argu
XBlockCompletionMode.COMPLETABLE
)
if completion_mode != XBlockCompletionMode.EXCLUDED:
aggregators = self.calculate_aggregators(block_structure, block_key)
aggregators, optional = self.calculate_aggregators(block_structure, block_key)
block_structure.set_transformer_block_field(block_key, self, self.AGGREGATORS, aggregators)
block_structure.set_transformer_block_field(block_key, self, self.OPTIONAL_CONTENT, optional)
6 changes: 6 additions & 0 deletions test_utils/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ def get_block_aggregators(self, course_blocks, block):

return [agg for agg in course_blocks.blocks if block.block_id.startswith(f'{agg.block_id}-')]

def is_block_optional(self, course_blocks, block):
"""
Returns whether a block is optional or not.
"""
return "optional" in block.block_id or "optional" in block.offering

def get_block_completions(self, user, course_key):
"""
Return all completions for the current course.
Expand Down
Loading

0 comments on commit b7ce759

Please sign in to comment.