Skip to content

E
edx-platform
Commit 7eb4970f by Victor Shnayder
Options

Merge pull request #1671 from MITx/fix/will/save_button_regression 

Fix/will/save button regression
parents 4b5837ff ce5e8689
Showing with 67 additions and 41 deletions
common/lib/xmodule/xmodule/capa_module.py
......@@ -310,11 +310,26 @@ class CapaModule(XModule):
is_survey_question = (self.max_attempts == 0)
needs_reset = self.is_completed() and self.rerandomize == "always"
# If the student has unlimited attempts, and their answers
# are not randomized, then we do not need a save button
# because they can use the "Check" button without consequences.
#
# The consequences we want to avoid are:
# * Using up an attempt (if max_attempts is set)
# * Changing the current problem, and no longer being
# able to view it (if rerandomize is "always")
#
# 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":
return False
# If the problem is closed (and not a survey question with max_attempts==0),
# then do NOT show the reset button
# then do NOT show the save button
# If we're waiting for the user to reset a randomized problem
# then do NOT show the reset button
if (self.closed() and not is_survey_question) or needs_reset:
# then do NOT show the save button
elif (self.closed() and not is_survey_question) or needs_reset:
return False
else:
return True
......@@ -403,7 +418,7 @@ class CapaModule(XModule):
# if we want to show a check button, and False otherwise
# This works because non-empty strings evaluate to True
if self.should_show_check_button():
check_button = self.check_button_name()
check_button = self.check_button_name()
else:
check_button = False
......@@ -556,7 +571,7 @@ class CapaModule(XModule):
else:
answers = self.lcp.get_question_answers()
# answers (eg <solution>) may have embedded images
# answers (eg <solution>) may have embedded images
# but be careful, some problems are using non-string answer dicts
new_answers = dict()
for answer_id in answers:
......@@ -606,7 +621,7 @@ class CapaModule(XModule):
to 'input_1' in the returned dict)
'''
answers = dict()
for key in get:
# e.g. input_resistor_1 ==> resistor_1
_, _, name = key.partition('_')
......@@ -729,7 +744,7 @@ class CapaModule(XModule):
event_info['answers'] = answers
# Too late. Cannot submit
if self.closed() and not self.max_attempts==0:
if self.closed() and not self.max_attempts ==0:
event_info['failure'] = 'closed'
self.system.track_function('save_problem_fail', event_info)
return {'success': False,
......@@ -747,7 +762,7 @@ class CapaModule(XModule):
self.system.track_function('save_problem_success', event_info)
msg = "Your answers have been saved"
if not self.max_attempts==0:
if not self.max_attempts ==0:
msg += " but not graded. Hit 'Check' to grade them."
return {'success': True,
'msg': msg}
......@@ -784,7 +799,7 @@ class CapaModule(XModule):
# reset random number generator seed (note the self.lcp.get_state()
# in next line)
self.lcp.seed = None
self.lcp = LoncapaProblem(self.definition['data'],
self.location.html_id(), self.lcp.get_state(),
......@@ -793,7 +808,7 @@ class CapaModule(XModule):
event_info['new_state'] = self.lcp.get_state()
self.system.track_function('reset_problem', event_info)
return { 'success': True,
return {'success': True,
'html': self.get_problem_html(encapsulate=False)}
......@@ -821,13 +836,13 @@ class CapaDescriptor(RawDescriptor):
def get_context(self):
_context = RawDescriptor.get_context(self)
_context.update({'markdown': self.metadata.get('markdown', ''),
'enable_markdown' : 'markdown' in self.metadata})
'enable_markdown': 'markdown' in self.metadata})
return _context
@property
def editable_metadata_fields(self):
"""Remove any metadata from the editable fields which have their own editor or shouldn't be edited by user."""
subset = [field for field in super(CapaDescriptor,self).editable_metadata_fields
subset = [field for field in super(CapaDescriptor, self).editable_metadata_fields
if field not in ['markdown', 'empty']]
return subset
......
common/lib/xmodule/xmodule/tests/test_capa_module.py
......@@ -356,7 +356,7 @@ class CapaModuleTest(unittest.TestCase):
valid_get_dict = self._querydict_from_dict({'input_2[]': ['test1', 'test2']})
result = CapaModule.make_dict_of_responses(valid_get_dict)
self.assertTrue('2' in result)
self.assertEqual(['test1','test2'], result['2'])
self.assertEqual(['test1', 'test2'], result['2'])
# If we use [] at the end of a key name, we should always
# get a list, even if there's just one value
......@@ -374,7 +374,7 @@ class CapaModuleTest(unittest.TestCase):
# One of the values would overwrite the other, so detect this
# and raise an exception
invalid_get_dict = self._querydict_from_dict({'input_1[]': 'test 1',
'input_1': 'test 2' })
'input_1': 'test 2'})
with self.assertRaises(ValueError):
result = CapaModule.make_dict_of_responses(invalid_get_dict)
......@@ -412,7 +412,7 @@ class CapaModuleTest(unittest.TestCase):
mock_html.return_value = "Test HTML"
# Check the problem
get_request_dict = { CapaFactory.input_key(): '3.14' }
get_request_dict = { CapaFactory.input_key(): '3.14'}
result = module.check_problem(get_request_dict)
# Expect that the problem is marked correct
......@@ -434,7 +434,7 @@ class CapaModuleTest(unittest.TestCase):
mock_is_correct.return_value = False
# Check the problem
get_request_dict = { CapaFactory.input_key(): '0' }
get_request_dict = { CapaFactory.input_key(): '0'}
result = module.check_problem(get_request_dict)
# Expect that the problem is marked correct
......@@ -452,7 +452,7 @@ class CapaModuleTest(unittest.TestCase):
with patch('xmodule.capa_module.CapaModule.closed') as mock_closed:
mock_closed.return_value = True
with self.assertRaises(xmodule.exceptions.NotFoundError):
get_request_dict = { CapaFactory.input_key(): '3.14' }
get_request_dict = { CapaFactory.input_key(): '3.14'}
module.check_problem(get_request_dict)
# Expect that number of attempts NOT incremented
......@@ -468,7 +468,7 @@ class CapaModuleTest(unittest.TestCase):
# Expect that we cannot submit
with self.assertRaises(xmodule.exceptions.NotFoundError):
get_request_dict = { CapaFactory.input_key(): '3.14' }
get_request_dict = { CapaFactory.input_key(): '3.14'}
module.check_problem(get_request_dict)
# Expect that number of attempts NOT incremented
......@@ -483,7 +483,7 @@ class CapaModuleTest(unittest.TestCase):
module.lcp.done = True
# Expect that we can submit successfully
get_request_dict = { CapaFactory.input_key(): '3.14' }
get_request_dict = { CapaFactory.input_key(): '3.14'}
result = module.check_problem(get_request_dict)
self.assertEqual(result['success'], 'correct')
......@@ -504,7 +504,7 @@ class CapaModuleTest(unittest.TestCase):
mock_is_queued.return_value = True
mock_get_queuetime.return_value = datetime.datetime.now()
get_request_dict = { CapaFactory.input_key(): '3.14' }
get_request_dict = { CapaFactory.input_key(): '3.14'}
result = module.check_problem(get_request_dict)
# Expect an AJAX alert message in 'success'
......@@ -521,7 +521,7 @@ class CapaModuleTest(unittest.TestCase):
with patch('capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade:
mock_grade.side_effect = capa.responsetypes.StudentInputError('test error')
get_request_dict = { CapaFactory.input_key(): '3.14' }
get_request_dict = { CapaFactory.input_key(): '3.14'}
result = module.check_problem(get_request_dict)
# Expect an AJAX alert message in 'success'
......@@ -595,11 +595,11 @@ class CapaModuleTest(unittest.TestCase):
module.lcp.done = False
# Save the problem
get_request_dict = { CapaFactory.input_key(): '3.14' }
get_request_dict = { CapaFactory.input_key(): '3.14'}
result = module.save_problem(get_request_dict)
# Expect that answers are saved to the problem
expected_answers = { CapaFactory.answer_key(): '3.14' }
expected_answers = { CapaFactory.answer_key(): '3.14'}
self.assertEqual(module.lcp.student_answers, expected_answers)
# Expect that the result is success
......@@ -617,7 +617,7 @@ class CapaModuleTest(unittest.TestCase):
mock_closed.return_value = True
# Try to save the problem
get_request_dict = { CapaFactory.input_key(): '3.14' }
get_request_dict = { CapaFactory.input_key(): '3.14'}
result = module.save_problem(get_request_dict)
# Expect that the result is failure
......@@ -631,7 +631,7 @@ class CapaModuleTest(unittest.TestCase):
module.lcp.done = True
# Try to save
get_request_dict = { CapaFactory.input_key(): '3.14' }
get_request_dict = { CapaFactory.input_key(): '3.14'}
result = module.save_problem(get_request_dict)
# Expect that we cannot save
......@@ -645,7 +645,7 @@ class CapaModuleTest(unittest.TestCase):
module.lcp.done = True
# Try to save
get_request_dict = { CapaFactory.input_key(): '3.14' }
get_request_dict = { CapaFactory.input_key(): '3.14'}
result = module.save_problem(get_request_dict)
# Expect that we succeed
......@@ -657,7 +657,7 @@ class CapaModuleTest(unittest.TestCase):
# Just in case, we also check what happens if we have
# more attempts than allowed.
attempts = random.randint(1, 10)
module = CapaFactory.create(attempts=attempts-1, max_attempts=attempts)
module = CapaFactory.create(attempts=attempts -1, max_attempts=attempts)
self.assertEqual(module.check_button_name(), "Final Check")
module = CapaFactory.create(attempts=attempts, max_attempts=attempts)
......@@ -667,14 +667,14 @@ class CapaModuleTest(unittest.TestCase):
self.assertEqual(module.check_button_name(), "Final Check")
# Otherwise, button name is "Check"
module = CapaFactory.create(attempts=attempts-2, max_attempts=attempts)
module = CapaFactory.create(attempts=attempts -2, max_attempts=attempts)
self.assertEqual(module.check_button_name(), "Check")
module = CapaFactory.create(attempts=attempts-3, max_attempts=attempts)
module = CapaFactory.create(attempts=attempts -3, max_attempts=attempts)
self.assertEqual(module.check_button_name(), "Check")
# If no limit on attempts, then always show "Check"
module = CapaFactory.create(attempts=attempts-3)
module = CapaFactory.create(attempts=attempts -3)
self.assertEqual(module.check_button_name(), "Check")
module = CapaFactory.create(attempts=0)
......@@ -682,7 +682,7 @@ class CapaModuleTest(unittest.TestCase):
def test_should_show_check_button(self):
attempts = random.randint(1,10)
attempts = random.randint(1, 10)
# If we're after the deadline, do NOT show check button
module = CapaFactory.create(due=self.yesterday_str)
......@@ -718,7 +718,7 @@ class CapaModuleTest(unittest.TestCase):
def test_should_show_reset_button(self):
attempts = random.randint(1,10)
attempts = random.randint(1, 10)
# If we're after the deadline, do NOT show the reset button
module = CapaFactory.create(due=self.yesterday_str)
......@@ -755,7 +755,7 @@ class CapaModuleTest(unittest.TestCase):
def test_should_show_save_button(self):
attempts = random.randint(1,10)
attempts = random.randint(1, 10)
# If we're after the deadline, do NOT show the save button
module = CapaFactory.create(due=self.yesterday_str)
......@@ -772,13 +772,24 @@ class CapaModuleTest(unittest.TestCase):
module.lcp.done = True
self.assertFalse(module.should_show_save_button())
# 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")
module.lcp.done = False
self.assertFalse(module.should_show_save_button())
module = CapaFactory.create(max_attempts=None, rerandomize="never")
module.lcp.done = True
self.assertFalse(module.should_show_save_button())
# Otherwise, DO show the save button
module = CapaFactory.create()
module.lcp.done = False
self.assertTrue(module.should_show_save_button())
# If we're not randomizing, then we can re-save
module = CapaFactory.create(rerandomize="never")
# If we're not randomizing and we have limited attempts, then we can save
module = CapaFactory.create(rerandomize="never", max_attempts=2)
module.lcp.done = True
self.assertTrue(module.should_show_save_button())
......@@ -797,7 +808,7 @@ class CapaModuleTest(unittest.TestCase):
self.assertFalse(module.should_show_save_button())
# If the user is out of attempts, do NOT show the save button
attempts = random.randint(1,10)
attempts = random.randint(1, 10)
module = CapaFactory.create(attempts=attempts,
max_attempts=attempts,
force_save_button="true")
......@@ -823,9 +834,9 @@ class CapaModuleTest(unittest.TestCase):
# We've tested the show/hide button logic in other tests,
# so here we hard-wire the values
show_check_button = bool(random.randint(0,1) % 2)
show_reset_button = bool(random.randint(0,1) % 2)
show_save_button = bool(random.randint(0,1) % 2)
show_check_button = bool(random.randint(0, 1) % 2)
show_reset_button = bool(random.randint(0, 1) % 2)
show_save_button = bool(random.randint(0, 1) % 2)
module.should_show_check_button = Mock(return_value=show_check_button)
module.should_show_reset_button = Mock(return_value=show_reset_button)
......@@ -848,7 +859,7 @@ class CapaModuleTest(unittest.TestCase):
self.assertEqual(html, "<div>Test Template HTML</div>")
# Check the rendering context
render_args,_ = module.system.render_template.call_args
render_args, _ = module.system.render_template.call_args
self.assertEqual(len(render_args), 2)
template_name = render_args[0]
......@@ -889,7 +900,7 @@ class CapaModuleTest(unittest.TestCase):
html = module.get_problem_html()
# Check the rendering context
render_args,_ = module.system.render_template.call_args
render_args, _ = module.system.render_template.call_args
context = render_args[1]
self.assertTrue("error" in context['problem']['html'])
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment