Return promises from action thunks

Returning promises from the `linkDwell` and `abandon` thunks and
removing some of the `wait` stubs in the unit test for these actions.

Also converting a fetch callback from a `.fail` to a Promise A+
compatible `.catch`.

Bug: T170807
Change-Id: I4bbf2863db090e222ba926d3bc36a99da4bdb601

src/actions.js

@@ -121,14 +121,13 @@ export function fetch( gateway, title, el, token ) {
 
 				return result;
 			} )
-			// FIXME: Convert to Promises A/A+ when "T124742: Upgrade to jQuery 3" is
+			.catch( function ( err ) {
-			// fully rolled out by changing fail to catch, and re-throwing the error
-			// to keep the promise in a rejected state.
-			.fail( function () {
 				dispatch( {
 					type: types.FETCH_FAILED,
 					el: el
 				} );
+				// Keep the request promise in a rejected status since it failed.
+				throw err;
 			} );
 
 		return $.when(
@@ -206,10 +205,10 @@ export function linkDwell( title, el, event, gateway, generateToken ) {
 		dispatch( action );
 
 		if ( !isNewInteraction() ) {
-			return;
+			return $.Deferred().resolve();
 		}
 
-		wait( FETCH_START_DELAY )
+		return wait( FETCH_START_DELAY )
 			.then( function () {
 				var previewState = getState().preview;
 
@@ -233,7 +232,7 @@ export function abandon() {
 		var token = getState().preview.activeToken;
 
 		if ( !token ) {
-			return;
+			return $.Deferred().resolve();
 		}
 
 		dispatch( timedAction( {
@@ -241,7 +240,7 @@ export function abandon() {
 			token: token
 		} ) );
 
-		wait( ABANDON_END_DELAY )
+		return wait( ABANDON_END_DELAY )
 			.then( function () {
 				dispatch( {
 					type: types.ABANDON_END,

tests/node-qunit/actions.test.js

@@ -107,7 +107,6 @@ QUnit.module( 'ext.popups/actions#linkDwell @integration', {
 		// The worst-case implementation of mw.now.
 		mw.now = function () { return Date.now(); };
 
-		setupWait( this );
 		setupEl( this );
 	}
 } );
@@ -125,7 +124,7 @@ QUnit.test( '#linkDwell', function ( assert ) {
 		activeToken: generateToken()
 	};
 
-	actions.linkDwell(
+	p = actions.linkDwell(
 		this.title, this.el, event, /* gateway = */ null, generateToken
 	)(
 		dispatch,
@@ -151,16 +150,13 @@ QUnit.test( '#linkDwell', function ( assert ) {
 
 	// ---
 
-	p = this.waitPromises[ 0 ].then( function () {
+	p.then( function () {
 		assert.strictEqual(
 			dispatch.callCount,
 			2,
 			'The fetch action is dispatched after FETCH_COMPLETE milliseconds.'
 		);
 	} );
-
-	// After FETCH_START_DELAY milliseconds...
-	this.waitDeferreds[ 0 ].resolve();
 	return p;
 } );
 
@@ -176,21 +172,19 @@ QUnit.test( '#linkDwell doesn\'t continue when previews are disabled', function
 		activeToken: generateToken()
 	};
 
-	actions.linkDwell(
+	p = actions.linkDwell(
 		this.title, this.el, event, /* gateway = */ null, generateToken
 	)(
 		dispatch,
 		this.getState
 	);
 
-	assert.strictEqual( this.wait.callCount, 1 );
+	assert.strictEqual( dispatch.callCount, 1 );
 
-	p = this.waitPromises[ 0 ].then( function () {
+	p.then( function () {
 		assert.strictEqual( dispatch.callCount, 1 );
 	} );
 
-	// After FETCH_START_DELAY milliseconds...
-	this.waitDeferreds[ 0 ].resolve();
 	return p;
 } );
 
@@ -206,7 +200,7 @@ QUnit.test( '#linkDwell doesn\'t continue if the token has changed', function (
 		activeToken: generateToken()
 	};
 
-	actions.linkDwell(
+	p = actions.linkDwell(
 		this.title, this.el, event, /* gateway = */ null, generateToken
 	)(
 		dispatch,
@@ -225,12 +219,10 @@ QUnit.test( '#linkDwell doesn\'t continue if the token has changed', function (
 		activeToken: 'banana'
 	};
 
-	p = this.waitPromises[ 0 ].then( function () {
+	p.then( function () {
 		assert.strictEqual( dispatch.callCount, 1 );
 	} );
 
-	// After FETCH_START_DELAY milliseconds...
-	this.waitDeferreds[ 0 ].resolve();
 	return p;
 } );
 
@@ -244,19 +236,15 @@ QUnit.test( '#linkDwell dispatches the fetch action', function ( assert ) {
 		activeToken: generateToken()
 	};
 
-	actions.linkDwell(
+	p = actions.linkDwell(
 		this.title, this.el, event, /* gateway = */ null, generateToken
 	)(
 		dispatch,
 		this.getState
-	);
+	).then( function () {
-
-	p = this.waitPromises[ 0 ].then( function () {
 		assert.strictEqual( dispatch.callCount, 2 );
 	} );
 
-	// After FETCH_START_DELAY milliseconds...
-	this.waitDeferreds[ 0 ].resolve();
 	return p;
 } );
 
@@ -439,7 +427,7 @@ QUnit.test( 'it should dispatch the FETCH_FAILED action when the request fails e
 
 QUnit.module( 'ext.popups/actions#abandon', {
 	beforeEach: function () {
-		setupWait( this );
+		setupEl( this );
 	}
 } );
 
@@ -456,6 +444,8 @@ QUnit.test( 'it should dispatch start and end actions', function ( assert ) {
 
 	this.sandbox.stub( mw, 'now' ).returns( new Date() );
 
+	setupWait( this );
+
 	actions.abandon( this.el )( dispatch, getState );
 
 	assert.ok( dispatch.calledWith( {
@@ -496,10 +486,11 @@ QUnit.test( 'it shouldn\'t dispatch under certain conditions', function ( assert
 			};
 		};
 
-	actions.abandon( this.el )( dispatch, getState );
+	return actions.abandon( this.el )( dispatch, getState )
-
+		.then( function () {
 			assert.ok( dispatch.notCalled );
 		} );
+} );
 
 QUnit.module( 'ext.popups/actions#saveSettings' );