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 582c238
Show file tree
Hide file tree
Showing 11 changed files with 178 additions and 30 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):
"""
Return whether a block is optional or not.
"""
return course_blocks.get_transformer_block_field(

Check warning on line 143 in completion_aggregator/compat.py

View check run for this annotation

Codecov / codecov/patch

completion_aggregator/compat.py#L143

Added line #L143 was not covered by tests
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
42 changes: 29 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,33 @@ 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(

Check warning on line 252 in completion_aggregator/core.py

View check run for this annotation

Codecov / codecov/patch

completion_aggregator/core.py#L252

Added line #L252 was not covered by tests
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 +279,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 +289,33 @@ 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

Check warning on line 3 in completion_aggregator/migrations/0006_aggregator_optional.py

View check run for this annotation

Codecov / codecov/patch

completion_aggregator/migrations/0006_aggregator_optional.py#L3

Added line #L3 was not covered by tests


class Migration(migrations.Migration):

Check warning on line 6 in completion_aggregator/migrations/0006_aggregator_optional.py

View check run for this annotation

Codecov / codecov/patch

completion_aggregator/migrations/0006_aggregator_optional.py#L6

Added line #L6 was not covered by tests

dependencies = [

Check warning on line 8 in completion_aggregator/migrations/0006_aggregator_optional.py

View check run for this annotation

Codecov / codecov/patch

completion_aggregator/migrations/0006_aggregator_optional.py#L8

Added line #L8 was not covered by tests
('completion_aggregator', '0005_cachegroupinvalidation'),
]

operations = [

Check warning on line 12 in completion_aggregator/migrations/0006_aggregator_optional.py

View check run for this annotation

Codecov / codecov/patch

completion_aggregator/migrations/0006_aggregator_optional.py#L12

Added line #L12 was not covered by tests
migrations.AddField(
model_name='aggregator',
name='optional',
field=models.BooleanField(default=False),
),
]
14 changes: 9 additions & 5 deletions completion_aggregator/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,17 @@

INSERT_OR_UPDATE_AGGREGATOR_QUERY = """
INSERT INTO completion_aggregator_aggregator
(user_id, course_key, block_key, aggregation_name, earned, possible, percent, last_modified, created, modified)
(user_id, course_key, block_key, aggregation_name, earned, possible, percent, last_modified, created, modified, optional)
VALUES
(%(user)s, %(course_key)s, %(block_key)s, %(aggregation_name)s, %(earned)s,
%(possible)s, %(percent)s, %(last_modified)s, %(created)s, %(modified)s)
%(possible)s, %(percent)s, %(last_modified)s, %(created)s, %(modified)s, %(optional)s)
ON DUPLICATE KEY UPDATE
earned=VALUES(earned),
possible=VALUES(possible),
percent=VALUES(percent),
last_modified=VALUES(last_modified),
modified=VALUES(modified);
modified=VALUES(modified),
optional=VALUES(optional);
"""


Expand Down Expand Up @@ -100,7 +101,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 +170,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 +192,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 +214,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 Expand Up @@ -239,7 +243,7 @@ def get_values(self):
Return a dict object containing fields and their values to be used in bulk create or update query.
"""
values = {key: getattr(self, key) for key in [
'course_key', 'block_key', 'aggregation_name', 'earned', 'possible',
'course_key', 'block_key', 'aggregation_name', 'earned', 'possible', 'optional'
]}
values['user'] = self.user.id
values['percent'] = get_percent(values['earned'], values['possible'])
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")

Check warning on line 52 in completion_aggregator/transformers.py

View check run for this annotation

Codecov / codecov/patch

completion_aggregator/transformers.py#L52

Added line #L52 was not covered by tests

def calculate_aggregators(self, block_structure, block_key):
"""
Calculate the set of aggregators for the specified block.
"""
aggregators = set()
optional = False

Check warning on line 59 in completion_aggregator/transformers.py

View check run for this annotation

Codecov / codecov/patch

completion_aggregator/transformers.py#L59

Added line #L59 was not covered by tests
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)

Check warning on line 64 in completion_aggregator/transformers.py

View check run for this annotation

Codecov / codecov/patch

completion_aggregator/transformers.py#L64

Added line #L64 was not covered by tests
if completion_mode == XBlockCompletionMode.EXCLUDED:
continue
if completion_mode == XBlockCompletionMode.AGGREGATOR:
aggregators.add(parent)
if optional_parent:
optional = True

Check warning on line 70 in completion_aggregator/transformers.py

View check run for this annotation

Codecov / codecov/patch

completion_aggregator/transformers.py#L70

Added line #L70 was not covered by tests
aggregators.update(self.get_block_aggregators(block_structure, parent))
return aggregators
return aggregators, optional

Check warning on line 72 in completion_aggregator/transformers.py

View check run for this annotation

Codecov / codecov/patch

completion_aggregator/transformers.py#L72

Added line #L72 was not covered by tests

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)

Check warning on line 85 in completion_aggregator/transformers.py

View check run for this annotation

Codecov / codecov/patch

completion_aggregator/transformers.py#L85

Added line #L85 was not covered by tests
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)

Check warning on line 87 in completion_aggregator/transformers.py

View check run for this annotation

Codecov / codecov/patch

completion_aggregator/transformers.py#L87

Added line #L87 was not covered by tests
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, _, block):
"""
Return whether a block is optional or not.
"""
return "optional" in block.block_id or "optional" in block.run

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

0 comments on commit 582c238

Please sign in to comment.