From 82e7a9ce1392ed4217c0d9ff01b8e8639e15ad01 Mon Sep 17 00:00:00 2001 From: Dillon Dumesnil Date: Tue, 18 Aug 2020 07:53:18 -0700 Subject: [PATCH] AA-307: Turn showanswer override back on This doesn't handle the default case where the showanswer has never been touched, in which case it will continue to return Finished, but that also happens when it's turned off so this just helps out for all of the other cases. --- .../show_answer/show_answer_field_override.py | 18 +----- .../tests/test_show_answer_override.py | 61 ++++++++++--------- 2 files changed, 34 insertions(+), 45 deletions(-) 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 index bd7bc4b0ba..9dbd93828d 100644 --- 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 @@ -26,17 +26,7 @@ class ShowAnswerFieldOverride(FieldOverrideProvider): 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 and block.category == 'course': - return SHOWANSWER.AFTER_ALL_ATTEMPTS_OR_CORRECT - elif not has_showanswer: + if name != 'showanswer' or not self.fallback_field_data.has(block, 'showanswer'): return default mapping = { @@ -53,8 +43,4 @@ class ShowAnswerFieldOverride(FieldOverrideProvider): @classmethod def enabled_for(cls, course): """ Enabled only for Self-Paced courses using Personalized User Schedules. """ - # Returning False while we figure out a bug where Course Level Show Answer settings are not - # being properly applied and are being overridden with AFTER_ALL_ATTEMPTS_OR_CORRECT - # *****IMPORTANT*****: comment the tests back in when this is re-enabled - return False - # return course and course.self_paced and RELATIVE_DATES_FLAG.is_enabled(course.id) + return course and course.self_paced and RELATIVE_DATES_FLAG.is_enabled(course.id) 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 index fce7b52d2e..4a6c8e35c2 100644 --- 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 @@ -34,34 +34,37 @@ class ShowAnswerFieldOverrideTest(ModuleStoreTestCase): 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) + @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) + # This should be updated to not explicitly add in the showanswer so it can test the + # default case of never touching showanswer. Reference ticket AA-307 (if that's closed, + # this can be updated!) + sp_course = self.setup_course(self_paced=True, showanswer=SHOWANSWER.FINISHED) + 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) + @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)