State: Notices middleware refactoring per feedback

Automattic · Jul 13, 2016 · ca092b5 · ca092b5
1 parent 810a33a
commit ca092b5
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 112 deletions.
diff --git a/client/state/notices/middleware.js b/client/state/notices/middleware.js
@@ -19,45 +19,63 @@ import {
  * Utility
  */
 
-const dispatchSuccess = ( message ) => ( { dispatch } ) => dispatch( successNotice( message ) );
+export function dispatchSuccess( message ) {
+	return ( dispatch ) => dispatch( successNotice( message ) );
+}
 
-export const HANDLERS = {
-	[ POST_DELETE_FAILURE ]: ( { dispatch, getState, action } ) => {
-		const post = getSitePost( getState(), action.siteId, action.postId );
+/**
+ * Handlers
+ */
 
-		let message;
-		if ( post ) {
-			message = translate( 'An error occurred while deleting "%s"', {
-				args: [ truncate( post.title, { length: 24 } ) ]
-			} );
-		} else {
-			message = translate( 'An error occurred while deleting the post' );
-		}
+export function onPostDeleteFailure( dispatch, action, getState ) {
+	const post = getSitePost( getState(), action.siteId, action.postId );
 
-		dispatch( errorNotice( message ) );
-	},
-	[ POST_DELETE_SUCCESS ]: dispatchSuccess( translate( 'Post successfully deleted' ) ),
-	[ POST_SAVE_SUCCESS ]: ( { dispatch, action } ) => {
-		let text;
-		switch ( action.post.status ) {
-			case 'trash':
-				text = translate( 'Post successfully moved to trash' );
-				break;
+	let message;
+	if ( post ) {
+		message = translate( 'An error occurred while deleting "%s"', {
+			args: [ truncate( post.title, { length: 24 } ) ]
+		} );
+	} else {
+		message = translate( 'An error occurred while deleting the post' );
+	}
 
-			case 'publish':
-				text = translate( 'Post successfully published' );
-				break;
-		}
+	dispatch( errorNotice( message ) );
+}
 
-		if ( text ) {
-			dispatch( successNotice( text ) );
-		}
+export function onPostSaveSuccess( dispatch, action ) {
+	let text;
+	switch ( action.post.status ) {
+		case 'trash':
+			text = translate( 'Post successfully moved to trash' );
+			break;
+
+		case 'publish':
+			text = translate( 'Post successfully published' );
+			break;
+	}
+
+	if ( text ) {
+		dispatch( successNotice( text ) );
 	}
+}
+
+/**
+ * Handler action type mapping
+ */
+
+export const handlers = {
+	[ POST_DELETE_FAILURE ]: onPostDeleteFailure,
+	[ POST_DELETE_SUCCESS ]: dispatchSuccess( translate( 'Post successfully deleted' ) ),
+	[ POST_SAVE_SUCCESS ]: onPostSaveSuccess
 };
 
+/**
+ * Middleware
+ */
+
 export default ( { dispatch, getState } ) => ( next ) => ( action ) => {
-	if ( HANDLERS.hasOwnProperty( action.type ) ) {
-		HANDLERS[ action.type ]( { dispatch, getState, action } );
+	if ( handlers.hasOwnProperty( action.type ) ) {
+		handlers[ action.type ]( dispatch, action, getState );
 	}
 
 	return next( action );

diff --git a/client/state/notices/test/middleware.js b/client/state/notices/test/middleware.js
@@ -8,13 +8,17 @@ import { noop } from 'lodash';
 /**
  * Internal dependencies
  */
-import noticesMiddleware, { HANDLERS } from '../middleware';
+import noticesMiddleware, {
+	handlers,
+	dispatchSuccess,
+	onPostDeleteFailure,
+	onPostSaveSuccess
+} from '../middleware';
 import { useSandbox } from 'test/helpers/use-sinon';
 import { successNotice } from 'state/notices/actions';
 import {
 	NOTICE_CREATE,
 	POST_DELETE_FAILURE,
-	POST_DELETE_SUCCESS,
 	POST_SAVE_SUCCESS
 } from 'state/action-types';
 
@@ -27,13 +31,13 @@ describe( 'middleware', () => {
 		} );
 
 		before( () => {
-			HANDLERS.DUMMY_TYPE = ( { action, dispatch, getState } ) => {
+			handlers.DUMMY_TYPE = ( dispatch, action, getState ) => {
 				dispatch( successNotice( `${ getState() } ${ action.target }` ) );
 			};
 		} );
 
 		after( () => {
-			delete HANDLERS.DUMMY_TYPE;
+			delete handlers.DUMMY_TYPE;
 		} );
 
 		it( 'should trigger the observer corresponding to the dispatched action type', () => {
@@ -48,116 +52,104 @@ describe( 'middleware', () => {
 		} );
 	} );
 
-	describe( 'HANDLERS', () => {
-		let spy;
+	context( 'utility', () => {
+		let dispatch;
 		useSandbox( ( sandbox ) => {
-			spy = sandbox.spy();
+			dispatch = sandbox.spy();
 		} );
 
-		context( '.POST_DELETE_FAILURE', () => {
-			it( 'should dispatch error notice with truncated title if known', () => {
-				HANDLERS[ POST_DELETE_FAILURE ]( {
-					dispatch: spy,
-					action: {
-						type: POST_DELETE_FAILURE,
-						siteId: 2916284,
-						postId: 841
-					},
-					getState: () => ( {
-						posts: {
-							items: {
-								'3d097cb7c5473c169bba0eb8e3c6cb64': {
-									ID: 841,
-									site_ID: 2916284,
-									global_ID: '3d097cb7c5473c169bba0eb8e3c6cb64',
-									title: 'Hello World, This Should Be Truncated'
-								}
-							}
-						}
-					} )
-				} );
+		describe( 'dispatchSuccess()', () => {
+			it( 'should return a function which upon being called dispatches the specified success message', () => {
+				dispatchSuccess( 'Success!' )( dispatch );
 
-				expect( spy ).to.have.been.calledWithMatch( {
+				expect( dispatch ).to.have.been.calledWithMatch( {
 					type: NOTICE_CREATE,
 					notice: {
-						status: 'is-error',
-						text: 'An error occurred while deleting "Hello World, This Sho..."'
+						status: 'is-success',
+						text: 'Success!'
 					}
 				} );
 			} );
+		} );
+	} );
 
-			it( 'should dispatch error notice with unknown title', () => {
-				HANDLERS[ POST_DELETE_FAILURE ]( {
-					dispatch: spy,
-					action: {
-						type: POST_DELETE_FAILURE,
-						siteId: 2916284,
-						postId: 841
-					},
-					getState: () => ( {
-						posts: {
-							items: {}
+	context( 'handlers', () => {
+		let dispatch;
+		useSandbox( ( sandbox ) => {
+			dispatch = sandbox.spy();
+		} );
+
+		describe( 'onPostDeleteFailure()', () => {
+			it( 'should dispatch error notice with truncated title if known', () => {
+				onPostDeleteFailure( dispatch, {
+					type: POST_DELETE_FAILURE,
+					siteId: 2916284,
+					postId: 841
+				}, () => ( {
+					posts: {
+						items: {
+							'3d097cb7c5473c169bba0eb8e3c6cb64': {
+								ID: 841,
+								site_ID: 2916284,
+								global_ID: '3d097cb7c5473c169bba0eb8e3c6cb64',
+								title: 'Hello World, This Should Be Truncated'
+							}
 						}
-					} )
-				} );
+					}
+				} ) );
 
-				expect( spy ).to.have.been.calledWithMatch( {
+				expect( dispatch ).to.have.been.calledWithMatch( {
 					type: NOTICE_CREATE,
 					notice: {
 						status: 'is-error',
-						text: 'An error occurred while deleting the post'
+						text: 'An error occurred while deleting "Hello World, This Sho..."'
 					}
 				} );
 			} );
-		} );
 
-		context( '.POST_DELETE_SUCCESS', () => {
-			it( 'should dispatch success notice', () => {
-				HANDLERS[ POST_DELETE_SUCCESS ]( {
-					dispatch: spy,
-					action: {
-						type: POST_DELETE_SUCCESS
+			it( 'should dispatch error notice with unknown title', () => {
+				onPostDeleteFailure( dispatch, {
+					type: POST_DELETE_FAILURE,
+					siteId: 2916284,
+					postId: 841
+				}, () => ( {
+					posts: {
+						items: {}
 					}
-				} );
+				} ) );
 
-				expect( spy ).to.have.been.calledWithMatch( {
+				expect( dispatch ).to.have.been.calledWithMatch( {
 					type: NOTICE_CREATE,
 					notice: {
-						status: 'is-success',
-						text: 'Post successfully deleted'
+						status: 'is-error',
+						text: 'An error occurred while deleting the post'
 					}
 				} );
 			} );
 		} );
 
-		context( '.POST_SAVE_SUCCESS', () => {
+		describe( 'onPostSaveSuccess()', () => {
 			it( 'should not dispatch if status has no corresponding text', () => {
-				HANDLERS[ POST_SAVE_SUCCESS ]( {
-					dispatch: spy,
-					action: {
-						type: POST_SAVE_SUCCESS,
-						post: {
-							title: 'Hello World',
-							status: 'invalid'
-						}
+				onPostSaveSuccess( dispatch, {
+					type: POST_SAVE_SUCCESS,
+					post: {
+						title: 'Hello World',
+						status: 'invalid'
 					}
 				} );
 
-				expect( spy ).to.not.have.been.calledWithMatch( {
+				expect( dispatch ).to.not.have.been.calledWithMatch( {
 					type: NOTICE_CREATE
 				} );
 			} );
 
 			it( 'should dispatch success notice for trash', () => {
-				HANDLERS[ POST_SAVE_SUCCESS ]( {
-					dispatch: spy,
-					action: {
-						type: POST_SAVE_SUCCESS,
-						post: { status: 'trash' }
-					}
+				onPostSaveSuccess( dispatch, {
+					type: POST_SAVE_SUCCESS,
+					post: { status: 'trash' }
 				} );
 
-				expect( spy ).to.have.been.calledWithMatch( {
+				expect( dispatch ).to.have.been.calledWithMatch( {
 					type: NOTICE_CREATE,
 					notice: {
 						status: 'is-success',
@@ -167,15 +159,12 @@ describe( 'middleware', () => {
 			} );
 
 			it( 'should dispatch success notice for publish', () => {
-				HANDLERS[ POST_SAVE_SUCCESS ]( {
-					dispatch: spy,
-					action: {
-						type: POST_SAVE_SUCCESS,
-						post: { status: 'publish' }
-					}
+				onPostSaveSuccess( dispatch, {
+					type: POST_SAVE_SUCCESS,
+					post: { status: 'publish' }
 				} );
 
-				expect( spy ).to.have.been.calledWithMatch( {
+				expect( dispatch ).to.have.been.calledWithMatch( {
 					type: NOTICE_CREATE,
 					notice: {
 						status: 'is-success',