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)