From 8f0dbd2c0324407c10c1fb9c0dff2e82fd3b40c6 Mon Sep 17 00:00:00 2001 From: Udit Desai Date: Sun, 6 Jun 2021 12:45:43 +1000 Subject: [PATCH 1/4] Add data store actions/selectors for current user session in block editor. --- packages/core-data/src/actions.js | 14 ++++++++++++++ packages/core-data/src/reducer.js | 24 ++++++++++++++++++++++++ packages/core-data/src/selectors.ts | 16 ++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/packages/core-data/src/actions.js b/packages/core-data/src/actions.js index 49776d0562984f..23f6e4eae242e7 100644 --- a/packages/core-data/src/actions.js +++ b/packages/core-data/src/actions.js @@ -57,6 +57,20 @@ export function receiveCurrentUser( currentUser ) { }; } +/** + * Returns an action object used to lock the editor. + * + * @param {Object} currentUserSession Meta information about the login session for current user. + * + * @return {Object} Action object. + */ +export function __experimentalUpdateCurrentUserSession( currentUserSession ) { + return { + type: 'UPDATE_CURRENT_USER_SESSION', + currentUserSession, + }; +} + /** * Returns an action object used in adding new entities. * diff --git a/packages/core-data/src/reducer.js b/packages/core-data/src/reducer.js index 8e6be425244687..18838dc83bb37f 100644 --- a/packages/core-data/src/reducer.js +++ b/packages/core-data/src/reducer.js @@ -92,6 +92,29 @@ export function currentUser( state = {}, action ) { return state; } +/** + * Reducer returning the login session for current user. + * + * The default state is { isActive: true }. We are just going to assume that by default the user session is active. + * If the block editor is already being loaded for user and if the data store is already being initialised for the user, + * it means user is already logged in. + * Eventually Heartbeat API can update this state if the auth cookie either expires or is deleted for the user. + * + * @param {Object} state Current state for login session. + * @param {Object} action Dispatched action. + */ +export function currentUserSession( state = { isActive: true }, action ) { + switch ( action.type ) { + case 'UPDATE_CURRENT_USER_SESSION': + return { + ...state, + ...action.currentUserSession, + }; + } + + return state; +} + /** * Reducer managing taxonomies. * @@ -629,6 +652,7 @@ export default combineReducers( { currentTheme, currentGlobalStylesId, currentUser, + currentUserSession, themeGlobalStyleVariations, themeBaseGlobalStyles, themeGlobalStyleRevisions, diff --git a/packages/core-data/src/selectors.ts b/packages/core-data/src/selectors.ts index a8486494302600..f9872506659f57 100644 --- a/packages/core-data/src/selectors.ts +++ b/packages/core-data/src/selectors.ts @@ -36,6 +36,7 @@ export interface State { currentGlobalStylesId: string; currentTheme: string; currentUser: ET.User< 'edit' >; + currentUserSession: { isActive: boolean }; embedPreviews: Record< string, { html: string } >; entities: EntitiesState; themeBaseGlobalStyles: Record< string, Object >; @@ -185,6 +186,21 @@ export function getCurrentUser( state: State ): ET.User< 'edit' > { return state.currentUser; } +/** + *Returns a boolean flag for current user's active login session. + * + * By default this returns true, because we assume that by default the user session is active. + * If the block editor is already being loaded for user and if the data store is already being initialised for the user, + * it means user is already logged in. + * + * @param {Object} state Current state data. + * + * @return {boolean} Whether the current user has active login maintained or not. + */ +export function __experimentalIsCurrentUserSessionActive( state: State ) { + return state.currentUserSession?.isActive ?? true; +} + /** * Returns all the users returned by a query ID. * From 9d4260d8eb98009fc554647a452f05bc59d5ff98 Mon Sep 17 00:00:00 2001 From: Udit Desai Date: Sun, 6 Jun 2021 12:49:59 +1000 Subject: [PATCH 2/4] Add post locked check in isEditedPostSaveable. So that we can prevent saving if the post is locked. We also prevent disabling Preview button when the post is not saveable. This is ripple effect of adding isPostLocked check in `isEditedPostSaveable` selector. We can safely remove this check on `Preview` button, because #32341 will prevent any save actions under the hood. --- .../editor/src/components/post-preview-button/index.js | 10 +++++----- packages/editor/src/store/selectors.js | 4 ++++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/editor/src/components/post-preview-button/index.js b/packages/editor/src/components/post-preview-button/index.js index 9a9aa92d210c3d..7f45e61ee62f21 100644 --- a/packages/editor/src/components/post-preview-button/index.js +++ b/packages/editor/src/components/post-preview-button/index.js @@ -106,8 +106,8 @@ export default function PostPreviewButton( { role, onPreview, } ) { - const { postId, currentPostLink, previewLink, isSaveable, isViewable } = - useSelect( ( select ) => { + const { postId, currentPostLink, previewLink, isViewable } = useSelect( + ( select ) => { const editor = select( editorStore ); const core = select( coreStore ); @@ -119,10 +119,11 @@ export default function PostPreviewButton( { postId: editor.getCurrentPostId(), currentPostLink: editor.getCurrentPostAttribute( 'link' ), previewLink: editor.getEditedPostPreviewLink(), - isSaveable: editor.isEditedPostSaveable(), isViewable: postType?.viewable ?? false, }; - }, [] ); + }, + [] + ); const { __unstableSaveForPreview } = useDispatch( editorStore ); @@ -168,7 +169,6 @@ export default function PostPreviewButton( { className={ className || 'editor-post-preview' } href={ href } target={ targetId } - disabled={ ! isSaveable } onClick={ openPreviewWindow } role={ role } > diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index f7bf1f58bba395..18010511b53902 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -495,6 +495,10 @@ export function isEditedPostSaveable( state ) { return false; } + if ( isPostLocked( state ) ) { + return false; + } + // TODO: Post should not be saveable if not dirty. Cannot be added here at // this time since posts where meta boxes are present can be saved even if // the post is not dirty. Currently this restriction is imposed at UI, but From 97e7ba9688052945c15cec936a88335ab09d47fc Mon Sep 17 00:00:00 2001 From: Udit Desai Date: Sun, 6 Jun 2021 13:00:58 +1000 Subject: [PATCH 3/4] Refactor PostLockedModal component. - Prevent autosave right before releasing the post lock. - Update current user session as per the heartbeat response. - We need to prevent save/update actions if the post is locked right after user logs back in on the same page. To do this, we need to show the modal as soon as user logs back in. Hence, we will re-trigger heartbeat right after nonce is renewed, so that we can update isPostLocked in data store. --- .../src/components/post-locked-modal/index.js | 78 ++++++++++++++++++- 1 file changed, 74 insertions(+), 4 deletions(-) diff --git a/packages/editor/src/components/post-locked-modal/index.js b/packages/editor/src/components/post-locked-modal/index.js index 7597bd9b4f3f30..d1c0f24b7481f7 100644 --- a/packages/editor/src/components/post-locked-modal/index.js +++ b/packages/editor/src/components/post-locked-modal/index.js @@ -10,7 +10,11 @@ import { } from '@wordpress/components'; import { useSelect, useDispatch } from '@wordpress/data'; import { addQueryArgs } from '@wordpress/url'; -import { useEffect, createInterpolateElement } from '@wordpress/element'; +import { + useEffect, + useState, + createInterpolateElement, +} from '@wordpress/element'; import { addAction, removeAction } from '@wordpress/hooks'; import { useInstanceId } from '@wordpress/compose'; import { store as coreStore } from '@wordpress/core-data'; @@ -21,10 +25,14 @@ import { store as coreStore } from '@wordpress/core-data'; import { store as editorStore } from '../../store'; export default function PostLockedModal() { + const [ heartbeatTickTriggered, triggerHeartbeatTick ] = useState( false ); const instanceId = useInstanceId( PostLockedModal ); const hookName = 'core/editor/post-locked-modal-' + instanceId; const { autosave, updatePostLock } = useDispatch( editorStore ); + const { __experimentalUpdateCurrentUserSession: updateCurrentUserSession } = + useDispatch( coreStore ); const { + isDraft, isLocked, isTakeover, user, @@ -33,6 +41,7 @@ export default function PostLockedModal() { activePostLock, postType, previewLink, + isCurrentUserSessionActive, } = useSelect( ( select ) => { const { isPostLocked, @@ -44,8 +53,12 @@ export default function PostLockedModal() { getEditedPostPreviewLink, getEditorSettings, } = select( editorStore ); - const { getPostType } = select( coreStore ); + const { getPostType, __experimentalIsCurrentUserSessionActive } = + select( coreStore ); return { + isDraft: [ 'draft', 'auto-draft' ].includes( + getEditedPostAttribute( 'status' ) + ), isLocked: isPostLocked(), isTakeover: isPostLockTakeover(), user: getPostLockUser(), @@ -54,9 +67,61 @@ export default function PostLockedModal() { activePostLock: getActivePostLock(), postType: getPostType( getEditedPostAttribute( 'type' ) ), previewLink: getEditedPostPreviewLink(), + isCurrentUserSessionActive: + __experimentalIsCurrentUserSessionActive(), }; }, [] ); + useEffect( () => { + const checkUserLoginHookName = `core/editor/check-user-login/post-locked-modal-${ instanceId }`; + + /** + * Binds to the Heartbeat Tick event and checks if the user is logged in or not. + * + * @param {Object} data Data received in the heartbeat request + */ + function checkUserLogin( data ) { + // Bail early, if the heartbeat has neither auth check data attribute nor nonce expiry attribute. + if ( + ! [ 'wp-auth-check', 'nonces_expired' ].some( ( attr ) => + Object.prototype.hasOwnProperty.call( data, attr ) + ) + ) { + return; + } + + if ( ! data[ 'wp-auth-check' ] && isCurrentUserSessionActive ) { + // Update login state to false, + // if server does not respond with auth check + // and current login state is true. + updateCurrentUserSession( { isActive: false } ); + } else if ( ! isCurrentUserSessionActive && data.nonces_expired ) { + // If current login state is false and nonce is renewed, + // trigger a heartbeat tick to check for latest post lock. + // Because first heartbeat will only renew the nonce. + // It will not return any errors if the post is already locked by other user or not. + if ( window.wp?.heartbeat?.connectNow ) { + triggerHeartbeatTick( true ); + window.wp.heartbeat.connectNow(); + } + } else if ( + ! isCurrentUserSessionActive && + heartbeatTickTriggered && + data[ 'wp-auth-check' ] + ) { + // If current login state is false and server responds with auth check for previously triggered heartbeat, + // update login state to true. + updateCurrentUserSession( { isActive: true } ); + } + } + + addAction( 'heartbeat.tick', checkUserLoginHookName, checkUserLogin ); + + return () => { + removeAction( 'heartbeat.tick', checkUserLoginHookName ); + }; + }, [ heartbeatTickTriggered, isCurrentUserSessionActive ] ); + useEffect( () => { /** * Keep the lock refreshed. @@ -90,7 +155,12 @@ export default function PostLockedModal() { const received = data[ 'wp-refresh-post-lock' ]; if ( received.lock_error ) { // Auto save and display the takeover modal. - autosave(); + // Meanwhile, other user may have opened the post and started to make new changes. + // Post may have been published or may have been switched back to draft. + // In either case, we don't want to accidentally loose the changes made by other user. + // If we perform server side autosave, and the post is in draft, previous changes will be overwritten in the database. + // Hence, we will only perform local autosave. + autosave( { local: true } ); updatePostLock( { isLocked: true, isTakeover: true, @@ -141,7 +211,7 @@ export default function PostLockedModal() { removeAction( 'heartbeat.tick', hookName ); window.removeEventListener( 'beforeunload', releasePostLock ); }; - }, [] ); + }, [ isDraft, isLocked ] ); if ( ! isLocked ) { return null; From 1ccdedae001041530e2c3b7baca9a089a2271561 Mon Sep 17 00:00:00 2001 From: Udit Desai Date: Sun, 6 Jun 2021 15:09:28 +1000 Subject: [PATCH 4/4] Fix unit tests. --- .../post-preview-button/test/index.js | 16 -------------- packages/editor/src/store/selectors.js | 2 +- packages/editor/src/store/test/selectors.js | 21 +++++++++++++++++++ 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/packages/editor/src/components/post-preview-button/test/index.js b/packages/editor/src/components/post-preview-button/test/index.js index e34c05caa178bd..d9a8fd0efa2243 100644 --- a/packages/editor/src/components/post-preview-button/test/index.js +++ b/packages/editor/src/components/post-preview-button/test/index.js @@ -139,22 +139,6 @@ describe( 'PostPreviewButton', () => { ).toBeInTheDocument(); } ); - it( 'should be disabled if post is not saveable.', () => { - mockUseSelect( { isEditedPostSaveable: () => false } ); - - render( ); - - expect( screen.getByRole( 'button' ) ).toBeDisabled(); - } ); - - it( 'should not be disabled if post is saveable.', () => { - mockUseSelect( { isEditedPostSaveable: () => true } ); - - render( ); - - expect( screen.getByRole( 'button' ) ).toBeEnabled(); - } ); - it( 'should set `href` to edited post preview link if specified.', () => { const url = 'https://wordpress.org'; mockUseSelect( { diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index 18010511b53902..bdb51e3b7865e1 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -1035,7 +1035,7 @@ export function getPermalinkParts( state ) { * @return {boolean} Is locked. */ export function isPostLocked( state ) { - return state.postLock.isLocked; + return state.postLock?.isLocked ?? false; } /** diff --git a/packages/editor/src/store/test/selectors.js b/packages/editor/src/store/test/selectors.js index 1de25604ebd7e3..546adfe34afcca 100644 --- a/packages/editor/src/store/test/selectors.js +++ b/packages/editor/src/store/test/selectors.js @@ -1421,6 +1421,27 @@ describe( 'selectors', () => { expect( isEditedPostSaveable( state ) ).toBe( true ); } ); + + it( 'should return false if the post is locked', () => { + const state = { + editor: { + present: { + blocks: { + value: [], + }, + edits: {}, + }, + }, + initialEdits: {}, + currentPost: {}, + saving: {}, + postLock: { + isLocked: true, + }, + }; + + expect( isEditedPostSaveable( state ) ).toBe( false ); + } ); } ); describe( 'isEditedPostAutosaveable', () => {