From 47d081b71b4b1906e94fbb5cb1b44c7108ae49ce Mon Sep 17 00:00:00 2001 From: Stephannie Jimenez Date: Mon, 9 Oct 2023 22:58:46 -0500 Subject: [PATCH] refactor: create runtime services for showCorrectness and showAnswer --- cms/djangoapps/contentstore/utils.py | 12 ++- cms/djangoapps/contentstore/views/preview.py | 6 +- .../xblock_storage_handlers/view_handlers.py | 4 + lms/djangoapps/courseware/block_render.py | 11 ++- .../courseware/tests/test_block_render.py | 4 + lms/djangoapps/grades/subsection_grade.py | 5 +- .../core/djangoapps/xblock/runtime/runtime.py | 6 +- .../show_answer/show_answer_field_override.py | 12 +-- .../tests/test_show_answer_override.py | 26 +++--- xmodule/capa_block.py | 26 ++---- xmodule/graders.py | 69 +-------------- xmodule/services.py | 84 ++++++++++++++++++- xmodule/tests/__init__.py | 5 ++ xmodule/tests/test_capa_block.py | 7 +- xmodule/tests/test_graders.py | 83 +----------------- xmodule/tests/test_services.py | 83 +++++++++++++++++- 16 files changed, 242 insertions(+), 201 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 1a4b709622e6..d5ed9bf05c51 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -83,7 +83,13 @@ from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order from xmodule.partitions.partitions_service import get_all_partitions_for_course # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.services import SettingsService, ConfigurationService, TeamsConfigurationService +from xmodule.services import ( + SettingsService, + ConfigurationService, + TeamsConfigurationService, + ShowAnswerService, + ShowCorrectnessService +) log = logging.getLogger(__name__) @@ -1159,7 +1165,9 @@ def load_services_for_studio(runtime, user): "settings": SettingsService(), "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), "teams_configuration": TeamsConfigurationService(), - "library_tools": LibraryToolsService(modulestore(), user.id) + "library_tools": LibraryToolsService(modulestore(), user.id), + "show_answer": ShowAnswerService(), + "show_correctness": ShowCorrectnessService(), } runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 48c323c1aa88..3afeebbab392 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -21,7 +21,7 @@ from xmodule.exceptions import NotFoundError, ProcessingError from xmodule.modulestore.django import XBlockI18nService, modulestore from xmodule.partitions.partitions_service import PartitionService -from xmodule.services import SettingsService, TeamsConfigurationService +from xmodule.services import SettingsService, TeamsConfigurationService, ShowAnswerService, ShowCorrectnessService from xmodule.studio_editable import has_author_view from xmodule.util.sandboxing import SandboxService from xmodule.util.builtin_assets import add_webpack_js_to_fragment @@ -211,7 +211,9 @@ def _prepare_runtime_for_preview(request, block): "teams_configuration": TeamsConfigurationService(), "sandbox": SandboxService(contentstore=contentstore, course_id=course_id), "cache": CacheService(cache), - 'replace_urls': ReplaceURLService + 'replace_urls': ReplaceURLService, + "show_answer": ShowAnswerService(), + "show_correctness": ShowCorrectnessService(), } block.runtime.get_block_for_descriptor = partial(_load_preview_block, request) diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 425f97e3751f..4260519147f2 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -82,6 +82,8 @@ ConfigurationService, SettingsService, TeamsConfigurationService, + ShowAnswerService, + ShowCorrectnessService ) # lint-amnesty, pylint: disable=wrong-import-order from xmodule.tabs import ( CourseTabList, @@ -350,6 +352,8 @@ def load_services_for_studio(runtime, user): "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), "teams_configuration": TeamsConfigurationService(), "library_tools": LibraryToolsService(modulestore(), user.id), + "show_answer": ShowAnswerService(), + "show_correctness": ShowCorrectnessService(), } runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index 650b4418b423..81afff2bde95 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -51,7 +51,14 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.partitions.partitions_service import PartitionService from xmodule.util.sandboxing import SandboxService -from xmodule.services import EventPublishingService, RebindUserService, SettingsService, TeamsConfigurationService +from xmodule.services import ( + EventPublishingService, + RebindUserService, + SettingsService, + TeamsConfigurationService, + ShowAnswerService, + ShowCorrectnessService +) from common.djangoapps.static_replace.services import ReplaceURLService from common.djangoapps.static_replace.wrapper import replace_urls_wrapper from lms.djangoapps.courseware.access import get_user_role, has_access @@ -635,6 +642,8 @@ def inner_get_block(block: XBlock) -> XBlock | None: 'teams_configuration': TeamsConfigurationService(), 'call_to_action': CallToActionService(), 'publish': EventPublishingService(user, course_id, track_function), + 'show_answer': ShowAnswerService(), + 'show_correctness': ShowCorrectnessService() } runtime.get_block_for_descriptor = inner_get_block diff --git a/lms/djangoapps/courseware/tests/test_block_render.py b/lms/djangoapps/courseware/tests/test_block_render.py index 80827afa632c..0ae51812bbea 100644 --- a/lms/djangoapps/courseware/tests/test_block_render.py +++ b/lms/djangoapps/courseware/tests/test_block_render.py @@ -130,6 +130,8 @@ @XBlock.needs('teams') @XBlock.needs('teams_configuration') @XBlock.needs('call_to_action') +@XBlock.needs('show_answer') +@XBlock.needs('show_correctness') class PureXBlock(XBlock): """ Pure XBlock to use in tests. @@ -2342,6 +2344,8 @@ class LMSXBlockServiceBindingTest(LMSXBlockServiceMixin): 'teams', 'teams_configuration', 'call_to_action', + 'show_correctness', + 'show_answer', ) def test_expected_services_exist(self, expected_service): """ diff --git a/lms/djangoapps/grades/subsection_grade.py b/lms/djangoapps/grades/subsection_grade.py index ba098a92a417..7500198b9fa7 100644 --- a/lms/djangoapps/grades/subsection_grade.py +++ b/lms/djangoapps/grades/subsection_grade.py @@ -12,7 +12,8 @@ from lms.djangoapps.grades.models import BlockRecord, PersistentSubsectionGrade from lms.djangoapps.grades.scores import compute_percent, get_score, possibly_scored from xmodule import block_metadata_utils, graders # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.graders import AggregatedScore, ShowCorrectness # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.graders import AggregatedScore # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.services import ShowCorrectnessService log = getLogger(__name__) @@ -59,7 +60,7 @@ def show_grades(self, has_staff_access): """ Returns whether subsection scores are currently available to users with or without staff access. """ - return ShowCorrectness.correctness_available(self.show_correctness, self.due, has_staff_access) + return ShowCorrectnessService.correctness_available(self.show_correctness, self.due, has_staff_access) @property def attempted_graded(self): diff --git a/openedx/core/djangoapps/xblock/runtime/runtime.py b/openedx/core/djangoapps/xblock/runtime/runtime.py index 74771833fedf..b1118d9c517a 100644 --- a/openedx/core/djangoapps/xblock/runtime/runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/runtime.py @@ -26,7 +26,7 @@ from xmodule.errortracker import make_error_tracker from xmodule.contentstore.django import contentstore from xmodule.modulestore.django import XBlockI18nService -from xmodule.services import EventPublishingService, RebindUserService +from xmodule.services import EventPublishingService, RebindUserService, ShowAnswerService, ShowCorrectnessService from xmodule.util.sandboxing import SandboxService from common.djangoapps.edxmako.services import MakoService from common.djangoapps.static_replace.services import ReplaceURLService @@ -299,6 +299,10 @@ def service(self, block: XBlock, service_name: str): ) elif service_name == 'publish': return EventPublishingService(self.user, context_key, make_track_function()) + elif service_name == 'show_answer': + return ShowAnswerService() + elif service_name == 'show_correctness': + return ShowCorrectnessService() # Check if the XBlockRuntimeSystem wants to handle this: service = self.system.get_service(block, service_name) 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 d6452a84962f..44bfe0ced9e9 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 @@ -5,7 +5,7 @@ from lms.djangoapps.courseware.field_overrides import FieldOverrideProvider from openedx.features.course_experience import RELATIVE_DATES_FLAG -from xmodule.capa_block import SHOWANSWER # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.graders import ShowAnswer class ShowAnswerFieldOverride(FieldOverrideProvider): @@ -27,11 +27,11 @@ def get(self, block, name, default): 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, + 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') 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 0a0c03725545..938da08b1f05 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 @@ -10,7 +10,7 @@ from lms.djangoapps.courseware.model_data import FieldDataCache from lms.djangoapps.courseware.block_render import get_block from openedx.features.course_experience import RELATIVE_DATES_FLAG -from xmodule.capa_block import SHOWANSWER # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.graders import ShowAnswer from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order @@ -41,27 +41,27 @@ def test_override_enabled_for(self, active): # Instructor paced course will just have the default value ip_course = self.setup_course() course_block = self.get_course_block(ip_course) - assert course_block.showanswer == SHOWANSWER.FINISHED + assert course_block.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) + sp_course = self.setup_course(self_paced=True, showanswer=ShowAnswer.FINISHED) course_block = self.get_course_block(sp_course) if active: - assert course_block.showanswer == SHOWANSWER.AFTER_ALL_ATTEMPTS_OR_CORRECT + assert course_block.showanswer == ShowAnswer.AFTER_ALL_ATTEMPTS_OR_CORRECT else: - assert course_block.showanswer == SHOWANSWER.FINISHED + assert course_block.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), + (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 @override_waffle_flag(RELATIVE_DATES_FLAG, active=True) diff --git a/xmodule/capa_block.py b/xmodule/capa_block.py index f8d4054c16c1..6968e751da09 100644 --- a/xmodule/capa_block.py +++ b/xmodule/capa_block.py @@ -74,24 +74,6 @@ FEATURES = {} -class SHOWANSWER: - """ - Constants for when to show answer - """ - ALWAYS = "always" - ANSWERED = "answered" - ATTEMPTED = "attempted" - CLOSED = "closed" - FINISHED = "finished" - CORRECT_OR_PAST_DUE = "correct_or_past_due" - 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: """ Constants for problem randomization @@ -123,6 +105,8 @@ def from_json(self, value): @XBlock.needs('sandbox') @XBlock.needs('replace_urls') @XBlock.wants('call_to_action') +@XBlock.wants('show_correctness') +@XBlock.wants('show_answer') class ProblemBlock( ScorableXBlockMixin, RawMixin, @@ -1407,9 +1391,9 @@ def answer_available(self): Is the user allowed to see an answer? """ user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) - return ShowAnswer.answer_available( + return self.runtime.service(self, 'show_answer').answer_available( show_answer=self.showanswer, - show_correctness=self.show_correctness_available(), + show_correctness=self.correctness_available(), past_due=self.is_past_due(), attempts=self.attempts, is_attempted=self.is_attempted(), @@ -1428,7 +1412,7 @@ def correctness_available(self): Limits access to the correct/incorrect flags, messages, and problem score. """ user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) - return ShowCorrectness.correctness_available( + return self.runtime.service(self, 'show_correctness').correctness_available( show_correctness=self.show_correctness, due_date=self.close_date, has_staff_access=user_is_staff, diff --git a/xmodule/graders.py b/xmodule/graders.py index a689e95009d8..d4e384e3bc51 100644 --- a/xmodule/graders.py +++ b/xmodule/graders.py @@ -472,7 +472,7 @@ def _min_or_none(itr): class ShowCorrectness: """ - Helper class for determining whether correctness is currently hidden for a block. + Helper class for determining the possible status of correctness. When correctness is hidden, this limits the user's access to the correct/incorrect flags, messages, problem scores, and aggregate subsection and course grades. @@ -483,29 +483,10 @@ class ShowCorrectness: PAST_DUE = "past_due" NEVER = "never" - @classmethod - def correctness_available(cls, show_correctness='', due_date=None, has_staff_access=False): - """ - Returns whether correctness is available now, for the given attributes. - """ - if show_correctness == cls.NEVER: - return False - elif has_staff_access: - # This is after the 'never' check because course staff can see correctness - # unless the sequence/problem explicitly prevents it - return True - elif show_correctness == cls.PAST_DUE: - # Is it now past the due date? - return (due_date is None or - due_date < datetime.now(UTC)) - - # else: show_correctness == cls.ALWAYS - return True - class ShowAnswer: """ - Helper class for determining whether an answer is currently hidden for a block. + Helper class for determining the possible status for showing the answer. When an answer is hidden, this limits the user's access to it. """ @@ -523,49 +504,3 @@ class ShowAnswer: AFTER_ALL_ATTEMPTS = "after_all_attempts" AFTER_ALL_ATTEMPTS_OR_CORRECT = "after_all_attempts_or_correct" ATTEMPTED_NO_PAST_DUE = "attempted_no_past_due" - - @classmethod - def answer_available( - cls, show_answer='', show_correctness='', past_due=False, attempts=0, - is_attempted=False, is_correct=False, required_attempts=0, - max_attempts=0, used_all_attempts=False, closed=False, - has_staff_access=False): - """ - Returns whether correctness is available now, for the given attributes. - """ - if not show_correctness: - # If correctness is being withheld, then don't show answers either. - return False - elif show_answer == cls.NEVER: - return False - elif has_staff_access: - # This is after the 'never' check because course staff can see correctness - # unless the sequence/problem explicitly prevents it - return True - elif show_answer == cls.ATTEMPTED: - return is_attempted or past_due - elif show_answer == cls.ANSWERED: - # NOTE: this is slightly different from 'attempted' -- resetting the problems - # makes lcp.done False, but leaves attempts unchanged. - return is_correct - elif show_answer == cls.CLOSED: - return closed - elif show_answer == cls.FINISHED: - return closed or is_correct - elif show_answer == cls.CORRECT_OR_PAST_DUE: - return is_correct or past_due - elif show_answer == cls.PAST_DUE: - return past_due - elif show_answer == cls.AFTER_SOME_NUMBER_OF_ATTEMPTS: - if max_attempts and required_attempts >= max_attempts: - required_attempts = max_attempts - return attempts >= required_attempts - elif show_answer == cls.ALWAYS: - return True - elif show_answer == cls.AFTER_ALL_ATTEMPTS: - return used_all_attempts - elif show_answer == cls.AFTER_ALL_ATTEMPTS_OR_CORRECT: - return used_all_attempts or is_correct - elif show_answer == cls.ATTEMPTED_NO_PAST_DUE: - return is_attempted - return False diff --git a/xmodule/services.py b/xmodule/services.py index 25c680a9653a..eccb9cc90438 100644 --- a/xmodule/services.py +++ b/xmodule/services.py @@ -6,6 +6,9 @@ import inspect import logging from functools import partial +from datetime import datetime + +from pytz import UTC from config_models.models import ConfigurationModel from django.conf import settings @@ -17,13 +20,14 @@ from common.djangoapps.track import contexts from lms.djangoapps.courseware.masquerade import is_masquerading_as_specific_student from xmodule.modulestore.django import modulestore +from xmodule.graders import ShowCorrectness, ShowAnswer from lms.djangoapps.courseware.field_overrides import OverrideFieldData from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache from lms.djangoapps.lms_xblock.field_data import LmsFieldData from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig -from lms.djangoapps.grades.api import signals as grades_signals +from lms.djangoapps.grades.signals import signals as grades_signals log = logging.getLogger(__name__) @@ -308,3 +312,81 @@ def _handle_deprecated_progress_event(self, block, event): # in order to avoid duplicate work and possibly conflicting semantics. if not getattr(block, 'has_custom_completion', False): self.completion_service.submit_completion(block.scope_ids.usage_id, 1.0) + + +class ShowAnswerService(Service): + """ + An XBlock Service that allows XModules to define if the answers are visible. + """ + @classmethod + def answer_available( + cls, show_answer='', show_correctness='', past_due=False, attempts=0, + is_attempted=False, is_correct=False, required_attempts=0, + max_attempts=0, used_all_attempts=False, closed=False, + has_staff_access=False): + """ + Returns whether correctness is available now, for the given attributes. + """ + if not show_correctness: + # If correctness is being withheld, then don't show answers either. + return False + elif show_answer == ShowAnswer.NEVER: + return False + elif has_staff_access: + # This is after the 'never' check because course staff can see correctness + # unless the sequence/problem explicitly prevents it + return True + elif show_answer == ShowAnswer.ATTEMPTED: + return is_attempted or past_due + elif show_answer == ShowAnswer.ANSWERED: + # NOTE: this is slightly different from 'attempted' -- resetting the problems + # makes lcp.done False, but leaves attempts unchanged. + return is_correct + elif show_answer == ShowAnswer.CLOSED: + return closed + elif show_answer == ShowAnswer.FINISHED: + return closed or is_correct + elif show_answer == ShowAnswer.CORRECT_OR_PAST_DUE: + return is_correct or past_due + elif show_answer == ShowAnswer.PAST_DUE: + return past_due + elif show_answer == ShowAnswer.AFTER_SOME_NUMBER_OF_ATTEMPTS: + if max_attempts and required_attempts >= max_attempts: + required_attempts = max_attempts + return attempts >= required_attempts + elif show_answer == ShowAnswer.ALWAYS: + return True + elif show_answer == ShowAnswer.AFTER_ALL_ATTEMPTS: + return used_all_attempts + elif show_answer == ShowAnswer.AFTER_ALL_ATTEMPTS_OR_CORRECT: + return used_all_attempts or is_correct + elif show_answer == ShowAnswer.ATTEMPTED_NO_PAST_DUE: + return is_attempted + return False + + +class ShowCorrectnessService(Service): + """ + An XBlock Service that allows XModules to define whether correctness is currently hidden. + + When correctness is hidden, this limits the user's access to the correct/incorrect flags, messages, problem scores, + and aggregate subsection and course grades. + """ + @classmethod + def correctness_available(cls, show_correctness='', due_date=None, has_staff_access=False): + """ + Returns whether correctness is available now, for the given attributes. + """ + if show_correctness == ShowCorrectness.NEVER: + return False + elif has_staff_access: + # This is after the 'never' check because course staff can see correctness + # unless the sequence/problem explicitly prevents it + return True + elif show_correctness == ShowCorrectness.PAST_DUE: + # Is it now past the due date? + return (due_date is None or + due_date < datetime.now(UTC)) + + # else: show_correctness == ShowCorrectness.ALWAYS + return True diff --git a/xmodule/tests/__init__.py b/xmodule/tests/__init__.py index b2cdd67b71ba..7b150af9a196 100644 --- a/xmodule/tests/__init__.py +++ b/xmodule/tests/__init__.py @@ -36,6 +36,7 @@ from xmodule.modulestore.xml import CourseLocationManager from xmodule.tests.helpers import StubReplaceURLService, mock_render_template, StubMakoService, StubUserService from xmodule.util.sandboxing import SandboxService +from xmodule.services import ShowAnswerService, ShowCorrectnessService from xmodule.x_module import DoNothingCache, XModuleMixin from openedx.core.lib.cache_utils import CacheService @@ -165,6 +166,8 @@ def get_block(block): 'cache': CacheService(DoNothingCache()), 'field-data': DictFieldData({}), 'sandbox': SandboxService(contentstore, course_id), + 'show_answer': ShowAnswerService(), + 'show_correctness': ShowCorrectnessService(), } descriptor_system.get_block_for_descriptor = get_block # lint-amnesty, pylint: disable=attribute-defined-outside-init @@ -220,6 +223,8 @@ def get_block(block): 'cache': CacheService(DoNothingCache()), 'field-data': DictFieldData({}), 'sandbox': SandboxService(contentstore, course_id), + 'show_answer': ShowAnswerService(), + 'show_correctness': ShowCorrectnessService(), } if add_overrides: diff --git a/xmodule/tests/test_capa_block.py b/xmodule/tests/test_capa_block.py index b511fe403e43..25cceea3b8c1 100644 --- a/xmodule/tests/test_capa_block.py +++ b/xmodule/tests/test_capa_block.py @@ -34,9 +34,10 @@ from xmodule.capa.responsetypes import LoncapaProblemError, ResponseError, StudentInputError from xmodule.capa.xqueue_interface import XQueueInterface from xmodule.capa_block import ComplexEncoder, ProblemBlock +from xmodule.graders import ShowAnswer from xmodule.tests import DATA_DIR -from ..capa_block import RANDOMIZATION, SHOWANSWER +from ..capa_block import RANDOMIZATION from . import get_test_system @@ -557,7 +558,7 @@ def test_showanswer_answered(self): """ # Can not see "Show Answer" when student answer is wrong answer_wrong = CapaFactory.create( - showanswer=SHOWANSWER.ANSWERED, + showanswer=ShowAnswer.ANSWERED, max_attempts="1", attempts="0", due=self.tomorrow_str, @@ -567,7 +568,7 @@ def test_showanswer_answered(self): # Expect to see "Show Answer" when answer is correct answer_correct = CapaFactory.create( - showanswer=SHOWANSWER.ANSWERED, + showanswer=ShowAnswer.ANSWERED, max_attempts="1", attempts="0", due=self.tomorrow_str, diff --git a/xmodule/tests/test_graders.py b/xmodule/tests/test_graders.py index 5073052b1ba8..fbc9aedcdadb 100644 --- a/xmodule/tests/test_graders.py +++ b/xmodule/tests/test_graders.py @@ -4,14 +4,13 @@ import unittest -from datetime import datetime, timedelta +from datetime import datetime import pytest import ddt -from pytz import UTC from lms.djangoapps.grades.scores import compute_percent from xmodule import graders -from xmodule.graders import AggregatedScore, ProblemScore, ShowCorrectness, aggregate_scores +from xmodule.graders import AggregatedScore, ProblemScore, aggregate_scores class GradesheetTest(unittest.TestCase): @@ -336,81 +335,3 @@ def test_grader_with_invalid_conf(self, invalid_conf, expected_error_message): with pytest.raises(ValueError) as error: graders.grader_from_conf([invalid_conf]) assert expected_error_message in str(error.value) - - -@ddt.ddt -class ShowCorrectnessTest(unittest.TestCase): - """ - Tests the correctness_available method - """ - - def setUp(self): - super().setUp() - - now = datetime.now(UTC) - day_delta = timedelta(days=1) - self.yesterday = now - day_delta - self.today = now - self.tomorrow = now + day_delta - - def test_show_correctness_default(self): - """ - Test that correctness is visible by default. - """ - assert ShowCorrectness.correctness_available() - - @ddt.data( - (ShowCorrectness.ALWAYS, True), - (ShowCorrectness.ALWAYS, False), - # Any non-constant values behave like "always" - ('', True), - ('', False), - ('other-value', True), - ('other-value', False), - ) - @ddt.unpack - def test_show_correctness_always(self, show_correctness, has_staff_access): - """ - Test that correctness is visible when show_correctness is turned on. - """ - assert ShowCorrectness.correctness_available(show_correctness=show_correctness, - has_staff_access=has_staff_access) - - @ddt.data(True, False) - def test_show_correctness_never(self, has_staff_access): - """ - Test that show_correctness="never" hides correctness from learners and course staff. - """ - assert not ShowCorrectness.correctness_available(show_correctness=ShowCorrectness.NEVER, - has_staff_access=has_staff_access) - - @ddt.data( - # Correctness not visible to learners if due date in the future - ('tomorrow', False, False), - # Correctness is visible to learners if due date in the past - ('yesterday', False, True), - # Correctness is visible to learners if due date in the past (just) - ('today', False, True), - # Correctness is visible to learners if there is no due date - (None, False, True), - # Correctness is visible to staff if due date in the future - ('tomorrow', True, True), - # Correctness is visible to staff if due date in the past - ('yesterday', True, True), - # Correctness is visible to staff if there is no due date - (None, True, True), - ) - @ddt.unpack - def test_show_correctness_past_due(self, due_date_str, has_staff_access, expected_result): - """ - Test show_correctness="past_due" to ensure: - * correctness is always visible to course staff - * correctness is always visible to everyone if there is no due date - * correctness is visible to learners after the due date, when there is a due date. - """ - if due_date_str is None: - due_date = None - else: - due_date = getattr(self, due_date_str) - assert ShowCorrectness.correctness_available(ShowCorrectness.PAST_DUE, due_date, has_staff_access) ==\ - expected_result diff --git a/xmodule/tests/test_services.py b/xmodule/tests/test_services.py index 3e571756ca5a..87ea588c6ae0 100644 --- a/xmodule/tests/test_services.py +++ b/xmodule/tests/test_services.py @@ -3,10 +3,12 @@ """ import unittest from unittest import mock +from datetime import datetime, timedelta import pytest from django.test import TestCase import ddt +from pytz import UTC from config_models.models import ConfigurationModel from django.conf import settings @@ -14,7 +16,8 @@ from xblock.runtime import Mixologist from opaque_keys.edx.locator import CourseLocator -from xmodule.services import ConfigurationService, SettingsService, TeamsConfigurationService +from xmodule.graders import ShowCorrectness +from xmodule.services import ConfigurationService, SettingsService, TeamsConfigurationService, ShowCorrectnessService from openedx.core.lib.teams_config import TeamsConfig @@ -163,3 +166,81 @@ class TestTeamsConfigurationService(ConfigurationServiceBaseClass): def test_get_teamsconfiguration(self): teams_config = self.configuration_service.get_teams_configuration(self.course.id) assert teams_config == self.teams_config + + +@ddt.ddt +class TestShowCorrectnessService(unittest.TestCase): + """ + Tests the correctness_available method + """ + + def setUp(self): + super().setUp() + + now = datetime.now(UTC) + day_delta = timedelta(days=1) + self.yesterday = now - day_delta + self.today = now + self.tomorrow = now + day_delta + + def test_show_correctness_default(self): + """ + Test that correctness is visible by default. + """ + assert ShowCorrectnessService.correctness_available() + + @ddt.data( + (ShowCorrectness.ALWAYS, True), + (ShowCorrectness.ALWAYS, False), + # Any non-constant values behave like "always" + ('', True), + ('', False), + ('other-value', True), + ('other-value', False), + ) + @ddt.unpack + def test_show_correctness_always(self, show_correctness, has_staff_access): + """ + Test that correctness is visible when show_correctness is turned on. + """ + assert ShowCorrectnessService.correctness_available( + show_correctness=show_correctness, has_staff_access=has_staff_access) + + @ddt.data(True, False) + def test_show_correctness_never(self, has_staff_access): + """ + Test that show_correctness="never" hides correctness from learners and course staff. + """ + assert not ShowCorrectnessService.correctness_available( + show_correctness=ShowCorrectness.NEVER, has_staff_access=has_staff_access) + + @ddt.data( + # Correctness not visible to learners if due date in the future + ('tomorrow', False, False), + # Correctness is visible to learners if due date in the past + ('yesterday', False, True), + # Correctness is visible to learners if due date in the past (just) + ('today', False, True), + # Correctness is visible to learners if there is no due date + (None, False, True), + # Correctness is visible to staff if due date in the future + ('tomorrow', True, True), + # Correctness is visible to staff if due date in the past + ('yesterday', True, True), + # Correctness is visible to staff if there is no due date + (None, True, True), + ) + @ddt.unpack + def test_show_correctness_past_due(self, due_date_str, has_staff_access, expected_result): + """ + Test show_correctness="past_due" to ensure: + * correctness is always visible to course staff + * correctness is always visible to everyone if there is no due date + * correctness is visible to learners after the due date, when there is a due date. + """ + if due_date_str is None: + due_date = None + else: + due_date = getattr(self, due_date_str) + assert ShowCorrectnessService.correctness_available(ShowCorrectness.PAST_DUE, due_date, has_staff_access) ==\ + expected_result