From b641420ef52e8dbc7c3baae852507fa5ebe13b4a Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 20 Dec 2016 13:49:37 +0100 Subject: [PATCH 1/5] Editor: Reset Post Edits when switching posts in the post editor If we create a new post, switch to a draft from the sidebar without saving and coming back to /post/new again, some initial post edits were kept instead of having a fresh new post --- client/post-editor/post-editor.jsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/client/post-editor/post-editor.jsx b/client/post-editor/post-editor.jsx index 079ad8d833ff8..99a6ed0c10bd1 100644 --- a/client/post-editor/post-editor.jsx +++ b/client/post-editor/post-editor.jsx @@ -154,6 +154,12 @@ export const PostEditor = React.createClass( { componentWillReceiveProps: function( nextProps ) { const { siteId, postId } = this.props; + + // When switching posts, reset post edits for the post we're leaving + if ( nextProps.postId !== postId ) { + this.props.resetPostEdits( siteId, postId ); + } + if ( nextProps.siteId === siteId && nextProps.postId !== postId ) { // make sure the history entry has the post ID in it, but don't dispatch page.replace( nextProps.editPath, null, false, false ); From d029c2565059c1b80ad1c7ad6295d5ef9137763f Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 20 Dec 2016 15:38:48 +0100 Subject: [PATCH 2/5] Changes per review --- client/post-editor/controller.js | 3 ++- client/post-editor/post-editor.jsx | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/client/post-editor/controller.js b/client/post-editor/controller.js index bfaab5b5c14da..c0e4cad464ee5 100644 --- a/client/post-editor/controller.js +++ b/client/post-editor/controller.js @@ -24,7 +24,7 @@ import PostEditor from './post-editor'; import { setEditorPostId } from 'state/ui/editor/actions'; import { getSelectedSiteId } from 'state/ui/selectors'; import { getEditorPostId, getEditorPath } from 'state/ui/editor/selectors'; -import { editPost } from 'state/posts/actions'; +import { editPost, resetPostEdits } from 'state/posts/actions'; function getPostID( context ) { if ( ! context.params.post || 'new' === context.params.post ) { @@ -115,6 +115,7 @@ module.exports = { function startEditing( siteId ) { context.store.dispatch( setEditorPostId( postID ) ); + context.store.dispatch( resetPostEdits( siteId, postID ) ); context.store.dispatch( editPost( siteId, postID, { type: postType } ) ); if ( maybeRedirect( context ) ) { diff --git a/client/post-editor/post-editor.jsx b/client/post-editor/post-editor.jsx index 99a6ed0c10bd1..d0f7d53187c77 100644 --- a/client/post-editor/post-editor.jsx +++ b/client/post-editor/post-editor.jsx @@ -156,7 +156,7 @@ export const PostEditor = React.createClass( { const { siteId, postId } = this.props; // When switching posts, reset post edits for the post we're leaving - if ( nextProps.postId !== postId ) { + if ( nextProps.siteId !== siteId || nextProps.postId !== postId ) { this.props.resetPostEdits( siteId, postId ); } From 31c9c580833ef7013180d09682aedd598b91126f Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 20 Dec 2016 16:55:26 +0100 Subject: [PATCH 3/5] Adding editor start action --- client/post-editor/controller.js | 7 +++-- client/post-editor/post-editor.jsx | 6 ----- client/state/action-types.js | 1 + client/state/posts/reducer.js | 2 ++ client/state/posts/test/reducer.js | 37 ++++++++++++++++++++++++++ client/state/ui/editor/actions.js | 22 ++++++++++++--- client/state/ui/editor/reducer.js | 3 ++- client/state/ui/editor/test/actions.js | 18 +++++++++++-- client/state/ui/editor/test/reducer.js | 12 ++++++++- 9 files changed, 91 insertions(+), 17 deletions(-) diff --git a/client/post-editor/controller.js b/client/post-editor/controller.js index c0e4cad464ee5..a1aa18b5435b2 100644 --- a/client/post-editor/controller.js +++ b/client/post-editor/controller.js @@ -21,10 +21,10 @@ var actions = require( 'lib/posts/actions' ), userUtils = require( 'lib/user/utils' ), analytics = require( 'lib/analytics' ); import PostEditor from './post-editor'; -import { setEditorPostId } from 'state/ui/editor/actions'; +import { startPostEditor } from 'state/ui/editor/actions'; import { getSelectedSiteId } from 'state/ui/selectors'; import { getEditorPostId, getEditorPath } from 'state/ui/editor/selectors'; -import { editPost, resetPostEdits } from 'state/posts/actions'; +import { editPost } from 'state/posts/actions'; function getPostID( context ) { if ( ! context.params.post || 'new' === context.params.post ) { @@ -114,8 +114,7 @@ module.exports = { const postID = getPostID( context ); function startEditing( siteId ) { - context.store.dispatch( setEditorPostId( postID ) ); - context.store.dispatch( resetPostEdits( siteId, postID ) ); + context.store.dispatch( startPostEditor( siteId, postID ) ); context.store.dispatch( editPost( siteId, postID, { type: postType } ) ); if ( maybeRedirect( context ) ) { diff --git a/client/post-editor/post-editor.jsx b/client/post-editor/post-editor.jsx index d0f7d53187c77..079ad8d833ff8 100644 --- a/client/post-editor/post-editor.jsx +++ b/client/post-editor/post-editor.jsx @@ -154,12 +154,6 @@ export const PostEditor = React.createClass( { componentWillReceiveProps: function( nextProps ) { const { siteId, postId } = this.props; - - // When switching posts, reset post edits for the post we're leaving - if ( nextProps.siteId !== siteId || nextProps.postId !== postId ) { - this.props.resetPostEdits( siteId, postId ); - } - if ( nextProps.siteId === siteId && nextProps.postId !== postId ) { // make sure the history entry has the post ID in it, but don't dispatch page.replace( nextProps.editPath, null, false, false ); diff --git a/client/state/action-types.js b/client/state/action-types.js index 4a6183be16be4..d4230338a3ac9 100644 --- a/client/state/action-types.js +++ b/client/state/action-types.js @@ -70,6 +70,7 @@ export const EDITOR_LAST_DRAFT_SET = 'EDITOR_LAST_DRAFT_SET'; export const EDITOR_MEDIA_EDIT_ITEM_SET = 'EDITOR_MEDIA_EDIT_ITEM_SET'; export const EDITOR_POST_ID_SET = 'EDITOR_POST_ID_SET'; export const EDITOR_SHOW_DRAFTS_TOGGLE = 'EDITOR_SHOW_DRAFTS_TOGGLE'; +export const EDITOR_START = 'EDITOR_START'; export const EXPORT_ADVANCED_SETTINGS_FETCH = 'EXPORT_ADVANCED_SETTINGS_FETCH'; export const EXPORT_ADVANCED_SETTINGS_FETCH_FAIL = 'EXPORT_ADVANCED_SETTINGS_FETCH_FAIL'; export const EXPORT_ADVANCED_SETTINGS_RECEIVE = 'EXPORT_ADVANCED_SETTINGS_RECEIVE'; diff --git a/client/state/posts/reducer.js b/client/state/posts/reducer.js index 39abe5a43b1a1..d6a583b95b2a2 100644 --- a/client/state/posts/reducer.js +++ b/client/state/posts/reducer.js @@ -9,6 +9,7 @@ import { get, set, omit, omitBy, isEqual, reduce, merge, findKey, mapValues } fr */ import PostQueryManager from 'lib/query-manager/post'; import { + EDITOR_START, POST_DELETE, POST_DELETE_SUCCESS, POST_DELETE_FAILURE, @@ -262,6 +263,7 @@ export function edits( state = {}, action ) { } ); case POST_EDITS_RESET: + case EDITOR_START: if ( ! state.hasOwnProperty( action.siteId ) ) { break; } diff --git a/client/state/posts/test/reducer.js b/client/state/posts/test/reducer.js index 6c3b6cb799601..25392205b45e4 100644 --- a/client/state/posts/test/reducer.js +++ b/client/state/posts/test/reducer.js @@ -9,6 +9,7 @@ import deepFreeze from 'deep-freeze'; */ import { useSandbox } from 'test/helpers/use-sinon'; import { + EDITOR_START, POST_DELETE, POST_DELETE_SUCCESS, POST_DELETE_FAILURE, @@ -925,6 +926,42 @@ describe( 'reducer', () => { } ); } ); + it( 'should ignore start editor action when site doesn\'t exist', () => { + const original = deepFreeze( {} ); + const state = edits( original, { + type: EDITOR_START, + siteId: 2916284, + postId: 841 + } ); + + expect( state ).to.equal( original ); + } ); + + it( 'should discard edits when start editor action is dispatched', () => { + const state = edits( deepFreeze( { + 2916284: { + 841: { + title: 'Hello World' + }, + '': { + title: 'Ribs & Chicken' + } + } + } ), { + type: EDITOR_START, + siteId: 2916284, + postId: 841 + } ); + + expect( state ).to.eql( { + 2916284: { + '': { + title: 'Ribs & Chicken' + } + } + } ); + } ); + it( 'should not persist state', () => { const state = edits( deepFreeze( { 2916284: { diff --git a/client/state/ui/editor/actions.js b/client/state/ui/editor/actions.js index 33c146e8f0576..ac28d30c05884 100644 --- a/client/state/ui/editor/actions.js +++ b/client/state/ui/editor/actions.js @@ -1,7 +1,7 @@ /** * Internal dependencies */ -import { EDITOR_POST_ID_SET, EDITOR_SHOW_DRAFTS_TOGGLE } from 'state/action-types'; +import { EDITOR_POST_ID_SET, EDITOR_SHOW_DRAFTS_TOGGLE, EDITOR_START } from 'state/action-types'; import { ModalViews } from 'state/ui/media-modal/constants'; import { setMediaModalView } from 'state/ui/media-modal/actions'; import { withAnalytics, bumpStat } from 'state/analytics/actions'; @@ -17,8 +17,8 @@ export const MODAL_VIEW_STATS = { }; /** - * Returns an action object to be used in signalling that the editor should - * begin to edit the post with the specified post ID, or `null` as a new post. + * Returns an action object to be used in signalling that the edited post ID has changed + * but we're still editing the same post * * @param {?Number} postId Post ID * @return {Object} Action object @@ -30,6 +30,22 @@ export function setEditorPostId( postId ) { }; } +/** + * Returns an action object to be used in signalling that the editor should + * begin to edit the post with the specified post ID, or `null` as a new post. + * + * @param {Number} siteId Site ID + * @param {?Number} postId Post ID + * @return {Object} Action object + */ +export function startPostEditor( siteId, postId ) { + return { + type: EDITOR_START, + siteId, + postId + }; +} + /** * Returns an action object to be used in signalling that the editor draft * drawer visibility state should be toggled. diff --git a/client/state/ui/editor/reducer.js b/client/state/ui/editor/reducer.js index fd9e54a706838..2b83e8009cd19 100644 --- a/client/state/ui/editor/reducer.js +++ b/client/state/ui/editor/reducer.js @@ -6,7 +6,7 @@ import { combineReducers } from 'redux'; /** * Internal dependencies */ -import { EDITOR_POST_ID_SET, EDITOR_SHOW_DRAFTS_TOGGLE } from 'state/action-types'; +import { EDITOR_POST_ID_SET, EDITOR_SHOW_DRAFTS_TOGGLE, EDITOR_START } from 'state/action-types'; import imageEditor from './image-editor/reducer'; import lastDraft from './last-draft/reducer'; import contactForm from './contact-form/reducer'; @@ -22,6 +22,7 @@ import contactForm from './contact-form/reducer'; export function postId( state = null, action ) { switch ( action.type ) { case EDITOR_POST_ID_SET: + case EDITOR_START: return action.postId; } diff --git a/client/state/ui/editor/test/actions.js b/client/state/ui/editor/test/actions.js index 366622351cbec..b8ac28dcdc46c 100644 --- a/client/state/ui/editor/test/actions.js +++ b/client/state/ui/editor/test/actions.js @@ -10,13 +10,15 @@ import { forEach } from 'lodash'; import { ANALYTICS_STAT_BUMP, EDITOR_POST_ID_SET, - EDITOR_SHOW_DRAFTS_TOGGLE + EDITOR_SHOW_DRAFTS_TOGGLE, + EDITOR_START } from 'state/action-types'; import { MODAL_VIEW_STAT_MAPPING, setEditorPostId, toggleEditorDraftsVisible, - setEditorMediaModalView + setEditorMediaModalView, + startPostEditor } from '../actions'; import { setMediaModalView } from 'state/ui/media-modal/actions'; @@ -32,6 +34,18 @@ describe( 'actions', () => { } ); } ); + describe( 'startPostEditor()', () => { + it( 'should return an action object', () => { + const action = startPostEditor( 10, 183 ); + + expect( action ).to.eql( { + type: EDITOR_START, + siteId: 10, + postId: 183 + } ); + } ); + } ); + describe( '#toggleEditorDraftsVisible()', () => { it( 'should return an action object', () => { const action = toggleEditorDraftsVisible(); diff --git a/client/state/ui/editor/test/reducer.js b/client/state/ui/editor/test/reducer.js index 386315a731fe7..7457ffd272ce6 100644 --- a/client/state/ui/editor/test/reducer.js +++ b/client/state/ui/editor/test/reducer.js @@ -6,7 +6,7 @@ import { expect } from 'chai'; /** * Internal dependencies */ -import { EDITOR_POST_ID_SET, EDITOR_SHOW_DRAFTS_TOGGLE } from 'state/action-types'; +import { EDITOR_POST_ID_SET, EDITOR_SHOW_DRAFTS_TOGGLE, EDITOR_START } from 'state/action-types'; import reducer, { postId, showDrafts } from '../reducer'; describe( 'reducer', () => { @@ -35,6 +35,16 @@ describe( 'reducer', () => { expect( state ).to.equal( 184 ); } ); + + it( 'should update the tracked id when starting the editor', () => { + const state = postId( undefined, { + type: EDITOR_START, + siteId: 1, + postId: 184 + } ); + + expect( state ).to.equal( 184 ); + } ) } ); describe( '#showDrafts()', () => { From de3074e68c6869ff9bc7a7beb1b915ad38ce11f1 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 22 Dec 2016 10:59:32 +0100 Subject: [PATCH 4/5] Trying a new approach --- client/post-editor/controller.js | 15 ++++++--- client/post-editor/index.js | 3 ++ client/post-editor/post-editor.jsx | 21 +++--------- client/state/action-types.js | 3 +- client/state/posts/actions.js | 28 +++++++--------- client/state/posts/reducer.js | 13 ++++++-- client/state/posts/test/actions.js | 31 +++++++++-------- client/state/posts/test/reducer.js | 46 +++++++++++++++++++++----- client/state/ui/editor/actions.js | 30 +++++++++-------- client/state/ui/editor/reducer.js | 5 +-- client/state/ui/editor/test/actions.js | 24 ++++++++------ client/state/ui/editor/test/reducer.js | 35 +++++++++++++++----- 12 files changed, 158 insertions(+), 96 deletions(-) diff --git a/client/post-editor/controller.js b/client/post-editor/controller.js index a1aa18b5435b2..48a356f0426db 100644 --- a/client/post-editor/controller.js +++ b/client/post-editor/controller.js @@ -21,10 +21,9 @@ var actions = require( 'lib/posts/actions' ), userUtils = require( 'lib/user/utils' ), analytics = require( 'lib/analytics' ); import PostEditor from './post-editor'; -import { startPostEditor } from 'state/ui/editor/actions'; +import { startEditingPost, stopEditingPost } from 'state/ui/editor/actions'; import { getSelectedSiteId } from 'state/ui/selectors'; import { getEditorPostId, getEditorPath } from 'state/ui/editor/selectors'; -import { editPost } from 'state/posts/actions'; function getPostID( context ) { if ( ! context.params.post || 'new' === context.params.post ) { @@ -114,8 +113,7 @@ module.exports = { const postID = getPostID( context ); function startEditing( siteId ) { - context.store.dispatch( startPostEditor( siteId, postID ) ); - context.store.dispatch( editPost( siteId, postID, { type: postType } ) ); + context.store.dispatch( startEditingPost( siteId, postID, postType ) ); if ( maybeRedirect( context ) ) { return; @@ -177,6 +175,15 @@ module.exports = { renderEditor( context, postType ); }, + exitPost: function( context, next ) { + const postId = getPostID( context ); + const siteId = getSelectedSiteId( context.store.getState() ); + if ( siteId ) { + context.store.dispatch( stopEditingPost( siteId, postId ) ); + } + next(); + }, + pressThis: function( context, next ) { context.getSiteSelectionHeaderText = function() { return i18n.translate( 'Select a site to start writing' ); diff --git a/client/post-editor/index.js b/client/post-editor/index.js index 1f422ef97b54e..5fc4076069639 100644 --- a/client/post-editor/index.js +++ b/client/post-editor/index.js @@ -14,14 +14,17 @@ module.exports = function() { page( '/post', controller.pressThis, sitesController.siteSelection, sitesController.sites ); page( '/post/new', () => page.redirect( '/post' ) ); // redirect from beep-beep-boop page( '/post/:site?/:post?', sitesController.siteSelection, sitesController.fetchJetpackSettings, controller.post ); + page.exit( '/post/:site?/:post?', controller.exitPost ); page( '/page', sitesController.siteSelection, sitesController.sites ); page( '/page/new', () => page.redirect( '/page' ) ); // redirect from beep-beep-boop page( '/page/:site?/:post?', sitesController.siteSelection, sitesController.fetchJetpackSettings, controller.post ); + page.exit( '/page/:site?/:post?', controller.exitPost ); if ( config.isEnabled( 'manage/custom-post-types' ) ) { page( '/edit/:type', sitesController.siteSelection, sitesController.sites ); page( '/edit/:type/new', ( context ) => page.redirect( `/edit/${ context.params.type }` ) ); page( '/edit/:type/:site?/:post?', sitesController.siteSelection, sitesController.fetchJetpackSettings, controller.post ); + page.exit( '/edit/:type/:site?/:post?', controller.exitPost ); } }; diff --git a/client/post-editor/post-editor.jsx b/client/post-editor/post-editor.jsx index 079ad8d833ff8..4f6e91422b526 100644 --- a/client/post-editor/post-editor.jsx +++ b/client/post-editor/post-editor.jsx @@ -36,8 +36,8 @@ const actions = require( 'lib/posts/actions' ), import { getSelectedSiteId } from 'state/ui/selectors'; import { setEditorLastDraft, resetEditorLastDraft } from 'state/ui/editor/last-draft/actions'; import { isEditorDraftsVisible, getEditorPostId, getEditorPath } from 'state/ui/editor/selectors'; -import { toggleEditorDraftsVisible, setEditorPostId } from 'state/ui/editor/actions'; -import { receivePost, resetPostEdits } from 'state/posts/actions'; +import { toggleEditorDraftsVisible } from 'state/ui/editor/actions'; +import { receivePost, savePostSuccess } from 'state/posts/actions'; import { getPostEdits, isEditedPostDirty } from 'state/posts/selectors'; import EditorDocumentHead from 'post-editor/editor-document-head'; import EditorPostTypeUnsupported from 'post-editor/editor-post-type-unsupported'; @@ -138,9 +138,6 @@ export const PostEditor = React.createClass( { componentWillUnmount: function() { PostEditStore.removeListener( 'change', this.onEditedPostChange ); - // Reset post edits after leaving editor - this.props.resetPostEdits( this.props.siteId, this.props.postId ); - // TODO: REDUX - remove flux actions when whole post-editor is reduxified actions.stopEditing(); @@ -690,19 +687,12 @@ export const PostEditor = React.createClass( { this.props.resetEditorLastDraft(); } - // Assign editor post ID to saved value (especially important when - // transitioning from an unsaved post to a saved one) - if ( post.ID !== this.props.postId ) { - this.props.setEditorPostId( post.ID ); - } + // Remove this when the editor is completely reduxified ( When using Redux actions for all post saving requests ) + this.props.savePostSuccess( post.site_ID, this.props.postId, post, {} ); // Receive updated post into state this.props.receivePost( post ); - // Reset previous edits, preserving type - this.props.resetPostEdits( this.props.siteId ); - this.props.resetPostEdits( post.site_ID, post.ID ); - const nextState = { isSaving: false, isPublishing: false @@ -785,8 +775,7 @@ export default connect( setEditorLastDraft, resetEditorLastDraft, receivePost, - resetPostEdits, - setEditorPostId, + savePostSuccess, setEditorModePreference: savePreference.bind( null, 'editor-mode' ), setLayoutFocus, }, dispatch ); diff --git a/client/state/action-types.js b/client/state/action-types.js index d4230338a3ac9..deabb35a2c23e 100644 --- a/client/state/action-types.js +++ b/client/state/action-types.js @@ -68,9 +68,9 @@ export const EDITOR_CONTACT_FORM_LOAD = 'EDITOR_CONTACT_FORM_LOAD'; export const EDITOR_CONTACT_FORM_SETTINGS_UPDATE = 'EDITOR_CONTACT_FORM_SETTINGS_UPDATE'; export const EDITOR_LAST_DRAFT_SET = 'EDITOR_LAST_DRAFT_SET'; export const EDITOR_MEDIA_EDIT_ITEM_SET = 'EDITOR_MEDIA_EDIT_ITEM_SET'; -export const EDITOR_POST_ID_SET = 'EDITOR_POST_ID_SET'; export const EDITOR_SHOW_DRAFTS_TOGGLE = 'EDITOR_SHOW_DRAFTS_TOGGLE'; export const EDITOR_START = 'EDITOR_START'; +export const EDITOR_STOP = 'EDITOR_STOP'; export const EXPORT_ADVANCED_SETTINGS_FETCH = 'EXPORT_ADVANCED_SETTINGS_FETCH'; export const EXPORT_ADVANCED_SETTINGS_FETCH_FAIL = 'EXPORT_ADVANCED_SETTINGS_FETCH_FAIL'; export const EXPORT_ADVANCED_SETTINGS_RECEIVE = 'EXPORT_ADVANCED_SETTINGS_RECEIVE'; @@ -293,7 +293,6 @@ export const POST_DELETE = 'POST_DELETE'; export const POST_DELETE_FAILURE = 'POST_DELETE_FAILURE'; export const POST_DELETE_SUCCESS = 'POST_DELETE_SUCCESS'; export const POST_EDIT = 'POST_EDIT'; -export const POST_EDITS_RESET = 'POST_EDITS_RESET'; export const POST_FORMATS_RECEIVE = 'POST_FORMATS_RECEIVE'; export const POST_FORMATS_REQUEST = 'POST_FORMATS_REQUEST'; export const POST_FORMATS_REQUEST_FAILURE = 'POST_FORMATS_REQUEST_FAILURE'; diff --git a/client/state/posts/actions.js b/client/state/posts/actions.js index 338cc31773aa3..f6fe84b46ef86 100644 --- a/client/state/posts/actions.js +++ b/client/state/posts/actions.js @@ -14,7 +14,6 @@ import { POST_DELETE_SUCCESS, POST_DELETE_FAILURE, POST_EDIT, - POST_EDITS_RESET, POST_REQUEST, POST_REQUEST_SUCCESS, POST_REQUEST_FAILURE, @@ -164,18 +163,21 @@ export function editPost( siteId, postId = null, post ) { } /** - * Returns an action object to be used in signalling that any post edits for - * the specified post should be discarded. + * Returns an action object to be used in signalling that a post has been saved * - * @param {Number} siteId Site ID - * @param {Number} postId Post ID - * @return {Object} Action object + * @param {Number} siteId Site ID + * @param {Number} postId Post ID + * @param {Object} savedPost Updated post + * @param {Object} post Post attributes + * @return {Object} Action thunk */ -export function resetPostEdits( siteId, postId ) { +export function savePostSuccess( siteId, postId = null, savedPost, post ) { return { - type: POST_EDITS_RESET, + type: POST_SAVE_SUCCESS, siteId, - postId + postId, + savedPost, + post }; } @@ -201,13 +203,7 @@ export function savePost( siteId, postId = null, post ) { const normalizedPost = normalizePostForApi( post ); postHandle = postHandle[ postId ? 'update' : 'add' ].bind( postHandle ); return postHandle( { apiVersion: '1.2' }, normalizedPost ).then( ( savedPost ) => { - dispatch( { - type: POST_SAVE_SUCCESS, - siteId, - postId, - savedPost, - post - } ); + dispatch( savePostSuccess( siteId, postId, savedPost, post ) ); dispatch( receivePost( savedPost ) ); } ).catch( ( error ) => { dispatch( { diff --git a/client/state/posts/reducer.js b/client/state/posts/reducer.js index d6a583b95b2a2..6731b762b092f 100644 --- a/client/state/posts/reducer.js +++ b/client/state/posts/reducer.js @@ -10,17 +10,18 @@ import { get, set, omit, omitBy, isEqual, reduce, merge, findKey, mapValues } fr import PostQueryManager from 'lib/query-manager/post'; import { EDITOR_START, + EDITOR_STOP, POST_DELETE, POST_DELETE_SUCCESS, POST_DELETE_FAILURE, POST_EDIT, - POST_EDITS_RESET, POST_REQUEST, POST_REQUEST_SUCCESS, POST_REQUEST_FAILURE, POST_RESTORE, POST_RESTORE_FAILURE, POST_SAVE, + POST_SAVE_SUCCESS, POSTS_RECEIVE, POSTS_REQUEST, POSTS_REQUEST_SUCCESS, @@ -262,8 +263,16 @@ export function edits( state = {}, action ) { } } ); - case POST_EDITS_RESET: case EDITOR_START: + return Object.assign( {}, state, { + [ action.siteId ]: { + ...state[ action.siteId ], + [ action.postId || '' ]: { type: action.postType } + } + } ); + + case EDITOR_STOP: + case POST_SAVE_SUCCESS: if ( ! state.hasOwnProperty( action.siteId ) ) { break; } diff --git a/client/state/posts/test/actions.js b/client/state/posts/test/actions.js index f875466c95ba6..85fab8f843a27 100644 --- a/client/state/posts/test/actions.js +++ b/client/state/posts/test/actions.js @@ -13,7 +13,6 @@ import { POST_DELETE_SUCCESS, POST_DELETE_FAILURE, POST_EDIT, - POST_EDITS_RESET, POST_REQUEST, POST_REQUEST_SUCCESS, POST_REQUEST_FAILURE, @@ -35,8 +34,8 @@ import { requestSitePost, requestPosts, editPost, - resetPostEdits, savePost, + savePostSuccess, trashPost, deletePost, restorePost, @@ -75,6 +74,22 @@ describe( 'actions', () => { } ); } ); + describe( 'savePostSuccess()', () => { + it( 'should return an action object', () => { + const savedPost = { ID: 841, title: 'Hello World' }; + const attributes = { status: 'draft' }; + const action = savePostSuccess( 10, 841, savedPost, attributes ); + + expect( action ).to.eql( { + type: POST_SAVE_SUCCESS, + siteId: 10, + postId: 841, + savedPost: savedPost, + post: attributes, + } ); + } ); + } ); + describe( '#requestSitePosts()', () => { useNock( ( nock ) => { nock( 'https://public-api.wordpress.com:443' ) @@ -274,18 +289,6 @@ describe( 'actions', () => { } ); } ); - describe( '#resetPostEdits()', () => { - it( 'should return an action object', () => { - const action = resetPostEdits( 2916284 ); - - expect( action ).to.eql( { - type: POST_EDITS_RESET, - siteId: 2916284, - postId: undefined - } ); - } ); - } ); - describe( 'savePost()', () => { useNock( ( nock ) => { nock( 'https://public-api.wordpress.com:443' ) diff --git a/client/state/posts/test/reducer.js b/client/state/posts/test/reducer.js index 25392205b45e4..2e9e62b7b5621 100644 --- a/client/state/posts/test/reducer.js +++ b/client/state/posts/test/reducer.js @@ -10,17 +10,18 @@ import deepFreeze from 'deep-freeze'; import { useSandbox } from 'test/helpers/use-sinon'; import { EDITOR_START, + EDITOR_STOP, POST_DELETE, POST_DELETE_SUCCESS, POST_DELETE_FAILURE, POST_EDIT, - POST_EDITS_RESET, POST_REQUEST, POST_REQUEST_SUCCESS, POST_REQUEST_FAILURE, POST_RESTORE, POST_RESTORE_FAILURE, POST_SAVE, + POST_SAVE_SUCCESS, POSTS_RECEIVE, POSTS_REQUEST, POSTS_REQUEST_FAILURE, @@ -893,7 +894,7 @@ describe( 'reducer', () => { it( 'should ignore reset edits action when discarded site doesn\'t exist', () => { const original = deepFreeze( {} ); const state = edits( original, { - type: POST_EDITS_RESET, + type: POST_SAVE_SUCCESS, siteId: 2916284, postId: 841 } ); @@ -901,7 +902,7 @@ describe( 'reducer', () => { expect( state ).to.equal( original ); } ); - it( 'should discard edits when reset edits action dispatched', () => { + it( 'should discard edits when the post is saved', () => { const state = edits( deepFreeze( { 2916284: { 841: { @@ -912,7 +913,7 @@ describe( 'reducer', () => { } } } ), { - type: POST_EDITS_RESET, + type: POST_SAVE_SUCCESS, siteId: 2916284, postId: 841 } ); @@ -926,10 +927,10 @@ describe( 'reducer', () => { } ); } ); - it( 'should ignore start editor action when site doesn\'t exist', () => { + it( 'should ignore stop editor action when site doesn\'t exist', () => { const original = deepFreeze( {} ); const state = edits( original, { - type: EDITOR_START, + type: EDITOR_STOP, siteId: 2916284, postId: 841 } ); @@ -937,7 +938,7 @@ describe( 'reducer', () => { expect( state ).to.equal( original ); } ); - it( 'should discard edits when start editor action is dispatched', () => { + it( 'should discard edits when we stop editing the post', () => { const state = edits( deepFreeze( { 2916284: { 841: { @@ -948,7 +949,7 @@ describe( 'reducer', () => { } } } ), { - type: EDITOR_START, + type: EDITOR_STOP, siteId: 2916284, postId: 841 } ); @@ -962,6 +963,35 @@ describe( 'reducer', () => { } ); } ); + it( 'should reset edits when we start editing a post', () => { + const state = edits( deepFreeze( { + 2916284: { + 841: { + title: 'Hello World' + }, + '': { + title: 'Ribs & Chicken' + } + } + } ), { + type: EDITOR_START, + siteId: 2916284, + postId: 841, + postType: 'jetpack-testimonial', + } ); + + expect( state ).to.eql( { + 2916284: { + 841: { + type: 'jetpack-testimonial' + }, + '': { + title: 'Ribs & Chicken' + } + } + } ); + } ); + it( 'should not persist state', () => { const state = edits( deepFreeze( { 2916284: { diff --git a/client/state/ui/editor/actions.js b/client/state/ui/editor/actions.js index ac28d30c05884..2fc5acf71070f 100644 --- a/client/state/ui/editor/actions.js +++ b/client/state/ui/editor/actions.js @@ -1,7 +1,7 @@ /** * Internal dependencies */ -import { EDITOR_POST_ID_SET, EDITOR_SHOW_DRAFTS_TOGGLE, EDITOR_START } from 'state/action-types'; +import { EDITOR_SHOW_DRAFTS_TOGGLE, EDITOR_START, EDITOR_STOP } from 'state/action-types'; import { ModalViews } from 'state/ui/media-modal/constants'; import { setMediaModalView } from 'state/ui/media-modal/actions'; import { withAnalytics, bumpStat } from 'state/analytics/actions'; @@ -17,32 +17,36 @@ export const MODAL_VIEW_STATS = { }; /** - * Returns an action object to be used in signalling that the edited post ID has changed - * but we're still editing the same post + * Returns an action object to be used in signalling that the editor should + * begin to edit the post with the specified post ID, or `null` as a new post. * - * @param {?Number} postId Post ID - * @return {Object} Action object + * @param {Number} siteId Site ID + * @param {?Number} postId Post ID + * @param {String} postType Post Type + * @return {Object} Action object */ -export function setEditorPostId( postId ) { +export function startEditingPost( siteId, postId, postType ) { return { - type: EDITOR_POST_ID_SET, - postId + type: EDITOR_START, + siteId, + postId, + postType, }; } /** * Returns an action object to be used in signalling that the editor should - * begin to edit the post with the specified post ID, or `null` as a new post. + * stop editing. * - * @param {Number} siteId Site ID + * @param {Number} siteId Site ID * @param {?Number} postId Post ID * @return {Object} Action object */ -export function startPostEditor( siteId, postId ) { +export function stopEditingPost( siteId, postId ) { return { - type: EDITOR_START, + type: EDITOR_STOP, siteId, - postId + postId, }; } diff --git a/client/state/ui/editor/reducer.js b/client/state/ui/editor/reducer.js index 2b83e8009cd19..992a21ba9cc22 100644 --- a/client/state/ui/editor/reducer.js +++ b/client/state/ui/editor/reducer.js @@ -6,7 +6,7 @@ import { combineReducers } from 'redux'; /** * Internal dependencies */ -import { EDITOR_POST_ID_SET, EDITOR_SHOW_DRAFTS_TOGGLE, EDITOR_START } from 'state/action-types'; +import { EDITOR_SHOW_DRAFTS_TOGGLE, EDITOR_START, POST_SAVE_SUCCESS } from 'state/action-types'; import imageEditor from './image-editor/reducer'; import lastDraft from './last-draft/reducer'; import contactForm from './contact-form/reducer'; @@ -21,9 +21,10 @@ import contactForm from './contact-form/reducer'; */ export function postId( state = null, action ) { switch ( action.type ) { - case EDITOR_POST_ID_SET: case EDITOR_START: return action.postId; + case POST_SAVE_SUCCESS: + return state === action.postId ? action.savedPost.ID : state; } return state; diff --git a/client/state/ui/editor/test/actions.js b/client/state/ui/editor/test/actions.js index b8ac28dcdc46c..f5b0adef8bd05 100644 --- a/client/state/ui/editor/test/actions.js +++ b/client/state/ui/editor/test/actions.js @@ -9,37 +9,39 @@ import { forEach } from 'lodash'; */ import { ANALYTICS_STAT_BUMP, - EDITOR_POST_ID_SET, EDITOR_SHOW_DRAFTS_TOGGLE, - EDITOR_START + EDITOR_START, + EDITOR_STOP, } from 'state/action-types'; import { MODAL_VIEW_STAT_MAPPING, - setEditorPostId, toggleEditorDraftsVisible, setEditorMediaModalView, - startPostEditor + startEditingPost, + stopEditingPost, } from '../actions'; import { setMediaModalView } from 'state/ui/media-modal/actions'; describe( 'actions', () => { - describe( '#setEditorPostId()', () => { + describe( 'startEditingPost()', () => { it( 'should return an action object', () => { - const action = setEditorPostId( 183 ); + const action = startEditingPost( 10, 183, 'post' ); expect( action ).to.eql( { - type: EDITOR_POST_ID_SET, - postId: 183 + type: EDITOR_START, + siteId: 10, + postId: 183, + postType: 'post' } ); } ); } ); - describe( 'startPostEditor()', () => { + describe( 'stopEditingPost()', () => { it( 'should return an action object', () => { - const action = startPostEditor( 10, 183 ); + const action = stopEditingPost( 10, 183 ); expect( action ).to.eql( { - type: EDITOR_START, + type: EDITOR_STOP, siteId: 10, postId: 183 } ); diff --git a/client/state/ui/editor/test/reducer.js b/client/state/ui/editor/test/reducer.js index 7457ffd272ce6..6fcfc7b2abcd3 100644 --- a/client/state/ui/editor/test/reducer.js +++ b/client/state/ui/editor/test/reducer.js @@ -6,7 +6,7 @@ import { expect } from 'chai'; /** * Internal dependencies */ -import { EDITOR_POST_ID_SET, EDITOR_SHOW_DRAFTS_TOGGLE, EDITOR_START } from 'state/action-types'; +import { EDITOR_SHOW_DRAFTS_TOGGLE, EDITOR_START, POST_SAVE_SUCCESS } from 'state/action-types'; import reducer, { postId, showDrafts } from '../reducer'; describe( 'reducer', () => { @@ -27,24 +27,43 @@ describe( 'reducer', () => { expect( state ).to.be.null; } ); - it( 'should track editor post ID', () => { + it( 'should update the tracked id when starting the editor', () => { const state = postId( undefined, { - type: EDITOR_POST_ID_SET, + type: EDITOR_START, + siteId: 1, postId: 184 } ); expect( state ).to.equal( 184 ); } ); - it( 'should update the tracked id when starting the editor', () => { - const state = postId( undefined, { - type: EDITOR_START, + it( 'should update the tracked post id if we save a draft post', () => { + const state = postId( null, { + type: POST_SAVE_SUCCESS, siteId: 1, - postId: 184 + postId: null, + savedPost: { + ID: 184 + }, + post: {} } ); expect( state ).to.equal( 184 ); - } ) + } ); + + it( 'should not update the tracked post id if we save a draft post but we already switched the tracked post ID', () => { + const state = postId( 10, { + type: POST_SAVE_SUCCESS, + siteId: 1, + postId: null, + savedPost: { + ID: 184 + }, + post: {} + } ); + + expect( state ).to.equal( 10 ); + } ); } ); describe( '#showDrafts()', () => { From f8f19dd704a4e2ab3adff49cfe0e64333ea27c8d Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 3 Jan 2017 09:00:02 +0100 Subject: [PATCH 5/5] Changes per review --- client/state/ui/editor/actions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/state/ui/editor/actions.js b/client/state/ui/editor/actions.js index 2fc5acf71070f..36c1929e71453 100644 --- a/client/state/ui/editor/actions.js +++ b/client/state/ui/editor/actions.js @@ -25,7 +25,7 @@ export const MODAL_VIEW_STATS = { * @param {String} postType Post Type * @return {Object} Action object */ -export function startEditingPost( siteId, postId, postType ) { +export function startEditingPost( siteId, postId, postType = 'post' ) { return { type: EDITOR_START, siteId,