Vote buttons should be disabled while in use


Hi there,

This PR covers which was assigned to me by @antoviaque and should be covered by the OpenCraft contributor agreement.



    1. Description of changes

From the ticket:

> 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 numbers look okay.

In common/static/coffee/src/discussion/, there are two methods that are intended to prevent this kind of problem:

*safeAjax* wraps jQuery ajax, but sets a disabled attribute on the element that triggers the ajax request, and returns early if the element is already disabled. In this case it's an anchor element.

*updateWithUndo* calls safeAjax, but also registers a failure callback to rollback a set of updates to a backbone model.

The core problem is that updateWithUndo assumed that safeAjax - like $.ajax - always returns a promise, when in fact it returned undefined when another request was in progress.

I thought about making safeAjax always return a promise, but I decided against it since it would result in surprising behaviour. If the ajax request is not idempotent then it's not clear whether a skipped request should be considered a success or a failure.

I've decided to keep safeAjax essentially as it is, except for adding an explicit null return value, which I've handled everywhere that was previously expecting a promise to be returned. I don't think this is particularly elegant but I couldn't find a nicer way of doing it.

In the case where a request is not in progress when clicking the button, the behaviour should be the same as before, i.e. the visual feedback is only given after the request has successfully completed.

As I mentioned on the JIRA ticket, it may technically be still possible to trigger race conditions due to the way the element is made enabled again once safeAjax returns, but this fix seems to resolved the immediate problem.


    1. Test plan

The changes are confined to common/static/coffee/src/discussion so basic regression testing of the discussion functionality should be sufficient to test nothing new is broken.

I've tested this manually by going to either the main discussion page (/Demo_Course/discussion/forum/) or the "be social" courseware in the demo course, creating an example post and comment as a verified user, then rapidly clicking the vote button.

Beforehand, this led to vote counts becoming > 1 or < 0, whereas now it simply toggles the counter & highlighting, in a similar way to the follow button.

I've also added a new spec for safeAjax and run with ```paver test_js_run -s common```.

The server side APIs are not affected by this bug.


Sarina Canelake
January 6, 2015, 6:12 PM

I am in the process of taking a Javascript class so that I can be better at reviewing these pull requests... sigh.

David Baumgold
January 6, 2015, 6:29 PM

: The coffeescript looks good to me, so I'm moving this to "awaiting prioritization"

Sarina Canelake
January 6, 2015, 6:48 PM

generally it would be good to have comments about the code captured on the pull request.

Adam Palay
January 27, 2015, 8:23 PM

, please review this PR

Sarina Canelake
February 2, 2015, 2:14 PM

ping to review this PR, thanks



Wajeeha Khalid


Edx Admin [Administrator]


Contributor Name






Epic Link


OSCM Assignee


Platform Map Area (Levels 1 &amp; 2)


Platform Map Area (Levels 3 &amp; 4)


Blended Hour Utilization Percentage


edX Theme


edX Squad


Github Lines Added


Github Lines Deleted