Merge pull request #2440 from glowe/prevent-duplicate-checks

Temporarily disable CAPA problem "Check" button.

common/lib/xmodule/xmodule/capa_base.py

@@ -390,6 +390,24 @@ class CapaMixin(CapaFields):
         else:
             return check
 
+    def check_button_checking_name(self):
+        """
+        Return the "checking..." text for the "check" button.
+
+        After the user presses the "check" button, the button will briefly
+        display the value returned by this function until a response is
+        received by the server.
+
+        The text can be customized by the text_customization setting.
+
+        """
+        # Apply customizations if present
+        if 'custom_checking' in self.text_customization:
+            return self.text_customization.get('custom_checking')
+
+        _ = self.runtime.service(self, "i18n").ugettext
+        return _('Checking...')
+
     def should_show_check_button(self):
         """
         Return True/False to indicate whether to show the "Check" button.
@@ -548,13 +566,16 @@ class CapaMixin(CapaFields):
         except Exception as err:  # pylint: disable=broad-except
             html = self.handle_problem_html_error(err)
 
-        # The convention is to pass the name of the check button
-        # if we want to show a check button, and False otherwise
-        # This works because non-empty strings evaluate to True
+        # The convention is to pass the name of the check button if we want
+        # to show a check button, and False otherwise This works because
+        # non-empty strings evaluate to True.  We use the same convention
+        # for the "checking" state text.
         if self.should_show_check_button():
             check_button = self.check_button_name()
+            check_button_checking = self.check_button_checking_name()
         else:
             check_button = False
+            check_button_checking = False
 
         content = {
             'name': self.display_name_with_default,
@@ -566,6 +587,7 @@ class CapaMixin(CapaFields):
             'problem': content,
             'id': self.id,
             'check_button': check_button,
+            'check_button_checking': check_button_checking,
             'reset_button': self.should_show_reset_button(),
             'save_button': self.should_show_save_button(),
             'answer_available': self.answer_available(),

common/lib/xmodule/xmodule/capa_module.py

@@ -180,6 +180,7 @@ class CapaDescriptor(CapaFields, RawDescriptor):
     # Proxy to CapaModule for access to any of its attributes
     answer_available = module_attr('answer_available')
     check_button_name = module_attr('check_button_name')
+    check_button_checking_name = module_attr('check_button_checking_name')
     check_problem = module_attr('check_problem')
     choose_new_seed = module_attr('choose_new_seed')
     closed = module_attr('closed')

common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee

@@ -142,6 +142,10 @@ describe 'Problem', ->
       @problem.answers = 'foo=1&bar=2'
 
     it 'log the problem_check event', ->
+      spyOn($, 'postWithPrefix').andCallFake (url, answers, callback) ->
+        promise =
+          always: (callable) -> callable()
+          done: (callable) -> callable()
       @problem.check()
       expect(Logger.log).toHaveBeenCalledWith 'problem_check', 'foo=1&bar=2'
 
@@ -151,11 +155,17 @@ describe 'Problem', ->
           success: 'correct'
           contents: 'mock grader response'
         callback(response)
+        promise =
+          always: (callable) -> callable()
+          done: (callable) -> callable()
       @problem.check()
       expect(Logger.log).toHaveBeenCalledWith 'problem_graded', ['foo=1&bar=2', 'mock grader response'], @problem.id
 
     it 'submit the answer for check', ->
-      spyOn $, 'postWithPrefix'
+      spyOn($, 'postWithPrefix').andCallFake (url, answers, callback) ->
+        promise =
+          always: (callable) -> callable()
+          done: (callable) -> callable()
       @problem.check()
       expect($.postWithPrefix).toHaveBeenCalledWith '/problem/Problem1/problem_check',
           'foo=1&bar=2', jasmine.any(Function)
@@ -164,6 +174,9 @@ describe 'Problem', ->
       it 'call render with returned content', ->
         spyOn($, 'postWithPrefix').andCallFake (url, answers, callback) ->
           callback(success: 'correct', contents: 'Correct!')
+          promise =
+            always: (callable) -> callable()
+            done: (callable) -> callable()
         @problem.check()
         expect(@problem.el.html()).toEqual 'Correct!'
         expect(window.SR.readElts).toHaveBeenCalled()
@@ -172,6 +185,9 @@ describe 'Problem', ->
       it 'call render with returned content', ->
         spyOn($, 'postWithPrefix').andCallFake (url, answers, callback) ->
           callback(success: 'incorrect', contents: 'Incorrect!')
+          promise =
+            always: (callable) -> callable()
+            done: (callable) -> callable()
         @problem.check()
         expect(@problem.el.html()).toEqual 'Incorrect!'
         expect(window.SR.readElts).toHaveBeenCalled()

common/lib/xmodule/xmodule/js/spec/crowdsource_hinter/display_spec.coffee

-describe 'Crowdsourced hinter', ->  
+describe 'Crowdsourced hinter', ->
   beforeEach ->
     window.update_schematics = ->
     jasmine.stubRequests()
@@ -13,10 +13,13 @@ describe 'Crowdsourced hinter', ->
       spyOn($, 'postWithPrefix').andCallFake( ->
         last_argument = arguments[arguments.length - 1]
         if typeof last_argument == 'function'
-          response = 
+          response =
             success: 'incorrect'
             contents: 'mock grader response'
           last_argument(response)
+          promise =
+            always: (callable) -> callable()
+            done: (callable) -> callable()
       )
       @problem = new Problem($('#problem'))
       @problem.bind()
@@ -28,7 +31,7 @@ describe 'Crowdsourced hinter', ->
 
     it 'knows when a capa problem is graded usig check_fd.', ->
       spyOn($, 'ajaxWithPrefix').andCallFake((url, settings) ->
-        response = 
+        response =
           success: 'incorrect'
           contents: 'mock grader response'
         settings.success(response)
@@ -50,5 +53,3 @@ describe 'Crowdsourced hinter', ->
       data = ['some answers', '<thing class="correct">']
       @hinter.capture_problem('problem_graded', data, 'fake element')
       expect($.postWithPrefix).toHaveBeenCalledWith("#{@hinter.url}/get_feedback", 'some answers', jasmine.any(Function))
-
-

common/lib/xmodule/xmodule/js/src/capa/display.coffee

@@ -5,6 +5,13 @@ class @Problem
     @id = @el.data('problem-id')
     @element_id = @el.attr('id')
     @url = @el.data('url')
+
+    # has_timed_out and has_response are used to ensure that are used to
+    # ensure that we wait a minimum of ~ 1s before transitioning the check
+    # button from disabled to enabled
+    @has_timed_out = false
+    @has_response = false
+
     @render()
 
   $: (selector) ->
@@ -20,7 +27,10 @@ class @Problem
     problem_prefix = @element_id.replace(/problem_/,'')
     @inputs = @$("[id^=input_#{problem_prefix}_]")
     @$('div.action input:button').click @refreshAnswers
-    @$('div.action input.check').click @check_fd
+    @checkButton = @$('div.action input.check')
+    @checkButtonCheckText = @checkButton.val()
+    @checkButtonCheckingText = @checkButton.data('checking')
+    @checkButton.click @check_fd
     @$('div.action input.reset').click @reset
     @$('div.action button.show').click @show
     @$('div.action input.save').click @save
@@ -201,10 +211,15 @@ class @Problem
       @check()
       return
 
+    @enableCheckButton false
+
     if not window.FormData
       alert "Submission aborted! Sorry, your browser does not support file uploads. If you can, please use Chrome or Safari which have been verified to support file uploads."
+      @enableCheckButton true
       return
 
+    timeout_id = @enableCheckButtonAfterTimeout()
+
     fd = new FormData()
 
     # Sanity checks on submission
@@ -251,12 +266,17 @@ class @Problem
     @gentle_alert error_html
 
     abort_submission = file_too_large or file_not_selected or unallowed_file_submitted or required_files_not_submitted
+    if abort_submission
+      window.clearTimeout(timeout_id)
+      @enableCheckButton true
+      return
 
     settings =
       type: "POST"
       data: fd
       processData: false
       contentType: false
+      complete: @enableCheckButtonAfterResponse
       success: (response) =>
         switch response.success
           when 'incorrect', 'correct'
@@ -266,14 +286,17 @@ class @Problem
             @gentle_alert response.success
         Logger.log 'problem_graded', [@answers, response.contents], @id
 
-    if not abort_submission
-      $.ajaxWithPrefix("#{@url}/problem_check", settings)
+    $.ajaxWithPrefix("#{@url}/problem_check", settings)
 
   check: =>
     if not @check_save_waitfor(@check_internal)
       @check_internal()
 
   check_internal: =>
+    @enableCheckButton false
+
+    timeout_id = @enableCheckButtonAfterTimeout()
+
     Logger.log 'problem_check', @answers
 
     # Segment.io
@@ -281,7 +304,7 @@ class @Problem
       problem_id: @id
       answers: @answers
 
-    $.postWithPrefix "#{@url}/problem_check", @answers, (response) =>
+    $.postWithPrefix("#{@url}/problem_check", @answers, (response) =>
       switch response.success
         when 'incorrect', 'correct'
           window.SR.readElts($(response.contents).find('.status'))
@@ -289,10 +312,11 @@ class @Problem
           @updateProgress response
           if @el.hasClass 'showed'
             @el.removeClass 'showed'
-          @$('div.action input.check').focus()            
+          @$('div.action input.check').focus()
         else
           @gentle_alert response.success
       Logger.log 'problem_graded', [@answers, response.contents], @id
+    ).always(@enableCheckButtonAfterResponse)
 
   reset: =>
     Logger.log 'problem_reset', @answers
@@ -636,3 +660,29 @@ class @Problem
     choicetextgroup: (element, display) =>
       element = $(element)
       element.find("section[id^='forinput']").removeClass('choicetextgroup_show_correct')
+
+  enableCheckButton: (enable) =>
+    # Used to disable check button to reduce chance of accidental double-submissions.
+    if enable
+      @checkButton.removeClass 'is-disabled'
+      @checkButton.val(@checkButtonCheckText)
+    else
+      @checkButton.addClass 'is-disabled'
+      @checkButton.val(@checkButtonCheckingText)
+
+  enableCheckButtonAfterResponse: =>
+    @has_response = true
+    if not @has_timed_out
+      # Server has returned response before our timeout
+      @enableCheckButton false
+    else
+      @enableCheckButton true
+
+  enableCheckButtonAfterTimeout: =>
+    @has_timed_out = false
+    @has_response = false
+    enableCheckButton = () =>
+      @has_timed_out = true
+      if @has_response
+        @enableCheckButton true
+    window.setTimeout(enableCheckButton, 750)

common/lib/xmodule/xmodule/tests/test_capa_module.py

@@ -933,11 +933,19 @@ class CapaModuleTest(unittest.TestCase):
         module = CapaFactory.create(attempts=0)
         self.assertEqual(module.check_button_name(), "Check")
 
+    def test_check_button_checking_name(self):
+        module = CapaFactory.create(attempts=1, max_attempts=10)
+        self.assertEqual(module.check_button_checking_name(), "Checking...")
+
+        module = CapaFactory.create(attempts=10, max_attempts=10)
+        self.assertEqual(module.check_button_checking_name(), "Checking...")
+
     def test_check_button_name_customization(self):
-        module = CapaFactory.create(attempts=1,
-                                    max_attempts=10,
-                                    text_customization={"custom_check": "Submit", "custom_final_check": "Final Submit"}
-                                    )
+        module = CapaFactory.create(
+            attempts=1,
+            max_attempts=10,
+            text_customization={"custom_check": "Submit", "custom_final_check": "Final Submit"}
+        )
         self.assertEqual(module.check_button_name(), "Submit")
 
         module = CapaFactory.create(attempts=9,
@@ -946,6 +954,29 @@ class CapaModuleTest(unittest.TestCase):
                                     )
         self.assertEqual(module.check_button_name(), "Final Submit")
 
+    def test_check_button_checking_name_customization(self):
+        module = CapaFactory.create(
+            attempts=1,
+            max_attempts=10,
+            text_customization={
+                "custom_check": "Submit",
+                "custom_final_check": "Final Submit",
+                "custom_checking": "Checking..."
+            }
+        )
+        self.assertEqual(module.check_button_checking_name(), "Checking...")
+
+        module = CapaFactory.create(
+            attempts=9,
+            max_attempts=10,
+            text_customization={
+                "custom_check": "Submit",
+                "custom_final_check": "Final Submit",
+                "custom_checking": "Checking..."
+            }
+        )
+        self.assertEqual(module.check_button_checking_name(), "Checking...")
+
     def test_should_show_check_button(self):
 
         attempts = random.randint(1, 10)

lms/templates/problem.html

@@ -14,7 +14,7 @@
     <input type="hidden" name="problem_id" value="${ problem['name'] }" />
 
     % if check_button:
-    <input class="check ${ check_button }" type="button" value="${ check_button }" />
+    <input class="check ${ check_button }" type="button" data-checking="${ check_button_checking }" value="${ check_button }" />
     % endif
     % if reset_button:
     <input class="reset" type="button" value="${_('Reset')}" />