Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: enable effort estimation feature by default

Browse Source 
Previously, effort estimation was disabled by default and gated by
an experiment flag (effort_estimation.location).

Now, it is enabled by default and courses can opt-out by adding
course waffle override flags (effort_estimation.disabled).
This commit is contained in:
Michael Terry
2021-07-27 10:58:10 -04:00
parent b2b03296bf
commit 721282233e
3 changed files with 27 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
openedx/features/effort_estimation/block_transformers.py
View File

@@ -13,7 +13,7 @@ from edxval.api import get_videos_for_course
from openedx.core.djangoapps.content.block_structure.transformer import BlockStructureTransformer
from openedx.core.lib.mobile_utils import is_request_from_mobile_app
from .toggles import EFFORT_ESTIMATION_LOCATION_FLAG
from .toggles import EFFORT_ESTIMATION_DISABLED_FLAG
class EffortEstimationTransformer(BlockStructureTransformer):
@@ -121,6 +121,10 @@ class EffortEstimationTransformer(BlockStructureTransformer):
block_structure.set_transformer_block_field(block_key, cls, cls.VIDEO_CLIP_DURATION, clip_duration)
def transform(self, usage_info, block_structure):
# Early exit if our per-course opt-out flag is enabled
if EFFORT_ESTIMATION_DISABLED_FLAG.is_enabled(block_structure.root_block_usage_key.course_key):
return
# Skip any transformation if our collection phase said to
cls = EffortEstimationTransformer
if block_structure.get_transformer_data(cls, cls.DISABLE_ESTIMATION, default=False):
@@ -154,28 +158,6 @@ class EffortEstimationTransformer(BlockStructureTransformer):
if activities is not None:
block_structure.override_xblock_field(block_key, self.EFFORT_ACTIVITIES, activities)
# Get bucket for this experiment. 0 is no estimate. 1 is only on sections. 2 is only on subsections.
# For cleanup ticket AA-659: remove everything below.
# We only want to get the bucket if there is data available - i.e. there is something to actually experiment
# on. This helps avoid rollout issues where we don't want to claim a user is in bucket 1 if we haven't even
# re-published the course so that it has any estimation data available.
root_key = block_structure.root_block_usage_key
total_activities = block_structure.get_xblock_field(root_key, self.EFFORT_ACTIVITIES)
total_time = block_structure.get_xblock_field(root_key, self.EFFORT_TIME)
if not total_activities and not total_time:
return
# Second pass to clear out collected estimate on levels we don't want to share. Just an easy way to test
# estimates at different levels, per the experiment.
bucket = EFFORT_ESTIMATION_LOCATION_FLAG.get_bucket(course_key=block_structure.root_block_usage_key.course_key)
for block_key in block_structure.post_order_traversal():
category = block_structure.get_xblock_field(block_key, 'category')
allowed = (bucket == 1 and category == 'chapter') or (bucket == 2 and category == 'sequential')
if not allowed:
block_structure.override_xblock_field(block_key, self.EFFORT_TIME, None)
block_structure.override_xblock_field(block_key, self.EFFORT_ACTIVITIES, None)
@cached_property
def _is_on_mobile(self):
"""Returns whether the current request is from our mobile app."""

45
openedx/features/effort_estimation/tests/test_block_transformers.py
View File

