VideoPress: Enhance behavior when deleting multiple videos (#28302)

* refactor delete action adding FLUSH_DELETED_VIDEOS * changelog * fix removedVideoCount processing order * enhancements from code review * remove videosBeingRemovedCount
Automattic · Jan 12, 2023 · a000af6 · a000af6
1 parent 394ee81
commit a000af6
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 20 deletions.
diff --git a/projects/packages/videopress/changelog/fix-videopress-delete-behavior b/projects/packages/videopress/changelog/fix-videopress-delete-behavior
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fixed
+
+VideoPress: Enhance behavior when deleting multiple videos
diff --git a/projects/packages/videopress/src/class-data.php b/projects/packages/videopress/src/class-data.php
@@ -354,7 +354,8 @@ function ( $video ) {
 				),
 				'query'                        => $videopress_data['query'],
 				'_meta'                        => array(
-					'relyOnInitialState' => true,
+					'processedAllVideosBeingRemoved' => true,
+					'relyOnInitialState'             => true,
 				),
 			),
 			'localVideos'  => array(

diff --git a/projects/packages/videopress/src/client/state/actions.js b/projects/packages/videopress/src/client/state/actions.js
@@ -51,6 +51,7 @@ import {
 	SET_VIDEOPRESS_SETTINGS,
 	WP_REST_API_VIDEOPRESS_SETTINGS_ENDPOINT,
 	UPDATE_PAGINATION_AFTER_DELETE,
+	FLUSH_DELETED_VIDEOS,
 } from './constants';
 import { mapVideoFromWPV2MediaEndpoint } from './utils/map-videos';
 
@@ -204,11 +205,13 @@ const removeVideo = id => {
  * @param {string|number} id - Video post ID
  * @returns {Function} Thunk action
  */
-const deleteVideo = id => async ( { dispatch } ) => {
+const deleteVideo = id => async ( { dispatch, select } ) => {
 	// Let's be optimistic and update the UI right away.
 	// @todo: Add a loading state to the state/UI.
 	dispatch.removeVideo( id );
 
+	let deleteAction = { type: DELETE_VIDEO, id, hasBeenDeleted: false, video: {} };
+
 	try {
 		const resp = await apiFetch( {
 			path: `${ WP_REST_API_MEDIA_ENDPOINT }/${ id }`,
@@ -220,15 +223,21 @@ const deleteVideo = id => async ( { dispatch } ) => {
 		} );
 
 		// dispach action to invalidate the cache
-		if ( ! resp?.deleted ) {
-			dispatch( { type: DELETE_VIDEO, id, hasBeenDeleted: false, video: {} } );
-		} else {
-			dispatch( { type: UPDATE_PAGINATION_AFTER_DELETE } );
-			dispatch( { type: DELETE_VIDEO, id, hasBeenDeleted: true, video: resp?.previous } );
+		if ( resp?.deleted ) {
+			deleteAction = { ...deleteAction, hasBeenDeleted: true, video: resp?.previous };
 		}
 	} catch ( error ) {
 		// @todo: implement error handling / UI
 		console.error( error ); // eslint-disable-line no-console
+	} finally {
+		dispatch( deleteAction );
+	}
+
+	const processedAllVideosBeingRemoved = select.getProcessedAllVideosBeingRemoved();
+
+	if ( processedAllVideosBeingRemoved ) {
+		dispatch( { type: UPDATE_PAGINATION_AFTER_DELETE } );
+		dispatch( { type: FLUSH_DELETED_VIDEOS } );
 	}
 };
 

diff --git a/projects/packages/videopress/src/client/state/constants.js b/projects/packages/videopress/src/client/state/constants.js
@@ -40,6 +40,7 @@ export const SET_VIDEO_PRIVACY = 'SET_VIDEO_PRIVACY';
 export const UPDATE_VIDEO_PRIVACY = 'UPDATE_VIDEO_PRIVACY';
 export const DELETE_VIDEO = 'DELETE_VIDEO';
 export const REMOVE_VIDEO = 'REMOVE_VIDEO';
+export const FLUSH_DELETED_VIDEOS = 'FLUSH_DELETED_VIDEOS';
 export const UPDATE_PAGINATION_AFTER_DELETE = 'UPDATE_PAGINATION_AFTER_DELETE';
 
 export const SET_VIDEO_UPLOADING = 'SET_VIDEO_UPLOADING';

diff --git a/projects/packages/videopress/src/client/state/reducers.js b/projects/packages/videopress/src/client/state/reducers.js
@@ -41,6 +41,7 @@ import {
 	EXPIRE_PLAYBACK_TOKEN,
 	SET_VIDEOPRESS_SETTINGS,
 	DISMISS_FIRST_VIDEO_POPOVER,
+	FLUSH_DELETED_VIDEOS,
 	UPDATE_PAGINATION_AFTER_DELETE,
 } from './constants';
 
@@ -247,10 +248,14 @@ const videos = ( state, action ) => {
 			return {
 				...state,
 				// Do not remove the video from the list, just update the meta data.
-				// Keep here in caswe we want to do it in the future.
+				// Keep here in case we want to do it in the future.
 				// items: [ ...state.items.slice( 0, videoIndex ), ...state.items.slice( videoIndex + 1 ) ],
 				_meta: {
 					...state._meta,
+					videosBeingRemoved: [
+						{ id, processed: false, deleted: false },
+						...( state._meta.videosBeingRemoved ?? [] ),
+					],
 					items: {
 						..._metaItems,
 						[ id ]: {
@@ -270,23 +275,37 @@ const videos = ( state, action ) => {
 			const { id, hasBeenDeleted, video: deletedVideo } = action;
 			const _metaItems = state?._meta?.items || [];
 			const _metaVideo = _metaItems[ id ] || {};
-			const uploadedVideoCount = state.uploadedVideoCount - 1;
+			const videosBeingRemoved = [ ...( state._meta.videosBeingRemoved ?? [] ) ];
+			const removedVideoIndex = videosBeingRemoved.findIndex( item => item.id === id );
 
-			if ( ! _metaVideo ) {
+			if ( ! _metaVideo || removedVideoIndex < 0 ) {
 				return state;
 			}
 
+			videosBeingRemoved[ removedVideoIndex ].processed = true;
+			videosBeingRemoved[ removedVideoIndex ].deleted = hasBeenDeleted;
+
+			const processedAllVideosBeingRemoved =
+				videosBeingRemoved.filter( item => ! item.processed ).length === 0;
+
+			let uploadedVideoCount = state.uploadedVideoCount ?? 0;
+
+			if ( processedAllVideosBeingRemoved ) {
+				const videosBeingRemovedCount = videosBeingRemoved.filter( item => item.deleted ).length;
+				uploadedVideoCount = uploadedVideoCount - videosBeingRemovedCount;
+			}
+
 			return {
 				...state,
 				uploadedVideoCount,
 				_meta: {
 					...state._meta,
-					relyOnInitialState: false,
+					videosBeingRemoved,
+					processedAllVideosBeingRemoved,
 					items: {
 						..._metaItems,
 						[ id ]: {
 							..._metaVideo,
-							isDeleting: false,
 							hasBeenDeleted,
 							deletedVideo,
 						},
@@ -295,23 +314,36 @@ const videos = ( state, action ) => {
 			};
 		}
 
+		case FLUSH_DELETED_VIDEOS: {
+			return {
+				...state,
+				_meta: {
+					...state._meta,
+					videosBeingRemoved: [],
+					relyOnInitialState: false,
+				},
+			};
+		}
+
 		/*
 		 * Check if query and pagination should change
 		 * after deleting video
 		 */
 		case UPDATE_PAGINATION_AFTER_DELETE: {
-			const { items = [], query = {}, pagination = {} } = state;
+			const { items = [], query = {}, pagination = {}, _meta = {} } = state;
+			const { videosBeingRemoved = [] } = _meta;
+			const videosBeingRemovedCount = videosBeingRemoved.filter( item => item.deleted ).length;
 
-			// If is the last video, reduce the page per 1
+			// If the last videos of the page are deleted, reduce the page by 1
 			// Being optimistic here
-			const isLastVideo = items?.length === 1;
+			const areLastVideos = items?.length === videosBeingRemovedCount;
 			const currentPage = query?.page ?? 1;
 			const currentTotalPage = pagination?.totalPages ?? 1;
 			const currentTotal = pagination?.total;
 
-			const page = isLastVideo && currentPage > 1 ? currentPage - 1 : currentPage;
+			const page = areLastVideos && currentPage > 1 ? currentPage - 1 : currentPage;
 			const totalPages =
-				isLastVideo && currentTotalPage > 1 ? currentTotalPage - 1 : currentTotalPage;
+				areLastVideos && currentTotalPage > 1 ? currentTotalPage - 1 : currentTotalPage;
 
 			return {
 				...state,

diff --git a/projects/packages/videopress/src/client/state/resolvers.js b/projects/packages/videopress/src/client/state/resolvers.js
@@ -10,7 +10,7 @@ import {
 	SET_VIDEOS_QUERY,
 	SET_VIDEOS_FILTER,
 	WP_REST_API_MEDIA_ENDPOINT,
-	DELETE_VIDEO,
+	FLUSH_DELETED_VIDEOS,
 	REST_API_SITE_INFO_ENDPOINT,
 	SET_VIDEO_PROCESSING,
 	SET_LOCAL_VIDEOS_QUERY,
@@ -164,8 +164,9 @@ const getVideos = {
 			console.error( error ); // eslint-disable-line no-console
 		}
 	},
+
 	shouldInvalidate: ( { type } ) => {
-		return type === SET_VIDEOS_QUERY || type === DELETE_VIDEO || type === SET_VIDEOS_FILTER;
+		return type === SET_VIDEOS_QUERY || type === FLUSH_DELETED_VIDEOS || type === SET_VIDEOS_FILTER;
 	},
 };
 
@@ -189,6 +190,7 @@ const getVideo = {
 
 		return video;
 	},
+
 	fulfill: id => async ( { dispatch, resolveSelect } ) => {
 		dispatch.setIsFetchingVideos( true );
 
@@ -241,7 +243,7 @@ const getUploadedVideoCount = {
 	},
 
 	shouldInvalidate: action => {
-		return action.type === SET_VIDEO_PROCESSING || action.type === DELETE_VIDEO;
+		return action.type === SET_VIDEO_PROCESSING || action.type === FLUSH_DELETED_VIDEOS;
 	},
 };
 
@@ -323,6 +325,7 @@ const getLocalVideos = {
 			console.error( error ); // eslint-disable-line no-console
 		}
 	},
+
 	shouldInvalidate: action => {
 		return action.type === SET_LOCAL_VIDEOS_QUERY;
 	},
@@ -371,6 +374,7 @@ const getPlaybackToken = {
 		const playbackTokens = state?.playbackTokens?.items ?? [];
 		return playbackTokens?.some( token => token?.guid === guid );
 	},
+
 	fulfill: guid => async ( { dispatch } ) => {
 		dispatch.setIsFetchingPlaybackToken( true );
 
@@ -393,6 +397,7 @@ const getPlaybackToken = {
 			console.error( error ); // eslint-disable-line no-console
 		}
 	},
+
 	shouldInvalidate: ( action, guid ) => {
 		return action.type === EXPIRE_PLAYBACK_TOKEN && action.guid === guid;
 	},
@@ -402,6 +407,7 @@ const getVideoPressSettings = {
 	isFulfilled: state => {
 		return state?.siteSettings !== undefined;
 	},
+
 	fulfill: () => async ( { dispatch } ) => {
 		try {
 			const { videopress_videos_private_for_site: videoPressVideosPrivateForSite } = await apiFetch(

diff --git a/projects/packages/videopress/src/client/state/selectors.js b/projects/packages/videopress/src/client/state/selectors.js
@@ -55,6 +55,10 @@ export const getDismissedFirstVideoPopover = state => {
 	return state?.videos?.dismissedFirstVideoPopover;
 };
 
+export const getProcessedAllVideosBeingRemoved = state => {
+	return state?.videos?._meta?.processedAllVideosBeingRemoved;
+};
+
 // Single Video stuff
 export const getVideo = ( state, id ) => {
 	const videos = getVideos( state );
@@ -128,6 +132,7 @@ const selectors = {
 	getFirstUploadedVideoId,
 	getFirstVideoProcessed,
 	getDismissedFirstVideoPopover,
+	getProcessedAllVideosBeingRemoved,
 
 	// Local videos
 	getLocalVideos,