From 75f693700396cc90695aa4bd873704bd46b91acd Mon Sep 17 00:00:00 2001 From: Dillon Dumesnil Date: Tue, 28 Jul 2020 13:56:08 -0700 Subject: [PATCH] AA-261: Overridding Show Answer functionality for PLS courses We noticed that since users can choose to reset their due dates, they would have the ability to let due dates pass and then for any assessment that allows viewing the answer after the due date would be visible. The user could thus view all answers and then reset their due dates to receive a perfect score. This PR works to fix that issue by changing all show answer values to not take into account being past the due date when inside a PLS course. --- common/lib/xmodule/xmodule/capa_base.py | 23 +++++-- .../xmodule/modulestore/inheritance.py | 3 +- lms/envs/production.py | 5 ++ .../__init__.py | 0 .../show_answer/__init__.py | 0 .../show_answer/show_answer_field_override.py | 56 ++++++++++++++++ .../show_answer/tests/__init__.py | 0 .../tests/test_show_answer_override.py | 67 +++++++++++++++++++ 8 files changed, 149 insertions(+), 5 deletions(-) create mode 100644 openedx/features/personalized_learner_schedules/__init__.py create mode 100644 openedx/features/personalized_learner_schedules/show_answer/__init__.py create mode 100644 openedx/features/personalized_learner_schedules/show_answer/show_answer_field_override.py create mode 100644 openedx/features/personalized_learner_schedules/show_answer/tests/__init__.py create mode 100644 openedx/features/personalized_learner_schedules/show_answer/tests/test_show_answer_override.py diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index 804fba7197..3d52de8a5e 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -64,6 +64,9 @@ class SHOWANSWER(object): PAST_DUE = "past_due" NEVER = "never" AFTER_SOME_NUMBER_OF_ATTEMPTS = "after_attempts" + AFTER_ALL_ATTEMPTS = "after_all_attempts" + AFTER_ALL_ATTEMPTS_OR_CORRECT = "after_all_attempts_or_correct" + ATTEMPTED_NO_PAST_DUE = "attempted_no_past_due" class RANDOMIZATION(object): @@ -167,13 +170,16 @@ class CapaFields(object): values=[ {"display_name": _("Always"), "value": SHOWANSWER.ALWAYS}, {"display_name": _("Answered"), "value": SHOWANSWER.ANSWERED}, - {"display_name": _("Attempted"), "value": SHOWANSWER.ATTEMPTED}, + {"display_name": _("Attempted or Past Due"), "value": SHOWANSWER.ATTEMPTED}, {"display_name": _("Closed"), "value": SHOWANSWER.CLOSED}, {"display_name": _("Finished"), "value": SHOWANSWER.FINISHED}, {"display_name": _("Correct or Past Due"), "value": SHOWANSWER.CORRECT_OR_PAST_DUE}, {"display_name": _("Past Due"), "value": SHOWANSWER.PAST_DUE}, {"display_name": _("Never"), "value": SHOWANSWER.NEVER}, {"display_name": _("After Some Number of Attempts"), "value": SHOWANSWER.AFTER_SOME_NUMBER_OF_ATTEMPTS}, + {"display_name": _("After All Attempts"), "value": SHOWANSWER.AFTER_ALL_ATTEMPTS}, + {"display_name": _("After All Attempts or Correct"), "value": SHOWANSWER.AFTER_ALL_ATTEMPTS_OR_CORRECT}, + {"display_name": _("Attempted"), "value": SHOWANSWER.ATTEMPTED_NO_PAST_DUE}, ] ) attempts_before_showanswer_button = Integer( @@ -877,6 +883,10 @@ class CapaMixin(ScorableXBlockMixin, CapaFields): hint_index = int(data['hint_index']) return self.get_demand_hint(hint_index) + def used_all_attempts(self): + """ All attempts have been used """ + return self.max_attempts is not None and self.attempts >= self.max_attempts + def is_past_due(self): """ Is it now past this problem's due date, including grace period? @@ -888,7 +898,7 @@ class CapaMixin(ScorableXBlockMixin, CapaFields): """ Is the student still allowed to submit answers? """ - if self.max_attempts is not None and self.attempts >= self.max_attempts: + if self.used_all_attempts(): return True if self.is_past_due(): return True @@ -936,7 +946,7 @@ class CapaMixin(ScorableXBlockMixin, CapaFields): # unless the problem explicitly prevents it return True elif self.showanswer == SHOWANSWER.ATTEMPTED: - return self.attempts > 0 or self.is_past_due() + return self.is_attempted() or self.is_past_due() elif self.showanswer == SHOWANSWER.ANSWERED: # NOTE: this is slightly different from 'attempted' -- resetting the problems # makes lcp.done False, but leaves attempts unchanged. @@ -957,7 +967,12 @@ class CapaMixin(ScorableXBlockMixin, CapaFields): return self.attempts >= required_attempts elif self.showanswer == SHOWANSWER.ALWAYS: return True - + elif self.showanswer == SHOWANSWER.AFTER_ALL_ATTEMPTS: + return self.used_all_attempts() + elif self.showanswer == SHOWANSWER.AFTER_ALL_ATTEMPTS_OR_CORRECT: + return self.used_all_attempts() or self.is_correct() + elif self.showanswer == SHOWANSWER.ATTEMPTED_NO_PAST_DUE: + return self.is_attempted() return False def correctness_available(self): diff --git a/common/lib/xmodule/xmodule/modulestore/inheritance.py b/common/lib/xmodule/xmodule/modulestore/inheritance.py index 49217e30ee..107c25f007 100644 --- a/common/lib/xmodule/xmodule/modulestore/inheritance.py +++ b/common/lib/xmodule/xmodule/modulestore/inheritance.py @@ -84,7 +84,8 @@ class InheritanceMixin(XBlockMixin): # specific words for the acceptable values. 'Specify when the Show Answer button appears for each problem. ' 'Valid values are "always", "answered", "attempted", "closed", ' - '"finished", "past_due", "correct_or_past_due", and "never".' + '"finished", "past_due", "correct_or_past_due", "after_all_attempts", ' + '"after_all_attempts_or_correct", "attempted_no_past_due", and "never".' ), scope=Scope.settings, default="finished", diff --git a/lms/envs/production.py b/lms/envs/production.py index 239d880941..c1533874a2 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -738,6 +738,11 @@ if FEATURES.get('INDIVIDUAL_DUE_DATES'): 'courseware.student_field_overrides.IndividualStudentOverrideProvider', ) +##### Show Answer Override for Self-Paced Courses ##### +FIELD_OVERRIDE_PROVIDERS += ( + 'openedx.features.personalized_learner_schedules.show_answer.show_answer_field_override.ShowAnswerFieldOverride', +) + ##### Self-Paced Course Due Dates ##### XBLOCK_FIELD_DATA_WRAPPERS += ( 'lms.djangoapps.courseware.field_overrides:OverrideModulestoreFieldData.wrap', diff --git a/openedx/features/personalized_learner_schedules/__init__.py b/openedx/features/personalized_learner_schedules/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/features/personalized_learner_schedules/show_answer/__init__.py b/openedx/features/personalized_learner_schedules/show_answer/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/features/personalized_learner_schedules/show_answer/show_answer_field_override.py b/openedx/features/personalized_learner_schedules/show_answer/show_answer_field_override.py new file mode 100644 index 0000000000..f216269b3e --- /dev/null +++ b/openedx/features/personalized_learner_schedules/show_answer/show_answer_field_override.py @@ -0,0 +1,56 @@ +""" +FieldOverride that forces Show Answer values that use Past Due logic to +new Show Answer values that remove the Past Due check (keeping the rest intact) +""" + + +from django.conf import settings + +from common.lib.xmodule.xmodule.capa_base import SHOWANSWER +from lms.djangoapps.courseware.field_overrides import FieldOverrideProvider +from openedx.features.course_experience import RELATIVE_DATES_FLAG + + +class ShowAnswerFieldOverride(FieldOverrideProvider): + """ + A concrete implementation of + :class:`~courseware.field_overrides.FieldOverrideProvider` which forces + Show Answer values that use Past Due logic to new Show Answer values + that remove the Past Due check (keeping the rest intact) + + Once Courseware is able to use BlockTransformers, this override should be + converted to a BlockTransformer to set the showanswer field. + """ + def get(self, block, name, default): + """ + Overwrites the 'showanswer' field on blocks in self-paced courses to + remove any checks about due dates being in the past. + """ + if name != 'showanswer': + return default + + has_showanswer = self.fallback_field_data.has(block, 'showanswer') + # This is to explicitly check the case where the default value of + # SHOWANSWER.FINISHED is left on a Course. In that case, we continue + # to follow the same mapping of FINISHED -> AFTER_ALL_ATTEMPTS_OR_CORRECT. + # This value will then be inherited throughout the rest of the Course. + if not has_showanswer and block.category == 'course': + return SHOWANSWER.AFTER_ALL_ATTEMPTS_OR_CORRECT + elif not has_showanswer: + return default + + mapping = { + SHOWANSWER.ATTEMPTED: SHOWANSWER.ATTEMPTED_NO_PAST_DUE, + SHOWANSWER.CLOSED: SHOWANSWER.AFTER_ALL_ATTEMPTS, + SHOWANSWER.CORRECT_OR_PAST_DUE: SHOWANSWER.ANSWERED, + SHOWANSWER.FINISHED: SHOWANSWER.AFTER_ALL_ATTEMPTS_OR_CORRECT, + SHOWANSWER.PAST_DUE: SHOWANSWER.NEVER, + } + current_show_answer_value = self.fallback_field_data.get(block, 'showanswer') + + return mapping.get(current_show_answer_value, default) + + @classmethod + def enabled_for(cls, course): + """ Enabled only for Self-Paced courses using Personalized User Schedules. """ + return course.self_paced and RELATIVE_DATES_FLAG.is_enabled(course.id) diff --git a/openedx/features/personalized_learner_schedules/show_answer/tests/__init__.py b/openedx/features/personalized_learner_schedules/show_answer/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/features/personalized_learner_schedules/show_answer/tests/test_show_answer_override.py b/openedx/features/personalized_learner_schedules/show_answer/tests/test_show_answer_override.py new file mode 100644 index 0000000000..752a8c5625 --- /dev/null +++ b/openedx/features/personalized_learner_schedules/show_answer/tests/test_show_answer_override.py @@ -0,0 +1,67 @@ +"""Tests for Show Answer overrides for self-paced courses.""" + +import ddt + +from django.test import RequestFactory +from django.test.utils import override_settings + +from common.lib.xmodule.xmodule.capa_base import SHOWANSWER +from lms.djangoapps.ccx.tests.test_overrides import inject_field_overrides +from lms.djangoapps.courseware.model_data import FieldDataCache +from lms.djangoapps.courseware.module_render import get_module +from openedx.features.course_experience import RELATIVE_DATES_FLAG +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + + +@override_settings( + FIELD_OVERRIDE_PROVIDERS=[ + 'openedx.features.personalized_learner_schedules.show_answer.show_answer_field_override.ShowAnswerFieldOverride' + ], +) +@ddt.ddt +class ShowAnswerFieldOverrideTest(ModuleStoreTestCase): + """ Tests for Show Answer overrides for self-paced courses. """ + + def setup_course(self, **course_kwargs): + """ Set up a course with provided course attributes. """ + course = CourseFactory.create(**course_kwargs) + inject_field_overrides((course,), course, self.user) + return course + + def get_course_module(self, course): + request = RequestFactory().request() + field_data_cache = FieldDataCache([], course.id, self.user) + return get_module(self.user, request, course.location, field_data_cache, course=course) + + @ddt.data(True, False) + def test_override_enabled_for(self, active): + with RELATIVE_DATES_FLAG.override(active=active): + # Instructor paced course will just have the default value + ip_course = self.setup_course() + course_module = self.get_course_module(ip_course) + self.assertEqual(course_module.showanswer, SHOWANSWER.FINISHED) + + sp_course = self.setup_course(self_paced=True) + course_module = self.get_course_module(sp_course) + if active: + self.assertEqual(course_module.showanswer, SHOWANSWER.AFTER_ALL_ATTEMPTS_OR_CORRECT) + else: + self.assertEqual(course_module.showanswer, SHOWANSWER.FINISHED) + + @ddt.data( + (SHOWANSWER.ATTEMPTED, SHOWANSWER.ATTEMPTED_NO_PAST_DUE), + (SHOWANSWER.CLOSED, SHOWANSWER.AFTER_ALL_ATTEMPTS), + (SHOWANSWER.CORRECT_OR_PAST_DUE, SHOWANSWER.ANSWERED), + (SHOWANSWER.FINISHED, SHOWANSWER.AFTER_ALL_ATTEMPTS_OR_CORRECT), + (SHOWANSWER.PAST_DUE, SHOWANSWER.NEVER), + (SHOWANSWER.NEVER, SHOWANSWER.NEVER), + (SHOWANSWER.AFTER_SOME_NUMBER_OF_ATTEMPTS, SHOWANSWER.AFTER_SOME_NUMBER_OF_ATTEMPTS), + (SHOWANSWER.ALWAYS, SHOWANSWER.ALWAYS), + ) + @ddt.unpack + @RELATIVE_DATES_FLAG.override(active=True) + def test_get(self, initial_value, expected_final_value): + course = self.setup_course(self_paced=True, showanswer=initial_value) + course_module = self.get_course_module(course) + self.assertEqual(course_module.showanswer, expected_final_value)