From 6d19a0c87c2aa9f7048bed4533fed4d6a9e13592 Mon Sep 17 00:00:00 2001 From: Jim Date: Tue, 14 Oct 2014 14:45:09 -0700 Subject: [PATCH] Support and tests for adding a reset button to units Users may want to start anew when answering a question. This commit decouples reset from randomization while still preserving backward compatibility. Users can clear their input. Instructors can set course-wide and problem-specific settings for reset button display. --- CHANGELOG.rst | 2 + .../contentstore/features/problem-editor.py | 2 + common/lib/xmodule/xmodule/capa_base.py | 120 +++--- .../xmodule/xmodule/capa_base_constants.py | 28 ++ .../xmodule/modulestore/inheritance.py | 15 +- .../xmodule/xmodule/tests/test_capa_module.py | 350 +++++++++++------- .../courseware/features/problems.feature | 122 ++++-- .../courseware/features/problems.py | 12 + 8 files changed, 430 insertions(+), 221 deletions(-) create mode 100644 common/lib/xmodule/xmodule/capa_base_constants.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b6390d58de..e2b7eb2983 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +Common: Add configurable reset button to units + LMS: Support adding cohorts from the instructor dashboard. TNL-162 LMS: Support adding students to a cohort via the instructor dashboard. TNL-163 diff --git a/cms/djangoapps/contentstore/features/problem-editor.py b/cms/djangoapps/contentstore/features/problem-editor.py index c49535b3de..7ad4675f6f 100644 --- a/cms/djangoapps/contentstore/features/problem-editor.py +++ b/cms/djangoapps/contentstore/features/problem-editor.py @@ -13,6 +13,7 @@ MAXIMUM_ATTEMPTS = "Maximum Attempts" PROBLEM_WEIGHT = "Problem Weight" RANDOMIZATION = 'Randomization' SHOW_ANSWER = "Show Answer" +SHOW_RESET_BUTTON = "Show Reset Button" TIMER_BETWEEN_ATTEMPTS = "Timer Between Attempts" MATLAB_API_KEY = "Matlab API key" @@ -102,6 +103,7 @@ def i_see_advanced_settings_with_values(step): [PROBLEM_WEIGHT, "", False], [RANDOMIZATION, "Never", False], [SHOW_ANSWER, "Finished", False], + [SHOW_RESET_BUTTON, "False", False], [TIMER_BETWEEN_ATTEMPTS, "0", False], ]) diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index c0ba5f0643..cbbee5ecac 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -29,6 +29,8 @@ from xblock.fields import Scope, String, Boolean, Dict, Integer, Float from .fields import Timedelta, Date from django.utils.timezone import UTC from .util.duedate import get_extended_due_date +from xmodule.capa_base_constants import RANDOMIZATION, SHOWANSWER +from django.conf import settings log = logging.getLogger("edx.courseware") @@ -63,9 +65,9 @@ class Randomization(String): """ def from_json(self, value): if value in ("", "true"): - return "always" + return RANDOMIZATION.ALWAYS elif value == "false": - return "per_student" + return RANDOMIZATION.PER_STUDENT return value to_json = from_json @@ -103,15 +105,15 @@ class CapaFields(object): max_attempts = Integer( display_name=_("Maximum Attempts"), help=_("Defines the number of times a student can try to answer this problem. " - "If the value is not set, infinite attempts are allowed."), + "If the value is not set, infinite attempts are allowed."), values={"min": 0}, scope=Scope.settings ) due = Date(help=_("Date that this problem is due by"), scope=Scope.settings) extended_due = Date( help=_("Date that this problem is due by for a particular student. This " - "can be set by an instructor, and will override the global due " - "date if it is set to a date that is later than the global due " - "date."), + "can be set by an instructor, and will override the global due " + "date if it is set to a date that is later than the global due " + "date."), default=None, scope=Scope.user_state, ) @@ -122,36 +124,45 @@ class CapaFields(object): showanswer = String( display_name=_("Show Answer"), help=_("Defines when to show the answer to the problem. " - "A default value can be set in Advanced Settings."), + "A default value can be set in Advanced Settings."), scope=Scope.settings, - default="finished", + default=SHOWANSWER.FINISHED, values=[ - {"display_name": _("Always"), "value": "always"}, - {"display_name": _("Answered"), "value": "answered"}, - {"display_name": _("Attempted"), "value": "attempted"}, - {"display_name": _("Closed"), "value": "closed"}, - {"display_name": _("Finished"), "value": "finished"}, - {"display_name": _("Correct or Past Due"), "value": "correct_or_past_due"}, - {"display_name": _("Past Due"), "value": "past_due"}, - {"display_name": _("Never"), "value": "never"}] + {"display_name": _("Always"), "value": SHOWANSWER.ALWAYS}, + {"display_name": _("Answered"), "value": SHOWANSWER.ANSWERED}, + {"display_name": _("Attempted"), "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}] ) force_save_button = Boolean( help=_("Whether to force the save button to appear on the page"), scope=Scope.settings, default=False ) + reset_key = "DEFAULT_SHOW_RESET_BUTTON" + default_reset_button = getattr(settings, reset_key) if hasattr(settings, reset_key) else False + show_reset_button = Boolean( + display_name=_("Show Reset Button"), + help=_("Determines whether a 'Reset' button is shown so the user may reset their answer. " + "A default value can be set in Advanced Settings."), + scope=Scope.settings, + default=default_reset_button + ) rerandomize = Randomization( display_name=_("Randomization"), help=_("Defines how often inputs are randomized when a student loads the problem. " - "This setting only applies to problems that can have randomly generated numeric values. " - "A default value can be set in Advanced Settings."), - default="never", + "This setting only applies to problems that can have randomly generated numeric values. " + "A default value can be set in Advanced Settings."), + default=RANDOMIZATION.NEVER, scope=Scope.settings, values=[ - {"display_name": _("Always"), "value": "always"}, - {"display_name": _("On Reset"), "value": "onreset"}, - {"display_name": _("Never"), "value": "never"}, - {"display_name": _("Per Student"), "value": "per_student"} + {"display_name": _("Always"), "value": RANDOMIZATION.ALWAYS}, + {"display_name": _("On Reset"), "value": RANDOMIZATION.ONRESET}, + {"display_name": _("Never"), "value": RANDOMIZATION.NEVER}, + {"display_name": _("Per Student"), "value": RANDOMIZATION.PER_STUDENT} ] ) data = String(help=_("XML data for the problem"), scope=Scope.content, default="") @@ -170,7 +181,7 @@ class CapaFields(object): weight = Float( display_name=_("Problem Weight"), help=_("Defines the number of points each problem is worth. " - "If the value is not set, each response field in the problem is worth one point."), + "If the value is not set, each response field in the problem is worth one point."), values={"min": 0, "step": .1}, scope=Scope.settings ) @@ -254,7 +265,7 @@ class CapaMixin(CapaFields): tb=cgi.escape( u''.join(['Traceback (most recent call last):\n'] + traceback.format_tb(sys.exc_info()[2]))) - ) + ) # create a dummy problem with error message instead of failing problem_text = (u'' u'Problem {url} has an error:{msg}'.format( @@ -274,9 +285,9 @@ class CapaMixin(CapaFields): """ Choose a new seed. """ - if self.rerandomize == 'never': + if self.rerandomize == RANDOMIZATION.NEVER: self.seed = 1 - elif self.rerandomize == "per_student" and hasattr(self.runtime, 'seed'): + elif self.rerandomize == RANDOMIZATION.PER_STUDENT and hasattr(self.runtime, 'seed'): # see comment on randomization_bin self.seed = randomization_bin(self.runtime.seed, unicode(self.location).encode('utf-8')) else: @@ -446,7 +457,7 @@ class CapaMixin(CapaFields): """ Return True/False to indicate whether to show the "Check" button. """ - submitted_without_reset = (self.is_submitted() and self.rerandomize == "always") + submitted_without_reset = (self.is_submitted() and self.rerandomize == RANDOMIZATION.ALWAYS) # If the problem is closed (past due / too many attempts) # then we do NOT show the "check" button @@ -463,19 +474,20 @@ class CapaMixin(CapaFields): """ is_survey_question = (self.max_attempts == 0) - if self.rerandomize in ["always", "onreset"]: + # If the problem is closed (and not a survey question with max_attempts==0), + # then do NOT show the reset button. + if (self.closed() and not is_survey_question): + return False - # If the problem is closed (and not a survey question with max_attempts==0), - # then do NOT show the reset button. - # If the problem hasn't been submitted yet, then do NOT show - # the reset button. - if (self.closed() and not is_survey_question) or not self.is_submitted(): + # Button only shows up for randomized problems if the question has been submitted + if self.rerandomize in [RANDOMIZATION.ALWAYS, RANDOMIZATION.ONRESET] and self.is_submitted(): + return True + else: + # Do NOT show the button if the problem is correct + if self.is_correct(): return False else: - return True - # Only randomized problems need a "reset" button - else: - return False + return self.show_reset_button def should_show_save_button(self): """ @@ -489,7 +501,7 @@ class CapaMixin(CapaFields): return not self.closed() else: is_survey_question = (self.max_attempts == 0) - needs_reset = self.is_submitted() and self.rerandomize == "always" + needs_reset = self.is_submitted() and self.rerandomize == RANDOMIZATION.ALWAYS # If the student has unlimited attempts, and their answers # are not randomized, then we do not need a save button @@ -503,7 +515,7 @@ class CapaMixin(CapaFields): # In those cases. the if statement below is false, # and the save button can still be displayed. # - if self.max_attempts is None and self.rerandomize != "always": + if self.max_attempts is None and self.rerandomize != RANDOMIZATION.ALWAYS: return False # If the problem is closed (and not a survey question with max_attempts==0), @@ -697,28 +709,28 @@ class CapaMixin(CapaFields): """ if self.showanswer == '': return False - elif self.showanswer == "never": + elif self.showanswer == SHOWANSWER.NEVER: return False elif self.runtime.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 == 'attempted': + elif self.showanswer == SHOWANSWER.ATTEMPTED: return self.attempts > 0 - elif self.showanswer == 'answered': + 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.lcp.done - elif self.showanswer == 'closed': + elif self.showanswer == SHOWANSWER.CLOSED: return self.closed() - elif self.showanswer == 'finished': + elif self.showanswer == SHOWANSWER.FINISHED: return self.closed() or self.is_correct() - elif self.showanswer == 'correct_or_past_due': + elif self.showanswer == SHOWANSWER.CORRECT_OR_PAST_DUE: return self.is_correct() or self.is_past_due() - elif self.showanswer == 'past_due': + elif self.showanswer == SHOWANSWER.PAST_DUE: return self.is_past_due() - elif self.showanswer == 'always': + elif self.showanswer == SHOWANSWER.ALWAYS: return True return False @@ -952,7 +964,7 @@ class CapaMixin(CapaFields): raise NotFoundError(_("Problem is closed.")) # Problem submitted. Student should reset before checking again - if self.done and self.rerandomize == "always": + if self.done and self.rerandomize == RANDOMIZATION.ALWAYS: event_info['failure'] = 'unreset' self.track_function_unmask('problem_check_fail', event_info) if dog_stats_api: @@ -1206,7 +1218,7 @@ class CapaMixin(CapaFields): # was presented to the user, with values interpolated etc, but that can be done # later if necessary. variant = '' - if self.rerandomize != 'never': + if self.rerandomize != RANDOMIZATION.NEVER: variant = self.seed is_correct = correct_map.is_correct(input_id) @@ -1333,7 +1345,7 @@ class CapaMixin(CapaFields): # Problem submitted. Student should reset before saving # again. - if self.done and self.rerandomize == "always": + if self.done and self.rerandomize == RANDOMIZATION.ALWAYS: event_info['failure'] = 'done' self.track_function_unmask('save_problem_fail', event_info) return { @@ -1357,7 +1369,7 @@ class CapaMixin(CapaFields): def reset_problem(self, _data): """ Changes problem state to unfinished -- removes student answers, - and causes problem to rerender itself. + Causes problem to rerender itself if randomization is enabled. Returns a dictionary of the form: {'success': True/False, @@ -1380,7 +1392,7 @@ class CapaMixin(CapaFields): 'error': _("Problem is closed."), } - if not self.done: + if not self.is_submitted(): event_info['failure'] = 'not_done' self.track_function_unmask('reset_problem_fail', event_info) return { @@ -1389,7 +1401,7 @@ class CapaMixin(CapaFields): 'error': _("Refresh the page and make an attempt before resetting."), } - if self.rerandomize in ["always", "onreset"]: + if self.is_submitted() and self.rerandomize in [RANDOMIZATION.ALWAYS, RANDOMIZATION.ONRESET]: # Reset random number generator seed. self.choose_new_seed() diff --git a/common/lib/xmodule/xmodule/capa_base_constants.py b/common/lib/xmodule/xmodule/capa_base_constants.py new file mode 100644 index 0000000000..20eab88a07 --- /dev/null +++ b/common/lib/xmodule/xmodule/capa_base_constants.py @@ -0,0 +1,28 @@ +# -*- coding: utf-8 -*- +""" +Constants for capa_base problems +""" + + +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" + + +class RANDOMIZATION: + """ + Constants for problem randomization + """ + ALWAYS = "always" + ONRESET = "onreset" + NEVER = "never" + PER_STUDENT = "per_student" diff --git a/common/lib/xmodule/xmodule/modulestore/inheritance.py b/common/lib/xmodule/xmodule/modulestore/inheritance.py index a8792d4917..607ce2f967 100644 --- a/common/lib/xmodule/xmodule/modulestore/inheritance.py +++ b/common/lib/xmodule/xmodule/modulestore/inheritance.py @@ -1,15 +1,16 @@ """ Support for inheritance of fields down an XBlock hierarchy. """ +from __future__ import absolute_import from datetime import datetime from pytz import UTC - from xmodule.partitions.partitions import UserPartition from xblock.fields import Scope, Boolean, String, Float, XBlockMixin, Dict, Integer, List from xblock.runtime import KeyValueStore, KvsFieldData - from xmodule.fields import Date, Timedelta +from django.conf import settings + # Make '_' a no-op so we can scrape strings _ = lambda text: text @@ -153,6 +154,16 @@ class InheritanceMixin(XBlockMixin): scope=Scope.settings ) + reset_key = "DEFAULT_SHOW_RESET_BUTTON" + default_reset_button = getattr(settings, reset_key) if hasattr(settings, reset_key) else False + show_reset_button = Boolean( + display_name=_("Show Reset Button for Problems"), + help=_("Enter true or false. If true, problems default to displaying a 'Reset' button. This value may be " + "overriden in each problem's settings. Existing problems whose reset setting have not been changed are affected."), + scope=Scope.settings, + default=default_reset_button + ) + def compute_inherited_metadata(descriptor): """Given a descriptor, traverse all of its descendants and do metadata diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 2bf73ec1a2..439a2a0cf4 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -13,6 +13,7 @@ import random import os import textwrap import unittest +import ddt from mock import Mock, patch import webob @@ -31,6 +32,7 @@ from xblock.fields import ScopeIds from . import get_test_system from pytz import UTC from capa.correctmap import CorrectMap +from ..capa_base_constants import RANDOMIZATION class CapaFactory(object): @@ -140,7 +142,6 @@ class CapaFactory(object): return module - class CapaFactoryWithFiles(CapaFactory): """ A factory for creating a Capa problem with files attached. @@ -182,6 +183,7 @@ if submission[0] == '': """) +@ddt.ddt class CapaModuleTest(unittest.TestCase): def setUp(self): @@ -540,39 +542,42 @@ class CapaModuleTest(unittest.TestCase): # Expect that number of attempts NOT incremented self.assertEqual(module.attempts, 3) - def test_check_problem_resubmitted_with_randomize(self): - rerandomize_values = ['always', 'true'] + @ddt.data( + RANDOMIZATION.ALWAYS, + 'true' + ) + def test_check_problem_resubmitted_with_randomize(self, rerandomize): + # Randomize turned on + module = CapaFactory.create(rerandomize=rerandomize, attempts=0) - for rerandomize in rerandomize_values: - # Randomize turned on - module = CapaFactory.create(rerandomize=rerandomize, attempts=0) + # Simulate that the problem is completed + module.done = True - # Simulate that the problem is completed - module.done = True - - # Expect that we cannot submit - with self.assertRaises(xmodule.exceptions.NotFoundError): - get_request_dict = {CapaFactory.input_key(): '3.14'} - module.check_problem(get_request_dict) - - # Expect that number of attempts NOT incremented - self.assertEqual(module.attempts, 0) - - def test_check_problem_resubmitted_no_randomize(self): - rerandomize_values = ['never', 'false', 'per_student'] - - for rerandomize in rerandomize_values: - # Randomize turned off - module = CapaFactory.create(rerandomize=rerandomize, attempts=0, done=True) - - # Expect that we can submit successfully + # Expect that we cannot submit + with self.assertRaises(xmodule.exceptions.NotFoundError): get_request_dict = {CapaFactory.input_key(): '3.14'} - result = module.check_problem(get_request_dict) + module.check_problem(get_request_dict) - self.assertEqual(result['success'], 'correct') + # Expect that number of attempts NOT incremented + self.assertEqual(module.attempts, 0) - # Expect that number of attempts IS incremented - self.assertEqual(module.attempts, 1) + @ddt.data( + RANDOMIZATION.NEVER, + 'false', + RANDOMIZATION.PER_STUDENT + ) + def test_check_problem_resubmitted_no_randomize(self, rerandomize): + # Randomize turned off + module = CapaFactory.create(rerandomize=rerandomize, attempts=0, done=True) + + # Expect that we can submit successfully + get_request_dict = {CapaFactory.input_key(): '3.14'} + result = module.check_problem(get_request_dict) + + self.assertEqual(result['success'], 'correct') + + # Expect that number of attempts IS incremented + self.assertEqual(module.attempts, 1) def test_check_problem_queued(self): module = CapaFactory.create(attempts=1) @@ -813,7 +818,7 @@ class CapaModuleTest(unittest.TestCase): def test_reset_problem_closed(self): # pre studio default - module = CapaFactory.create(rerandomize="always") + module = CapaFactory.create(rerandomize=RANDOMIZATION.ALWAYS) # Simulate that the problem is closed with patch('xmodule.capa_module.CapaModule.closed') as mock_closed: @@ -944,35 +949,36 @@ class CapaModuleTest(unittest.TestCase): # Expect that the result is failure self.assertTrue('success' in result and not result['success']) - def test_save_problem_submitted_with_randomize(self): - + @ddt.data( + RANDOMIZATION.ALWAYS, + 'true' + ) + def test_save_problem_submitted_with_randomize(self, rerandomize): # Capa XModule treats 'always' and 'true' equivalently - rerandomize_values = ['always', 'true'] + module = CapaFactory.create(rerandomize=rerandomize, done=True) - for rerandomize in rerandomize_values: - module = CapaFactory.create(rerandomize=rerandomize, done=True) + # Try to save + get_request_dict = {CapaFactory.input_key(): '3.14'} + result = module.save_problem(get_request_dict) - # Try to save - get_request_dict = {CapaFactory.input_key(): '3.14'} - result = module.save_problem(get_request_dict) - - # Expect that we cannot save - self.assertTrue('success' in result and not result['success']) - - def test_save_problem_submitted_no_randomize(self): + # Expect that we cannot save + self.assertTrue('success' in result and not result['success']) + @ddt.data( + RANDOMIZATION.NEVER, + 'false', + RANDOMIZATION.PER_STUDENT + ) + def test_save_problem_submitted_no_randomize(self, rerandomize): # Capa XModule treats 'false' and 'per_student' equivalently - rerandomize_values = ['never', 'false', 'per_student'] + module = CapaFactory.create(rerandomize=rerandomize, done=True) - for rerandomize in rerandomize_values: - module = CapaFactory.create(rerandomize=rerandomize, done=True) + # Try to save + get_request_dict = {CapaFactory.input_key(): '3.14'} + result = module.save_problem(get_request_dict) - # Try to save - get_request_dict = {CapaFactory.input_key(): '3.14'} - result = module.save_problem(get_request_dict) - - # Expect that we succeed - self.assertTrue('success' in result and result['success']) + # Expect that we succeed + self.assertTrue('success' in result and result['success']) def test_check_button_name(self): @@ -1066,7 +1072,7 @@ class CapaModuleTest(unittest.TestCase): # If user submitted a problem but hasn't reset, # do NOT show the check button # Note: we can only reset when rerandomize="always" or "true" - module = CapaFactory.create(rerandomize="always", done=True) + module = CapaFactory.create(rerandomize=RANDOMIZATION.ALWAYS, done=True) self.assertFalse(module.should_show_check_button()) module = CapaFactory.create(rerandomize="true", done=True) @@ -1080,13 +1086,13 @@ class CapaModuleTest(unittest.TestCase): # and we do NOT have a reset button, then we can show the check button # Setting rerandomize to "never" or "false" ensures that the reset button # is not shown - module = CapaFactory.create(rerandomize="never", done=True) + module = CapaFactory.create(rerandomize=RANDOMIZATION.NEVER, done=True) self.assertTrue(module.should_show_check_button()) module = CapaFactory.create(rerandomize="false", done=True) self.assertTrue(module.should_show_check_button()) - module = CapaFactory.create(rerandomize="per_student", done=True) + module = CapaFactory.create(rerandomize=RANDOMIZATION.PER_STUDENT, done=True) self.assertTrue(module.should_show_check_button()) def test_should_show_reset_button(self): @@ -1101,30 +1107,36 @@ class CapaModuleTest(unittest.TestCase): module = CapaFactory.create(attempts=attempts, max_attempts=attempts, done=True) self.assertFalse(module.should_show_reset_button()) - # If we're NOT randomizing, then do NOT show the reset button - module = CapaFactory.create(rerandomize="never", done=True) - self.assertFalse(module.should_show_reset_button()) - - # If we're NOT randomizing, then do NOT show the reset button - module = CapaFactory.create(rerandomize="per_student", done=True) - self.assertFalse(module.should_show_reset_button()) - - # If we're NOT randomizing, then do NOT show the reset button - module = CapaFactory.create(rerandomize="false", done=True) - self.assertFalse(module.should_show_reset_button()) - - # If the user hasn't submitted an answer yet, - # then do NOT show the reset button - module = CapaFactory.create(done=False) - self.assertFalse(module.should_show_reset_button()) - # pre studio default value, DO show the reset button - module = CapaFactory.create(rerandomize="always", done=True) + module = CapaFactory.create(rerandomize=RANDOMIZATION.ALWAYS, done=True) self.assertTrue(module.should_show_reset_button()) # If survey question for capa (max_attempts = 0), # DO show the reset button - module = CapaFactory.create(rerandomize="always", max_attempts=0, done=True) + module = CapaFactory.create(rerandomize=RANDOMIZATION.ALWAYS, max_attempts=0, done=True) + self.assertTrue(module.should_show_reset_button()) + + # If the question is not correct + # DO show the reset button + module = CapaFactory.create(rerandomize=RANDOMIZATION.ALWAYS, max_attempts=0, done=True, correct=False) + self.assertTrue(module.should_show_reset_button()) + + # If the question is correct and randomization is never + # DO not show the reset button + module = CapaFactory.create(rerandomize=RANDOMIZATION.NEVER, max_attempts=0, done=True, correct=True) + self.assertFalse(module.should_show_reset_button()) + + # If the question is correct and randomization is always + # Show the reset button + module = CapaFactory.create(rerandomize=RANDOMIZATION.ALWAYS, max_attempts=0, done=True, correct=True) + self.assertTrue(module.should_show_reset_button()) + + # Don't show reset button if randomization is turned on and the question is not done + module = CapaFactory.create(rerandomize=RANDOMIZATION.ALWAYS, show_reset_button=False, done=False) + self.assertFalse(module.should_show_reset_button()) + + # Show reset button if randomization is turned on and the problem is done + module = CapaFactory.create(rerandomize=RANDOMIZATION.ALWAYS, show_reset_button=False, done=True) self.assertTrue(module.should_show_reset_button()) def test_should_show_save_button(self): @@ -1140,7 +1152,7 @@ class CapaModuleTest(unittest.TestCase): self.assertFalse(module.should_show_save_button()) # If user submitted a problem but hasn't reset, do NOT show the save button - module = CapaFactory.create(rerandomize="always", done=True) + module = CapaFactory.create(rerandomize=RANDOMIZATION.ALWAYS, done=True) self.assertFalse(module.should_show_save_button()) module = CapaFactory.create(rerandomize="true", done=True) @@ -1149,27 +1161,27 @@ class CapaModuleTest(unittest.TestCase): # If the user has unlimited attempts and we are not randomizing, # then do NOT show a save button # because they can keep using "Check" - module = CapaFactory.create(max_attempts=None, rerandomize="never", done=False) + module = CapaFactory.create(max_attempts=None, rerandomize=RANDOMIZATION.NEVER, done=False) self.assertFalse(module.should_show_save_button()) module = CapaFactory.create(max_attempts=None, rerandomize="false", done=True) self.assertFalse(module.should_show_save_button()) - module = CapaFactory.create(max_attempts=None, rerandomize="per_student", done=True) + module = CapaFactory.create(max_attempts=None, rerandomize=RANDOMIZATION.PER_STUDENT, done=True) self.assertFalse(module.should_show_save_button()) # pre-studio default, DO show the save button - module = CapaFactory.create(rerandomize="always", done=False) + module = CapaFactory.create(rerandomize=RANDOMIZATION.ALWAYS, done=False) self.assertTrue(module.should_show_save_button()) # If we're not randomizing and we have limited attempts, then we can save - module = CapaFactory.create(rerandomize="never", max_attempts=2, done=True) + module = CapaFactory.create(rerandomize=RANDOMIZATION.NEVER, max_attempts=2, done=True) self.assertTrue(module.should_show_save_button()) module = CapaFactory.create(rerandomize="false", max_attempts=2, done=True) self.assertTrue(module.should_show_save_button()) - module = CapaFactory.create(rerandomize="per_student", max_attempts=2, done=True) + module = CapaFactory.create(rerandomize=RANDOMIZATION.PER_STUDENT, max_attempts=2, done=True) self.assertTrue(module.should_show_save_button()) # If survey question for capa (max_attempts = 0), @@ -1197,7 +1209,7 @@ class CapaModuleTest(unittest.TestCase): # then show it even if we would ordinarily # require a reset first module = CapaFactory.create(force_save_button="true", - rerandomize="always", + rerandomize=RANDOMIZATION.ALWAYS, done=True) self.assertTrue(module.should_show_save_button()) @@ -1331,48 +1343,66 @@ class CapaModuleTest(unittest.TestCase): context = render_args[1] self.assertTrue(error_msg in context['problem']['html']) - def test_random_seed_no_change(self): + @ddt.data( + 'false', + 'true', + RANDOMIZATION.NEVER, + RANDOMIZATION.PER_STUDENT, + RANDOMIZATION.ALWAYS, + RANDOMIZATION.ONRESET + ) + def test_random_seed_no_change(self, rerandomize): # Run the test for each possible rerandomize value - for rerandomize in ['false', 'never', - 'per_student', 'always', - 'true', 'onreset']: - module = CapaFactory.create(rerandomize=rerandomize) - # Get the seed - # By this point, the module should have persisted the seed - seed = module.seed - self.assertTrue(seed is not None) + module = CapaFactory.create(rerandomize=rerandomize) - # If we're not rerandomizing, the seed is always set - # to the same value (1) - if rerandomize in ['never']: - self.assertEqual(seed, 1, - msg="Seed should always be 1 when rerandomize='%s'" % rerandomize) + # Get the seed + # By this point, the module should have persisted the seed + seed = module.seed + self.assertTrue(seed is not None) - # Check the problem - get_request_dict = {CapaFactory.input_key(): '3.14'} - module.check_problem(get_request_dict) + # If we're not rerandomizing, the seed is always set + # to the same value (1) + if rerandomize == RANDOMIZATION.NEVER: + self.assertEqual(seed, 1, + msg="Seed should always be 1 when rerandomize='%s'" % rerandomize) - # Expect that the seed is the same - self.assertEqual(seed, module.seed) + # Check the problem + get_request_dict = {CapaFactory.input_key(): '3.14'} + module.check_problem(get_request_dict) - # Save the problem - module.save_problem(get_request_dict) + # Expect that the seed is the same + self.assertEqual(seed, module.seed) - # Expect that the seed is the same - self.assertEqual(seed, module.seed) + # Save the problem + module.save_problem(get_request_dict) + + # Expect that the seed is the same + self.assertEqual(seed, module.seed) + + @ddt.data( + 'false', + 'true', + RANDOMIZATION.NEVER, + RANDOMIZATION.PER_STUDENT, + RANDOMIZATION.ALWAYS, + RANDOMIZATION.ONRESET + ) + def test_random_seed_with_reset(self, rerandomize): + """ + Run the test for each possible rerandomize value + """ - def test_random_seed_with_reset(self): def _reset_and_get_seed(module): - ''' + """ Reset the XModule and return the module's seed - ''' + """ # Simulate submitting an attempt # We need to do this, or reset_problem() will - # fail with a complaint that we haven't submitted + # fail because it won't re-randomize until the problem has been submitted # the problem yet. module.done = True @@ -1397,45 +1427,83 @@ class CapaModuleTest(unittest.TestCase): break return success - # Run the test for each possible rerandomize value - for rerandomize in ['never', 'false', 'per_student', - 'always', 'true', 'onreset']: - module = CapaFactory.create(rerandomize=rerandomize) + module = CapaFactory.create(rerandomize=rerandomize, done=True) - # Get the seed - # By this point, the module should have persisted the seed - seed = module.seed - self.assertTrue(seed is not None) + # Get the seed + # By this point, the module should have persisted the seed + seed = module.seed + self.assertTrue(seed is not None) - # We do NOT want the seed to reset if rerandomize - # is set to 'never' -- it should still be 1 - # The seed also stays the same if we're randomizing - # 'per_student': the same student should see the same problem - if rerandomize in ['never', 'false', 'per_student']: - self.assertEqual(seed, _reset_and_get_seed(module)) + # We do NOT want the seed to reset if rerandomize + # is set to 'never' -- it should still be 1 + # The seed also stays the same if we're randomizing + # 'per_student': the same student should see the same problem + if rerandomize in [RANDOMIZATION.NEVER, + 'false', + RANDOMIZATION.PER_STUDENT]: + self.assertEqual(seed, _reset_and_get_seed(module)) - # Otherwise, we expect the seed to change - # to another valid seed - else: + # Otherwise, we expect the seed to change + # to another valid seed + else: - # Since there's a small chance we might get the - # same seed again, give it 5 chances - # to generate a different seed - success = _retry_and_check(5, lambda: _reset_and_get_seed(module) != seed) + # Since there's a small chance we might get the + # same seed again, give it 5 chances + # to generate a different seed + success = _retry_and_check(5, lambda: _reset_and_get_seed(module) != seed) - self.assertTrue(module.seed is not None) - msg = 'Could not get a new seed from reset after 5 tries' - self.assertTrue(success, msg) + self.assertTrue(module.seed is not None) + msg = 'Could not get a new seed from reset after 5 tries' + self.assertTrue(success, msg) - def test_random_seed_bins(self): + @ddt.data( + 'false', + 'true', + RANDOMIZATION.NEVER, + RANDOMIZATION.PER_STUDENT, + RANDOMIZATION.ALWAYS, + RANDOMIZATION.ONRESET + ) + def test_random_seed_with_reset_question_unsubmitted(self, rerandomize): + """ + Run the test for each possible rerandomize value + """ + def _reset_and_get_seed(module): + """ + Reset the XModule and return the module's seed + """ + + # Reset the problem + # By default, the problem is instantiated as unsubmitted + module.reset_problem({}) + + # Return the seed + return module.seed + + module = CapaFactory.create(rerandomize=rerandomize, done=False) + + # Get the seed + # By this point, the module should have persisted the seed + seed = module.seed + self.assertTrue(seed is not None) + + #the seed should never change because the student hasn't finished the problem + self.assertEqual(seed, _reset_and_get_seed(module)) + + @ddt.data( + RANDOMIZATION.ALWAYS, + RANDOMIZATION.PER_STUDENT, + 'true', + RANDOMIZATION.ONRESET + ) + def test_random_seed_bins(self, rerandomize): # Assert that we are limiting the number of possible seeds. - - # Check the conditions that generate random seeds - for rerandomize in ['always', 'per_student', 'true', 'onreset']: - # Get a bunch of seeds, they should all be in 0-999. - for i in range(200): - module = CapaFactory.create(rerandomize=rerandomize) - assert 0 <= module.seed < 1000 + # Get a bunch of seeds, they should all be in 0-999. + i = 200 + while i > 0: + module = CapaFactory.create(rerandomize=rerandomize) + assert 0 <= module.seed < 1000 + i -= 1 @patch('xmodule.capa_base.log') @patch('xmodule.capa_base.Progress') @@ -1765,7 +1833,7 @@ class TestProblemCheckTracking(unittest.TestCase): def test_rerandomized_inputs(self): factory = CapaFactory - module = factory.create(rerandomize='always') + module = factory.create(rerandomize=RANDOMIZATION.ALWAYS) answer_input_dict = { factory.input_key(2): '3.14' diff --git a/lms/djangoapps/courseware/features/problems.feature b/lms/djangoapps/courseware/features/problems.feature index 2985fc2432..9f9c03ef30 100644 --- a/lms/djangoapps/courseware/features/problems.feature +++ b/lms/djangoapps/courseware/features/problems.feature @@ -72,37 +72,77 @@ Feature: LMS.Answer problems Scenario: I can reset a problem - Given I am viewing a "" problem + Given I am viewing a randomization "" "" problem with reset button on And I answer a "" problem "ly" When I reset the problem Then my "" answer is marked "unanswered" And The "" problem displays a "blank" answer Examples: - | ProblemType | Correctness | - | drop down | correct | - | drop down | incorrect | - | multiple choice | correct | - | multiple choice | incorrect | - | checkbox | correct | - | checkbox | incorrect | - | radio | correct | - | radio | incorrect | - | string | correct | - | string | incorrect | - | numerical | correct | - | numerical | incorrect | - | formula | correct | - | formula | incorrect | - | script | correct | - | script | incorrect | - | radio_text | correct | - | radio_text | incorrect | - | checkbox_text | correct | - | checkbox_text | incorrect | - | image | correct | - | image | incorrect | + | ProblemType | Correctness | Randomization | + | drop down | correct | always | + | drop down | incorrect | always | + | multiple choice | correct | always | + | multiple choice | incorrect | always | + | checkbox | correct | always | + | checkbox | incorrect | always | + | radio | correct | always | + | radio | incorrect | always | + | string | correct | always | + | string | incorrect | always | + | numerical | correct | always | + | numerical | incorrect | always | + | formula | correct | always | + | formula | incorrect | always | + | script | correct | always | + | script | incorrect | always | + | radio_text | correct | always | + | radio_text | incorrect | always | + | checkbox_text | correct | always | + | checkbox_text | incorrect | always | + | image | correct | always | + | image | incorrect | always | + Scenario: I can reset a non-randomized problem that I answer incorrectly + Given I am viewing a randomization "" "" problem with reset button on + And I answer a "" problem "ly" + When I reset the problem + Then my "" answer is marked "unanswered" + And The "" problem displays a "blank" answer + + Examples: + | ProblemType | Correctness | Randomization | + | drop down | incorrect | never | + | drop down | incorrect | never | + | multiple choice | incorrect | never | + | checkbox | incorrect | never | + | radio | incorrect | never | + | string | incorrect | never | + | numerical | incorrect | never | + | formula | incorrect | never | + | script | incorrect | never | + | radio_text | incorrect | never | + | checkbox_text | incorrect | never | + | image | incorrect | never | + + Scenario: The reset button doesn't show up + Given I am viewing a randomization "" "" problem with reset button on + And I answer a "" problem "ly" + Then The "Reset" button does not appear + + Examples: + | ProblemType | Correctness | Randomization | + | drop down | correct | never | + | multiple choice | correct | never | + | checkbox | correct | never | + | radio | correct | never | + | string | correct | never | + | numerical | correct | never | + | formula | correct | never | + | script | correct | never | + | radio_text | correct | never | + | checkbox_text | correct | never | + | image | correct | never | Scenario: I can answer a problem with one attempt correctly and not reset Given I am viewing a "multiple choice" problem with "1" attempt @@ -115,6 +155,12 @@ Feature: LMS.Answer problems When I answer a "multiple choice" problem "correctly" Then The "Reset" button does appear + Scenario: I can answer a problem with multiple attempts correctly but cannot reset because randomization is off + Given I am viewing a randomization "never" "multiple choice" problem with "3" attempts with reset + Then I should see "You have used 0 of 3 submissions" somewhere in the page + When I answer a "multiple choice" problem "correctly" + Then The "Reset" button does not appear + Scenario: I can view how many attempts I have left on a problem Given I am viewing a "multiple choice" problem with "3" attempts Then I should see "You have used 0 of 3 submissions" somewhere in the page @@ -165,6 +211,34 @@ Feature: LMS.Answer problems | image | correct | 1/1 point | 1 point possible | | image | incorrect | 1 point possible | 1 point possible | + Scenario: I can see my score on a problem when I answer it and after I reset it + Given I am viewing a "" problem with randomization "" with reset button on + When I answer a "" problem "ly" + Then I should see a score of "" + When I reset the problem + Then I should see a score of "" + + Examples: + | ProblemType | Correctness | Score | Points Possible | Randomization | + | drop down | correct | 1/1 point | 1 point possible | never | + | drop down | incorrect | 1 point possible | 1 point possible | never | + | multiple choice | correct | 1/1 point | 1 point possible | never | + | multiple choice | incorrect | 1 point possible | 1 point possible | never | + | checkbox | correct | 1/1 point | 1 point possible | never | + | checkbox | incorrect | 1 point possible | 1 point possible | never | + | radio | correct | 1/1 point | 1 point possible | never | + | radio | incorrect | 1 point possible | 1 point possible | never | + | string | correct | 1/1 point | 1 point possible | never | + | string | incorrect | 1 point possible | 1 point possible | never | + | numerical | correct | 1/1 point | 1 point possible | never | + | numerical | incorrect | 1 point possible | 1 point possible | never | + | formula | correct | 1/1 point | 1 point possible | never | + | formula | incorrect | 1 point possible | 1 point possible | never | + | script | correct | 2/2 points | 2 points possible | never | + | script | incorrect | 2 points possible | 2 points possible | never | + | image | correct | 1/1 point | 1 point possible | never | + | image | incorrect | 1 point possible | 1 point possible | never | + Scenario: I can see my score on a problem to which I submit a blank answer Given I am viewing a "" problem When I check a problem diff --git a/lms/djangoapps/courseware/features/problems.py b/lms/djangoapps/courseware/features/problems.py index 6e31af897c..3d578044ff 100644 --- a/lms/djangoapps/courseware/features/problems.py +++ b/lms/djangoapps/courseware/features/problems.py @@ -26,6 +26,13 @@ def view_problem_with_attempts(step, problem_type, attempts): _view_problem(step, problem_type, {'max_attempts': attempts}) +@step(u'I am viewing a randomization "([^"]*)" "([^"]*)" problem with "([^"]*)" attempts with reset') +def view_problem_attempts_reset(step, randomization, problem_type, attempts, ): + _view_problem(step, problem_type, {'max_attempts': attempts, + 'rerandomize': randomization, + 'show_reset_button': True}) + + @step(u'I am viewing a "([^"]*)" that shows the answer "([^"]*)"') def view_problem_with_show_answer(step, problem_type, answer): _view_problem(step, problem_type, {'showanswer': answer}) @@ -36,6 +43,11 @@ def view_problem(step, problem_type): _view_problem(step, problem_type) +@step(u'I am viewing a randomization "([^"]*)" "([^"]*)" problem with reset button on') +def view_random_reset_problem(step, randomization, problem_type): + _view_problem(step, problem_type, {'rerandomize': randomization, 'show_reset_button': True}) + + @step(u'External graders respond "([^"]*)"') def set_external_grader_response(step, correctness): assert(correctness in ['correct', 'incorrect'])