refactor: create problem feedback runtime service

cms/djangoapps/contentstore/utils.py

@@ -108,7 +108,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
+)
 
 
 IMPORTABLE_FILE_TYPES = ('.tar.gz', '.zip')
@@ -1279,7 +1284,8 @@ def load_services_for_studio(runtime, user):
         "settings": SettingsService(),
         "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag),
         "teams_configuration": TeamsConfigurationService(),
-        "library_tools": LegacyLibraryToolsService(modulestore(), user.id)
+        "library_tools": LegacyLibraryToolsService(modulestore(), user.id),
+        "problem_feedback": ProblemFeedbackService,
     }
 
     runtime._services.update(services)  # lint-amnesty, pylint: disable=protected-access

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)

cms/envs/common.py

@@ -995,7 +995,7 @@
 ############# XBlock Configuration ##########
 
 # Import after sys.path fixup
-from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.modulestore.inheritance import InheritableFieldsMixin
 from xmodule.x_module import XModuleMixin
 
 # These are the Mixins that will be added to every Blocklike upon instantiation.
@@ -1008,7 +1008,7 @@
     #  (a) merge their functionality into the base Blocklike class, or
     #  (b) refactor their functionality out of the Blocklike objects and into the edx-platform block runtimes.
     LmsBlockMixin,
-    InheritanceMixin,
+    InheritableFieldsMixin,
     XModuleMixin,
     EditInfoMixin,
     AuthoringMixin,

lms/djangoapps/courseware/block_render.py

@@ -50,7 +50,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
@@ -635,6 +641,7 @@ def inner_get_block(block: XBlock) -> XBlock | None:
         'call_to_action': CallToActionService(),
         'publish': EventPublishingService(user, course_id, track_function),
         'enrollments': EnrollmentsService(),
+        'problem_feedback': partial(ProblemFeedbackService, user_is_staff=user_is_staff),
     }
 
     runtime.get_block_for_descriptor = inner_get_block

lms/djangoapps/courseware/field_overrides.py

@@ -24,7 +24,7 @@
 from edx_django_utils.cache import DEFAULT_REQUEST_CACHE
 from xblock.field_data import FieldData
 
-from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.modulestore.inheritance import InheritableFieldsMixin
 
 NOTSET = object()
 ENABLED_OVERRIDE_PROVIDERS_KEY = 'lms.djangoapps.courseware.field_overrides.enabled_providers.{course_id}'
@@ -234,7 +234,7 @@ def has(self, block, name):
             # If this is an inheritable field and an override is set above,
             # then we want to return False here, so the field_data uses the
             # override and not the original value for this block.
-            inheritable = list(InheritanceMixin.fields.keys())  # pylint: disable=no-member
+            inheritable = list(InheritableFieldsMixin.fields.keys())  # pylint: disable=no-member
             if name in inheritable:
                 for ancestor in _lineage(block):
                     if self.get_override(ancestor, name) is not NOTSET:
@@ -249,7 +249,7 @@ def default(self, block, name):
         # The `default` method is overloaded by the field storage system to
         # also handle inheritance.
         if self.providers and not overrides_disabled():
-            inheritable = list(InheritanceMixin.fields.keys())  # pylint: disable=no-member
+            inheritable = list(InheritableFieldsMixin.fields.keys())  # pylint: disable=no-member
             if name in inheritable:
                 for ancestor in _lineage(block):
                     value = self.get_override(ancestor, name)

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.
@@ -2360,6 +2361,7 @@ class LMSXBlockServiceBindingTest(LMSXBlockServiceMixin):
         'teams',
         'teams_configuration',
         'call_to_action',
+        'problem_feedback',
     )
     def test_expected_services_exist(self, expected_service):
         """

lms/djangoapps/grades/subsection_grade.py

@@ -12,7 +12,7 @@
 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
 
 log = getLogger(__name__)
 
@@ -23,6 +23,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 +60,11 @@ 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)
+        # NOTE: Importing here to avoid circular imports between `grades.api`
+        # and `xmodule.services`.
+        from xmodule.services import ProblemFeedbackService
+
+        return ProblemFeedbackService(block=self._block, user_is_staff=has_staff_access).correctness_available()
 
     @property
     def attempted_graded(self):

lms/envs/common.py

@@ -1636,7 +1636,7 @@ def _make_mako_template_dirs(settings):
 
 # Import after sys.path fixup
 from xmodule.modulestore.edit_info import EditInfoMixin  # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position
-from xmodule.modulestore.inheritance import InheritanceMixin  # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position
+from xmodule.modulestore.inheritance import InheritableFieldsMixin  # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position
 from xmodule.x_module import XModuleMixin  # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position
 
 # These are the Mixins that will be added to every Blocklike upon instantiation.
@@ -1649,7 +1649,7 @@ def _make_mako_template_dirs(settings):
     #  (a) merge their functionality into the base Blocklike class, or
     #  (b) refactor their functionality out of the Blocklike objects and into the edx-platform block runtimes.
     LmsBlockMixin,
-    InheritanceMixin,
+    InheritableFieldsMixin,
     XModuleMixin,
     EditInfoMixin,
 )

openedx/core/djangoapps/xblock/runtime/runtime.py

@@ -25,7 +25,8 @@
 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.modulestore.inheritance import InheritableFieldsMixin
+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
@@ -124,6 +125,7 @@ def __init__(
             mixins=(
                 LmsBlockMixin,  # Adds Non-deprecated LMS/Studio functionality
                 XBlockShim,  # Adds deprecated LMS/Studio functionality / backwards compatibility
+                InheritableFieldsMixin,  # Adds inheritable fields common for all XBlocks
             ),
             default_class=None,
             select=None,
@@ -341,6 +343,8 @@ def service(self, block: XBlock, service_name: str):
             return EnrollmentsService()
         elif service_name == 'error_tracker':
             return make_error_tracker()
+        elif service_name == 'problem_feedback':
+            return ProblemFeedbackService(block=block, user_is_staff=self.user.is_staff)  # type: ignore
 
         # Otherwise, fall back to the base implementation which loads services
         # defined in the constructor:

openedx/core/djangoapps/xblock/runtime/shims.py

@@ -328,17 +328,6 @@ def system(self):
         )
         return self.runtime
 
-    @property
-    def graded(self):
-        """
-        Not sure what this is or how it's supposed to be set. Capa seems to
-        expect a 'graded' attribute to be present on itself. Possibly through
-        contentstore's update_section_grader_type() ?
-        """
-        if self.scope_ids.block_type != 'problem':
-            raise AttributeError(".graded shim is only for capa")
-        return False
-
     # Attributes defined by XModuleMixin and sometimes used by the LMS
     # Set sensible defaults.
     # If any of these are meant to be used in new stuff (are not deprecated)

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')
 

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)