Submit only one comment at a time in SG

Do not attempt to submit comments in SpeedGrader if a submission request
is already in progress. Although we already disable the "Submit" button
while waiting for a response, navigating between students would still
attempt to record the current comment text as a draft (even if the user
had already explicitly saved it) without checking whether a request was
in progress.

flag=none
fixes TALLY-620

Test plan:

- Have a course with an assignment and some students
- Open SpeedGrader for the assignment

- Ensure the server will take a while to process comment requests
  - To do this, you can make a change on the server to force a reload
    (e.g., add a line to update_submission in gradebooks_controller),
    or get yourself a terrible internet connection if you're not testing
    locally.

- Ensure non-draft comments aren't wrongly saved as drafts:
  - Type some text in the comment box for a student
  - Click "Submit"
  - Immediately start hitting the "j" and/or "k" keys with great vigor
  - This should *not* cause a zillion draft comments to be saved
  - Eventually the actual submission request should finish, and should
    add a non-draft comment for the intended student (and only that
    student)
- Ensure draft comments only get one copy saved:
  - Slow down your server again, as above
  - Type some comment text, but do not submit it
  - Using the arrow buttons at the top, furiously move between students
  - This should *not* cause zillions of draft comments to be saved
  - Eventually, the actual draft comment should be saved for the
    intended student (and only that student)
- Ensure the more civilized behavior below works as expected:
  - Submitting a comment and calmly waiting for the submission to go
    through
  - Typing some comment text, switching students, and waiting for the
    draft comment to be saved without making a ruckus
  - Typing some comment text, closing the page, and then re-opening the
    page with no hullabaloo (the text should have been saved as a draft)

Change-Id: I57e0781333c763ba22842749f0130fe6c21eb319
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/237373
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Spencer Olson <solson@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
This commit is contained in:
Adrian Packel 2020-05-12 12:52:55 -05:00
parent 1c0ac2e274
commit f7e168c77b
2 changed files with 57 additions and 0 deletions

View File

@ -114,6 +114,8 @@ let anonymizableStudentId
let anonymizableAuthorId
let isModerated
let commentSubmissionInProgress
let $window
let $full_width_container
let $left_side
@ -2860,6 +2862,13 @@ EG = {
},
addSubmissionComment(draftComment) {
// Avoid submitting additional comments if a request is already in progress.
// This can happen if the user submits a comment and then switches students
// (which attempts to save a draft comment) before the request finishes.
if (commentSubmissionInProgress) {
return false
}
// This is to continue existing behavior of creating finalized comments by default
if (draftComment === undefined) {
draftComment = false
@ -2875,6 +2884,8 @@ EG = {
// that means that they did not type a comment, attach a file or record any media. so dont do anything.
return false
}
commentSubmissionInProgress = true
const url = `${assignmentUrl}/${isAnonymous ? 'anonymous_' : ''}submissions/${
EG.currentStudent[anonymizableId]
}`
@ -2909,11 +2920,13 @@ EG = {
window.setTimeout(() => {
$rightside_inner.scrollTo($rightside_inner[0].scrollHeight, 500)
})
commentSubmissionInProgress = false
}
const formError = (data, _xhr, _textStatus, _errorThrown) => {
EG.handleGradingError(data)
EG.revertFromFormSubmit({errorSubmitting: true})
commentSubmissionInProgress = false
}
if ($add_a_comment.find("input[type='file']:visible").length) {
@ -3759,6 +3772,8 @@ export default {
speedGraderJSONErrorFn
)
commentSubmissionInProgress = false
$.when(getGradingPeriods(), speedGraderJsonDfd).then(setupSpeedGrader)
// run the stuff that just attaches event handlers and dom stuff, but does not need the jsonData

View File

@ -4630,6 +4630,48 @@ QUnit.module('SpeedGrader', rootHooks => {
revertFromFormSubmit.restore()
})
test('does not submit a comment request if one is already in progress', () => {
$.ajaxJSON.resetHistory()
SpeedGrader.EG.addSubmissionComment('hello')
SpeedGrader.EG.addSubmissionComment('hello???')
strictEqual($.ajaxJSON.callCount, 1)
})
test('submits a comment request if the preceding request succeeded', () => {
$.ajaxJSON.resetHistory()
$.ajaxJSON.callsFake((_url, _method, _form, success, _error) => {
sinon.stub(SpeedGrader.EG, 'revertFromFormSubmit')
sinon.stub(window, 'setTimeout')
success([])
window.setTimeout.restore()
SpeedGrader.EG.revertFromFormSubmit.restore()
})
SpeedGrader.EG.addSubmissionComment('ok')
SpeedGrader.EG.addSubmissionComment('ok!')
strictEqual($.ajaxJSON.callCount, 2)
})
test('submits a comment request if the preceding request failed', () => {
$.ajaxJSON.resetHistory()
$.ajaxJSON.callsFake((_url, _method, _form, _success, error) => {
sinon.stub(SpeedGrader.EG, 'revertFromFormSubmit')
sinon.stub(SpeedGrader.EG, 'handleGradingError')
error()
SpeedGrader.EG.handleGradingError.restore()
SpeedGrader.EG.revertFromFormSubmit.restore()
})
SpeedGrader.EG.addSubmissionComment('no')
SpeedGrader.EG.addSubmissionComment('noooooo')
strictEqual($.ajaxJSON.callCount, 2)
})
})
QUnit.module('#handleGradeSubmit', hooks => {