@@ -1,20 +1,19 @@
"""Tests for effort_estimation transformers."""
from datetime import timedelta
from unittest.mock import patch
from crum import set_current_request
from django.test.client import RequestFactory
from edx_toggles.toggles.testutils import override_waffle_flag
from edxval.api import create_video, remove_video_for_course
from lms.djangoapps.experiments.testutils import override_experiment_waffle_flag
from openedx.core.djangoapps.content.block_structure.factory import BlockStructureFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import SampleCourseFactory
from xmodule.modulestore.tests.sample_courses import BlockInfo
from ..block_transformers import EffortEstimationTransformer
from ..toggles import EFFORT_ESTIMATION_LOCATION_FLAG
from ..toggles import EFFORT_ESTIMATION_DISABLED_FLAG
# Copied here, rather than used directly from class, just to catch any accidental changes
@@ -108,9 +107,8 @@ class TestEffortEstimationTransformer(ModuleStoreTestCase):
def get_collection_field(self, key, name):
return self.block_structure.get_transformer_block_field(key, EffortEstimationTransformer, name)
def test_collection(self):
self.collect()
def assert_collected(self):
"""Confirm we at least collected the data (but not necessarily that we injected that data into block tree)"""
assert self.get_collection_field(self.video_clip_key, VIDEO_DURATION) == 200
assert self.get_collection_field(self.video_clip_key, VIDEO_CLIP_DURATION) == 40
assert self.get_collection_field(self.video_normal_key, VIDEO_DURATION) == 30
@@ -121,7 +119,10 @@ class TestEffortEstimationTransformer(ModuleStoreTestCase):
assert self.block_structure.get_transformer_data(EffortEstimationTransformer, DISABLE_ESTIMATION) is None
@override_experiment_waffle_flag(EFFORT_ESTIMATION_LOCATION_FLAG, bucket=1)
def test_collection(self):
self.collect()
self.assert_collected()
def test_incomplete_data_collection(self):
"""Ensure that missing video data prevents any estimates from being generated"""
remove_video_for_course(str(self.course_key), 'edxval3')
@@ -134,44 +135,22 @@ class TestEffortEstimationTransformer(ModuleStoreTestCase):
assert self.block_structure.get_xblock_field(self.subsection_key, EFFORT_ACTIVITIES) is None
assert self.block_structure.get_xblock_field(self.subsection_key, EFFORT_TIME) is None
@override_experiment_waffle_flag(EFFORT_ESTIMATION_LOCATION_FLAG, bucket=0)
def test_control_bucket(self):
@override_waffle_flag(EFFORT_ESTIMATION_DISABLED_FLAG, True)
def test_disabled(self):
self.collect_and_transform()
self.assert_collected()
assert self.block_structure.get_xblock_field(self.section_key, EFFORT_ACTIVITIES) is None
assert self.block_structure.get_xblock_field(self.section_key, EFFORT_TIME) is None
assert self.block_structure.get_xblock_field(self.subsection_key, EFFORT_ACTIVITIES) is None
assert self.block_structure.get_xblock_field(self.subsection_key, EFFORT_TIME) is None
@override_experiment_waffle_flag(EFFORT_ESTIMATION_LOCATION_FLAG, bucket=1)
def test_section_bucket(self):
def test_enabled(self):
self.collect_and_transform()
assert self.block_structure.get_xblock_field(self.section_key, EFFORT_ACTIVITIES) == 1
assert self.block_structure.get_xblock_field(self.section_key, EFFORT_TIME) == 121
assert self.block_structure.get_xblock_field(self.subsection_key, EFFORT_ACTIVITIES) is None
assert self.block_structure.get_xblock_field(self.subsection_key, EFFORT_TIME) is None
@override_experiment_waffle_flag(EFFORT_ESTIMATION_LOCATION_FLAG, bucket=2)
def test_subsection_bucket(self):
self.collect_and_transform()
assert self.block_structure.get_xblock_field(self.section_key, EFFORT_ACTIVITIES) is None
assert self.block_structure.get_xblock_field(self.section_key, EFFORT_TIME) is None
assert self.block_structure.get_xblock_field(self.subsection_key, EFFORT_ACTIVITIES) == 1
assert self.block_structure.get_xblock_field(self.subsection_key, EFFORT_TIME) == 121
def test_no_collection_no_bucket(self):
"""
Test that if we don't have any collection data, we don't bucket at all.
Useful to make sure that during rollout before we re-publish a course, we aren't trying to bucket anyone.
"""
where = 'openedx.features.effort_estimation.toggles.EFFORT_ESTIMATION_LOCATION_FLAG.get_bucket'
with patch(where, return_value=1) as mock_get_bucket:
self.transform() # no collection
assert self.block_structure.get_xblock_field(self.section_key, EFFORT_ACTIVITIES) is None
assert self.block_structure.get_xblock_field(self.section_key, EFFORT_TIME) is None
assert mock_get_bucket.call_count == 0
@override_experiment_waffle_flag(EFFORT_ESTIMATION_LOCATION_FLAG, bucket=1)
def test_mobile_video_support(self):
"""Clips values are ignored and web only videos should be excluded"""
self.set_mobile_request()

18
openedx/features/effort_estimation/toggles.py
View File

@@ -1,17 +1,19 @@
"""
Feature/experiment toggles used for effort estimation.
Feature toggles used for effort estimation.
"""
from edx_toggles.toggles import LegacyWaffleFlagNamespace
from lms.djangoapps.experiments.flags import ExperimentWaffleFlag
from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag
WAFFLE_FLAG_NAMESPACE = LegacyWaffleFlagNamespace(name='effort_estimation')
# Temporary flag while we test which location works best:
# - Bucket 0: off
# - Bucket 1: section (chapter) estimations
# - Bucket 2: subsection (sequential) estimations
EFFORT_ESTIMATION_LOCATION_FLAG = ExperimentWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'location', __name__, num_buckets=3, # lint-amnesty, pylint: disable=toggle-missing-annotation
use_course_aware_bucketing=False)
# .. toggle_name: effort_estimation.disabled
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: If effort estimations are confusing for a given course (e.g. the course team has added manual
# estimates), you can turn them off case by case here.
# .. toggle_use_cases: opt_out
# .. toggle_creation_date: 2021-07-27
EFFORT_ESTIMATION_DISABLED_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'disabled', __name__)