From d857695d620da324457c56a2cd328b2ed441b04d Mon Sep 17 00:00:00 2001 From: Mat Moore Date: Mon, 5 Jan 2015 15:19:08 +0000 Subject: [PATCH] Vote buttons should be disabled while in use Clicking quickly on up/down votes in forums generates a race condition where the votes can continue going up or down (and I can get e.g. large negative numbers). This is client-side; the server-side is not affected. This is caused by safeAjax calling code (including updateWithUndo) not handling the possible undefined return value. This is returned in the case where the element that triggers the event is already disabled. Corrected typo in DiscussionThreadView cleanup: @responseRequest -> @responsesRequest Resolves TNL-1061 --- AUTHORS | 1 + .../coffee/spec/discussion/utils_spec.coffee | 21 +++++++++++++++++++ .../static/coffee/src/discussion/utils.coffee | 6 +++++- .../views/discussion_thread_view.coffee | 2 +- 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/AUTHORS b/AUTHORS index 45b0c48603..1db15d22bb 100644 --- a/AUTHORS +++ b/AUTHORS @@ -195,3 +195,4 @@ Dino Cikatić Davorin Šego Marko Jevtić Ahsan Ulhaq +Mat Moore diff --git a/common/static/coffee/spec/discussion/utils_spec.coffee b/common/static/coffee/spec/discussion/utils_spec.coffee index e4285014e5..c456855e06 100644 --- a/common/static/coffee/spec/discussion/utils_spec.coffee +++ b/common/static/coffee/spec/discussion/utils_spec.coffee @@ -25,3 +25,24 @@ describe 'DiscussionUtil', -> # if the ajax call ends in failure, the model state should be reverted deferred.reject() expect(model.attributes).toEqual({hello: false, number: 42}) + + it "rolls back the changes if the associated element is disabled", -> + spyOn(DiscussionUtil, "safeAjax").andCallThrough() + + model = new Backbone.Model({hello: false, number: 42}) + updates = {hello: "world"} + + # This is the element that is disabled/enabled while the ajax request is + # in progress + $elem = jasmine.createSpyObj('$elem', ['attr']) + $elem.attr.andReturn(true) + + res = DiscussionUtil.updateWithUndo(model, updates, {foo: "bar", $elem:$elem}, "error message") + + expect($elem.attr).toHaveBeenCalledWith("disabled") + expect(DiscussionUtil.safeAjax).toHaveBeenCalled() + expect(model.attributes).toEqual({hello: false, number: 42}) + + failed = false + res.fail(() => failed = true) + expect(failed).toBe(true); diff --git a/common/static/coffee/src/discussion/utils.coffee b/common/static/coffee/src/discussion/utils.coffee index 0d2b31fdfd..dd6e126e6f 100644 --- a/common/static/coffee/src/discussion/utils.coffee +++ b/common/static/coffee/src/discussion/utils.coffee @@ -134,8 +134,12 @@ class @DiscussionUtil @safeAjax: (params) -> $elem = params.$elem + if $elem and $elem.attr("disabled") - return + deferred = $.Deferred() + deferred.reject() + return deferred.promise() + params["url"] = URI(params["url"]).addSearch ajax: 1 params["beforeSend"] = -> if $elem diff --git a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee index 6df9807e3f..7b315571c3 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee @@ -127,7 +127,7 @@ if Backbone? $loading: elem takeFocus: true complete: => - @responseRequest = null + @responsesRequest = null success: (data, textStatus, xhr) => Content.loadContentInfos(data['annotated_content_info']) if @isQuestion()