diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 8e3f97eee98e..a98a2db22752 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -84,7 +84,12 @@ 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, + ProblemFeedbackService +) log = logging.getLogger(__name__) @@ -1186,7 +1191,8 @@ 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), + "problem_feedback": ProblemFeedbackService, } 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 acab35471813..808b05eb9e13 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, ProblemFeedbackService 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 @@ -212,7 +212,8 @@ 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, + "problem_feedback": partial(ProblemFeedbackService, user_is_staff=True), } 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 530c1b34f3ea..51060228956e 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -56,6 +56,12 @@ from openedx.core.lib.gating import api as gating_api from openedx.core.lib.cache_utils import request_cached from openedx.core.toggles import ENTRANCE_EXAMS +from xmodule.services import ( + ConfigurationService, + ProblemFeedbackService, + SettingsService, + TeamsConfigurationService, +) from xmodule.course_block import DEFAULT_START_DATE from xmodule.library_tools import LibraryToolsService from xmodule.modulestore import EdxJSONEncoder, ModuleStoreEnum @@ -63,7 +69,6 @@ from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError from xmodule.modulestore.inheritance import own_metadata -from xmodule.services import ConfigurationService, SettingsService, TeamsConfigurationService from xmodule.tabs import CourseTabList from ..utils import ( @@ -331,6 +336,7 @@ def load_services_for_studio(runtime, user): "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), "teams_configuration": TeamsConfigurationService(), "library_tools": LibraryToolsService(modulestore(), user.id), + "problem_feedback": ProblemFeedbackService, } 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 fc02e2662e67..e3fe5ce9da68 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -49,7 +49,13 @@ 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, + ProblemFeedbackService +) 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 @@ -632,6 +638,7 @@ def inner_get_block(block: XBlock) -> XBlock | None: 'teams_configuration': TeamsConfigurationService(), 'call_to_action': CallToActionService(), 'publish': EventPublishingService(user, course_id, track_function), + 'problem_feedback': partial(ProblemFeedbackService, user_is_staff=user_is_staff) } 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 668ce5171213..373fc650c655 100644 --- a/lms/djangoapps/courseware/tests/test_block_render.py +++ b/lms/djangoapps/courseware/tests/test_block_render.py @@ -128,6 +128,7 @@ @XBlock.needs('teams') @XBlock.needs('teams_configuration') @XBlock.needs('call_to_action') +@XBlock.needs('problem_feedback') class PureXBlock(XBlock): """ Pure XBlock to use in tests. @@ -2340,6 +2341,7 @@ class LMSXBlockServiceBindingTest(LMSXBlockServiceMixin): 'teams', 'teams_configuration', 'call_to_action', + 'problem_feedback', ) 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..54e844349276 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 ProblemFeedbackService log = getLogger(__name__) @@ -23,6 +24,7 @@ class SubsectionGradeBase(metaclass=ABCMeta): """ def __init__(self, subsection): + self._block = subsection self.location = subsection.location self.display_name = block_metadata_utils.display_name_with_default(subsection) self.url_name = block_metadata_utils.url_name_for_block(subsection) @@ -59,7 +61,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 ProblemFeedbackService(block=self._block, user_is_staff=has_staff_access).correctness_available() @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..2986c92bf5d9 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, ProblemFeedbackService from xmodule.util.sandboxing import SandboxService from common.djangoapps.edxmako.services import MakoService from common.djangoapps.static_replace.services import ReplaceURLService @@ -299,6 +299,8 @@ def service(self, block: XBlock, service_name: str): ) elif service_name == 'publish': return EventPublishingService(self.user, context_key, make_track_function()) + elif service_name == 'problem_feedback': + return ProblemFeedbackService(block=block, user_is_staff=self.user.is_staff) # type: ignore # 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 7b58b5aa9ab8..2549b57aeba3 100644 --- a/xmodule/capa_block.py +++ b/xmodule/capa_block.py @@ -33,7 +33,7 @@ from xmodule.contentstore.django import contentstore from xmodule.editing_block import EditingMixin from xmodule.exceptions import NotFoundError, ProcessingError -from xmodule.graders import ShowCorrectness +from xmodule.graders import ShowCorrectness, ShowAnswer from xmodule.raw_block import RawMixin from xmodule.util.sandboxing import SandboxService from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment @@ -74,22 +74,8 @@ 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" +# Left for backwards compatibility +SHOWANSWER = ShowAnswer class RANDOMIZATION: @@ -122,6 +108,7 @@ def from_json(self, value): @XBlock.needs('cache') @XBlock.needs('sandbox') @XBlock.needs('replace_urls') +@XBlock.needs('problem_feedback') @XBlock.wants('call_to_action') class ProblemBlock( ScorableXBlockMixin, @@ -203,20 +190,20 @@ class ProblemBlock( help=_("Defines when to show the answer to the problem. " "A default value can be set in Advanced Settings."), scope=Scope.settings, - default=SHOWANSWER.FINISHED, + default=ShowAnswer.FINISHED, values=[ - {"display_name": _("Always"), "value": SHOWANSWER.ALWAYS}, - {"display_name": _("Answered"), "value": SHOWANSWER.ANSWERED}, - {"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}, + {"display_name": _("Always"), "value": ShowAnswer.ALWAYS}, + {"display_name": _("Answered"), "value": ShowAnswer.ANSWERED}, + {"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( @@ -1355,25 +1342,19 @@ def hint_button(self, data): def used_all_attempts(self): """ All attempts have been used """ - return self.max_attempts is not None and self.attempts >= self.max_attempts + return self.runtime.service(self, 'problem_feedback').used_all_attempts() def is_past_due(self): """ Is it now past this problem's due date, including grace period? """ - return (self.close_date is not None and - datetime.datetime.now(utc) > self.close_date) + return self.runtime.service(self, 'problem_feedback').is_past_due() def closed(self): """ Is the student still allowed to submit answers? """ - if self.used_all_attempts(): - return True - if self.is_past_due(): - return True - - return False + return self.runtime.service(self, 'problem_feedback').closed() def is_submitted(self): """ @@ -1392,7 +1373,7 @@ def is_attempted(self): used by conditional block """ - return self.attempts > 0 + return self.runtime.service(self, 'problem_feedback').is_attempted() def is_correct(self): """ @@ -1406,47 +1387,7 @@ 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) - if not self.correctness_available(): - # If correctness is being withheld, then don't show answers either. - return False - elif self.showanswer == '': - return False - elif self.showanswer == SHOWANSWER.NEVER: - return False - elif user_is_staff: - # This is after the 'never' check because admins can see the answer - # unless the problem explicitly prevents it - return True - elif self.showanswer == SHOWANSWER.ATTEMPTED: - 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. - return self.is_correct() - elif self.showanswer == SHOWANSWER.CLOSED: - return self.closed() - elif self.showanswer == SHOWANSWER.FINISHED: - return self.closed() or self.is_correct() - - elif self.showanswer == SHOWANSWER.CORRECT_OR_PAST_DUE: - return self.is_correct() or self.is_past_due() - elif self.showanswer == SHOWANSWER.PAST_DUE: - return self.is_past_due() - elif self.showanswer == SHOWANSWER.AFTER_SOME_NUMBER_OF_ATTEMPTS: - required_attempts = self.attempts_before_showanswer_button - if self.max_attempts and required_attempts >= self.max_attempts: - required_attempts = self.max_attempts - 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 + return self.runtime.service(self, 'problem_feedback').answer_available() def correctness_available(self): """ @@ -1454,12 +1395,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( - show_correctness=self.show_correctness, - due_date=self.close_date, - has_staff_access=user_is_staff, - ) + return self.runtime.service(self, 'problem_feedback').correctness_available() def update_score(self, data): """ diff --git a/xmodule/graders.py b/xmodule/graders.py index a587204d682e..c58b6f877b5a 100644 --- a/xmodule/graders.py +++ b/xmodule/graders.py @@ -9,9 +9,7 @@ import random import sys from collections import OrderedDict -from datetime import datetime -from pytz import UTC from django.utils.translation import gettext_lazy as _ from xmodule.util.misc import get_short_labeler @@ -472,7 +470,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,21 +481,24 @@ 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 the possible status for showing the answer. + + When an answer is hidden, this limits the user's access to it. + """ + + # Constants used to indicate when to show an 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" diff --git a/xmodule/services.py b/xmodule/services.py index 25c680a9653a..3108d99567f3 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,140 @@ 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 ProblemFeedbackService(Service): + """ + An XBlock Service that allows XModules to define: + + - if the answers are visible + - 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. + """ + def __init__(self, block=None, user_is_staff=False, **kwargs): + super().__init__(**kwargs) + self._user_is_staff = user_is_staff + # This is needed because the `Service` class initialization expects the XBlock passed as an `xblock` keyword + # argument, but the `service` method from the `DescriptorSystem` passes a `block`. + self._xblock = self.xblock() or block + + def is_past_due(self): + """ + Returns if the problem is past its due date. + """ + if self._xblock: + return (self._xblock.close_date is not None and + datetime.now(UTC) > self._xblock.close_date) + return False + + def is_attempted(self): + """ + Has the problem been attempted? + """ + if self._xblock: + return self._xblock.attempts > 0 + return False + + def used_all_attempts(self): + """ All attempts have been used """ + if self._xblock: + return (self._xblock.max_attempts is not None and + self._xblock.attempts >= self._xblock.max_attempts) + return False + + def closed(self): + """ + Is the student still allowed to submit answers? + """ + if self.used_all_attempts(): + return True + if self.is_past_due(): + return True + return False + + def answer_available(self): + """ + Returns whether correctness is available now, for the given attributes. + """ + if self._xblock: + show_correctness = self._xblock.show_correctness + show_answer = self._xblock.showanswer + max_attempts = self._xblock.max_attempts + attempts = self._xblock.attempts + is_correct = self._xblock.is_correct() + required_attempts = self._xblock.attempts_before_showanswer_button + else: + show_correctness = '' + show_answer = '' + max_attempts = 0 + attempts = 0 + required_attempts = 0 + is_correct = False + past_due = self.is_past_due() + is_attempted = self.is_attempted() + used_all_attempts = self.used_all_attempts() + closed = self.closed() + + if not show_correctness: + # If correctness is being withheld, then don't show answers either. + return False + elif show_correctness == ShowCorrectness.NEVER: + # If correctness is being hidden the answer is not shown + return False + elif show_answer == ShowAnswer.NEVER: + return False + elif self._user_is_staff: + # 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 + + def correctness_available(self): + """ + Returns whether correctness is available now, for the given attributes. + """ + if self._xblock: + due_date = self._xblock.due + show_correctness = self._xblock.show_correctness + else: + due_date = None + show_correctness = '' + + if show_correctness == ShowCorrectness.NEVER: + return False + elif self._user_is_staff: + # 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)) + return True diff --git a/xmodule/tests/__init__.py b/xmodule/tests/__init__.py index b2cdd67b71ba..1d7e2679f227 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 ProblemFeedbackService from xmodule.x_module import DoNothingCache, XModuleMixin from openedx.core.lib.cache_utils import CacheService @@ -165,6 +166,7 @@ def get_block(block): 'cache': CacheService(DoNothingCache()), 'field-data': DictFieldData({}), 'sandbox': SandboxService(contentstore, course_id), + 'problem_feedback': ProblemFeedbackService, } descriptor_system.get_block_for_descriptor = get_block # lint-amnesty, pylint: disable=attribute-defined-outside-init @@ -220,6 +222,7 @@ def get_block(block): 'cache': CacheService(DoNothingCache()), 'field-data': DictFieldData({}), 'sandbox': SandboxService(contentstore, course_id), + 'problem_feedback': ProblemFeedbackService, } if add_overrides: diff --git a/xmodule/tests/test_capa_block.py b/xmodule/tests/test_capa_block.py index ab94028fc955..2df537718d56 100644 --- a/xmodule/tests/test_capa_block.py +++ b/xmodule/tests/test_capa_block.py @@ -35,9 +35,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 @@ -559,7 +560,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, @@ -569,7 +570,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..d3725e8c4f98 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, ProblemFeedbackService from openedx.core.lib.teams_config import TeamsConfig @@ -163,3 +166,82 @@ 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 TestProblemFeedbackService(unittest.TestCase): + """ + Tests the correctness_available method + """ + + def setUp(self): + super().setUp() + self._xblock = mock.MagicMock() + 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 ProblemFeedbackService(self._xblock).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. + """ + self._xblock.show_correctness = show_correctness + assert ProblemFeedbackService(self._xblock, user_is_staff=has_staff_access).correctness_available() + + @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. + """ + self._xblock.show_correctness = ShowCorrectness.NEVER + assert not ProblemFeedbackService(self._xblock, user_is_staff=has_staff_access).correctness_available() + + @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. + """ + self._xblock.show_correctness = ShowCorrectness.PAST_DUE + if due_date_str is None: + self._xblock.due = None + else: + self._xblock.due = getattr(self, due_date_str) + assert ProblemFeedbackService(self._xblock, user_is_staff=has_staff_access).correctness_available() ==\ + expected_result