Skip to content

Commit

Permalink
refactor: create runtime services for showCorrectness and showAnswer
Browse files Browse the repository at this point in the history
  • Loading branch information
steff456 committed Oct 15, 2023
1 parent d92dee3 commit 47d081b
Show file tree
Hide file tree
Showing 16 changed files with 242 additions and 201 deletions.
12 changes: 10 additions & 2 deletions cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions cms/djangoapps/contentstore/views/preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@
ConfigurationService,
SettingsService,
TeamsConfigurationService,
ShowAnswerService,
ShowCorrectnessService
) # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.tabs import (
CourseTabList,
Expand Down Expand Up @@ -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
Expand Down
11 changes: 10 additions & 1 deletion lms/djangoapps/courseware/block_render.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions lms/djangoapps/courseware/tests/test_block_render.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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):
"""
Expand Down
5 changes: 3 additions & 2 deletions lms/djangoapps/grades/subsection_grade.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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):
Expand Down
6 changes: 5 additions & 1 deletion openedx/core/djangoapps/xblock/runtime/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down
26 changes: 5 additions & 21 deletions xmodule/capa_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand All @@ -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,
Expand Down
69 changes: 2 additions & 67 deletions xmodule/graders.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
"""
Expand All @@ -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
Loading

0 comments on commit 47d081b

Please sign in to comment.