Forms: Refactor feedback status tracking and add unread count refresh (#46505)

* Forms: Refactor feedback status tracking and add tests

Replaces track_spam_status_change with track_feedback_status_change to handle feedback post status transitions, including spam and publish. Adds private helpers for spam meta and unread count recalculation. Comprehensive unit tests added for feedback status changes, spam meta updates, unread count recalculation, and non-feedback post handling.

* Add change log

* Fix the frontend as well.

* Update projects/packages/forms/src/contact-form/class-contact-form-plugin.php

Co-authored-by: Copilot <[email protected]>

* Fixing typos

Co-authored-by: Copilot <[email protected]>

* Add tests for updateMenuCounter and updateMenuCounterOptimistically

* fix typos

* Add js tests

* Update actions.tsx

* Refactor processStatusChange to separate module

Moved the processStatusChange logic from actions.tsx to a new process-status-change.ts file for better modularity and maintainability. Updated imports accordingly and enhanced tests to cover error handling and revert logic for menu counter updates.

* use shutdown hook instead

* Fix typo

Co-authored-by: Copilot <[email protected]>

* Move withTimeout to utils

* minor fixes

* fix changelog

* minor remove comment

* make the tests pass again

* update changelog

* minor fixes to comments

* Improve the class calling for readability

* Improve tests. Check that the action in registered

* Making undo work

---------

Co-authored-by: Copilot <[email protected]>

Author: enejb

projects/packages/forms/changelog/fix-unread-count-refresh

@@ -0,0 +1,4 @@
+Significance: patch
+Type: changed
+
+Fix unread count not updating when feedback status changes between published and unpublished states.

projects/packages/forms/src/contact-form/class-contact-form-plugin.php

@@ -320,8 +320,8 @@ protected function __construct() {
 			)
 		);
 
-		// Track when post status changes to 'spam' for accurate deletion timing
-		add_action( 'transition_post_status', array( $this, 'track_spam_status_change' ), 10, 3 );
+		// Track when post status changes to feedback posts types.
+		add_action( 'transition_post_status', array( $this, 'track_feedback_status_change' ), 10, 3 );
 
 		// POST handler
 		if (
@@ -3610,8 +3610,12 @@ public function untrash_feedback_status_handler( $current_status, $post_id, $pre
 	 * @param string       $new_status The new post status.
 	 * @param string       $old_status The old post status.
 	 * @param WP_Post|null $post       The post object, when available.
+	 *
+	 * @deprecated since $$next-version$$
 	 */
 	public function track_spam_status_change( $new_status, $old_status, ?WP_Post $post = null ) {
+		_deprecated_function( __METHOD__, 'package-jetpack-forms-$$next-version$$' );
+
 		if ( ! $post instanceof WP_Post ) {
 			// Some callers fire the action without a populated post object (e.g. failed get_post lookups).
 			return;
@@ -3622,13 +3626,67 @@ public function track_spam_status_change( $new_status, $old_status, ?WP_Post $po
 			return;
 		}
 
+		$this->track_spam_status( $new_status, $old_status, $post->ID );
+	}
+
+	/**
+	 * Tracks when a feedback post status changes and triggers related handlers.
+	 * Used to handle spam meta tracking and unread count recalculation for feedback posts.
+	 *
+	 * @param string       $new_status The new post status.
+	 * @param string       $old_status The old post status.
+	 * @param WP_Post|null $post       The post object, when available.
+	 */
+	public function track_feedback_status_change( $new_status, $old_status, ?WP_Post $post = null ) {
+		if ( ! $post instanceof WP_Post ) {
+			// Some callers fire the action without a populated post object (e.g. failed get_post lookups).
+			return;
+		}
+
+		// Only track for feedback posts
+		if ( 'feedback' !== $post->post_type ) {
+			return;
+		}
+		$this->track_spam_status( $new_status, $old_status, $post->ID );
+		$this->track_recount_unread( $new_status, $old_status, $post );
+	}
+
+	/**
+	 * Tracks when a feedback post status changes to 'spam' and stores the timestamp.
+	 * This allows us to accurately determine when spam was marked, independent of other post updates.
+	 *
+	 * @param string $new_status The new post status.
+	 * @param string $old_status The old post status.
+	 * @param int    $post_id    The post ID.
+	 */
+	private function track_spam_status( $new_status, $old_status, $post_id ) {
 		// Only track when status changes TO spam (not from spam to something else)
 		if ( 'spam' === $new_status && 'spam' !== $old_status ) {
 			// Store the current GMT timestamp when status changes to spam
-			update_post_meta( $post->ID, '_spam_status_changed_gmt', current_time( 'mysql', 1 ) );
+			update_post_meta( $post_id, '_spam_status_changed_gmt', current_time( 'mysql', 1 ) );
 		} elseif ( 'spam' === $old_status && 'spam' !== $new_status ) {
 			// Remove the meta when post is no longer spam
-			delete_post_meta( $post->ID, '_spam_status_changed_gmt' );
+			delete_post_meta( $post_id, '_spam_status_changed_gmt' );
+		}
+	}
+
+	/**
+	 * Tracks when a feedback post status changes to or from 'publish' and triggers unread count recalculation.
+	 *
+	 * @param string  $new_status The new post status.
+	 * @param string  $old_status The old post status.
+	 * @param WP_Post $post       The post object.
+	 */
+	private function track_recount_unread( $new_status, $old_status, WP_Post $post ) {
+		// If the feedback is already marked as read, it doesn't matter if its status changes.
+		if ( $post->comment_status === Feedback::STATUS_READ ) {
+			return;
+		}
+
+		// If the status changed to or from 'publish', we need to recount unread feedbacks.
+		if ( ( 'publish' === $new_status && 'publish' !== $old_status ) ||
+			( 'publish' === $old_status && 'publish' !== $new_status ) ) {
+			add_action( 'shutdown', array( __CLASS__, 'recalculate_unread_count' ) );
 		}
 	}
 

projects/packages/forms/src/dashboard/inbox/stage/actions.tsx

@@ -13,13 +13,13 @@ import { store as noticesStore } from '@wordpress/notices';
  */
 import { notSpam, spam } from '../../icons/index.ts';
 import { store as dashboardStore } from '../../store/index.js';
-import { updateMenuCounter, updateMenuCounterOptimistically } from '../utils.js';
+import { updateMenuCounter, updateMenuCounterOptimistically, withTimeout } from '../utils.js';
+import { optimisticallyUpdateUnreadCount, processStatusChange } from './process-status-change';
 import { defaultView } from './views.js';
 /**
  * Types
  */
-import type { Action, DispatchActions, QueryParams, Registry } from './types.tsx';
-import type { FormResponse } from '../../../types/index.ts';
+import type { Action, QueryParams, Registry } from './types.tsx';
 
 /**
  * Helper function to extract count-relevant query params from the current query.
@@ -132,26 +132,6 @@ const getGenericErrorMessage = ( numberOfErrors: number ): string => {
 		  );
 };
 
-/**
- * Wraps a promise with a timeout to ensure it rejects after a reasonable time.
- * This is useful for network requests that might hang when the network is disabled.
- *
- * @param {Promise} promise   - The promise to wrap.
- * @param {number}  timeoutMs - The timeout in milliseconds (default: 30000).
- * @return {Promise} The wrapped promise that will reject on timeout.
- */
-const withTimeout = (
-	promise: Promise< unknown >,
-	timeoutMs: number = 30000
-): Promise< unknown > => {
-	return Promise.race( [
-		promise,
-		new Promise( ( _, reject ) =>
-			setTimeout( () => reject( new Error( 'Request timeout' ) ), timeoutMs )
-		),
-	] );
-};
-
 /*
  * Waits until the current entity records query resolves (or times out).
  */
@@ -176,101 +156,6 @@ const waitForEntityRecordsResolution = async (
 	}
 };
 
-/**
- * Type for the result of processStatusChange.
- */
-type StatusChangeResult = {
-	itemsUpdated: { id: number }[];
-	itemsFailed: number[];
-	numberOfErrors: number;
-};
-
-type ProcessStatusChangeParams = {
-	items: FormResponse[];
-	newStatus: string;
-	apiCall: ( id: number ) => Promise< unknown >;
-	editEntityRecord: DispatchActions[ 'editEntityRecord' ];
-	updateCountsOptimistically: DispatchActions[ 'updateCountsOptimistically' ];
-	queryParams: QueryParams;
-};
-
-/**
- * Helper function to process status changes with optimistic updates and error handling.
- * Optimistic Update Strategy:
- * 1. Immediately update local state and counts
- * 2. Make API call
- * 3. On success: invalidate cache to sync with server
- * 4. On failure: rollback local changes
- * 5. Undo actions must preserve original status for proper restoration
- * @param {object}         params                            - The parameters for the status change.
- * @param {FormResponse[]} params.items                      - The items to update.
- * @param {string}         params.newStatus                  - The new status to set.
- * @param {Function}       params.apiCall                    - The API call function (saveEntityRecord or deleteEntityRecord).
- * @param {Function}       params.editEntityRecord           - The editEntityRecord dispatch function.
- * @param {Function}       params.updateCountsOptimistically - The updateCountsOptimistically dispatch function.
- * @param {QueryParams}    params.queryParams                - The query params for count updates.
- * @return {Promise<StatusChangeResult>} The result of the status change operation.
- */
-const processStatusChange = async ( {
-	items,
-	newStatus,
-	apiCall,
-	editEntityRecord,
-	updateCountsOptimistically,
-	queryParams,
-}: ProcessStatusChangeParams ): Promise< StatusChangeResult > => {
-	// Store original statuses before making optimistic changes
-	const originalStatuses = items.map( item => item.status );
-
-	// Make optimistic updates
-	items.forEach( item => {
-		editEntityRecord( 'postType', 'feedback', item.id, {
-			status: newStatus,
-		} );
-
-		// Update counts optimistically
-		updateCountsOptimistically( item.status, newStatus, 1, queryParams );
-	} );
-
-	// Call API with timeout
-	const promises = await Promise.allSettled(
-		items.map( ( { id } ) => withTimeout( apiCall( id ) ) as Promise< { id: number } > )
-	);
-
-	// Check for both rejected promises and fulfilled promises with undefined/invalid results
-	// const itemsUpdated: PromiseFulfilledResult< { id: number } >[] = [];
-	const itemsUpdated: { id: number }[] = [];
-	const itemsFailed: number[] = [];
-
-	promises.forEach( ( promise, index ) => {
-		// Failed if rejected OR if fulfilled but result is invalid
-		if ( promise.status === 'rejected' || ! promise.value?.id ) {
-			itemsFailed.push( index );
-		} else {
-			itemsUpdated.push( promise.value );
-		}
-	} );
-
-	// Revert optimistic changes for failed items
-	itemsFailed.forEach( index => {
-		const item = items[ index ];
-		const originalStatus = originalStatuses[ index ];
-
-		editEntityRecord( 'postType', 'feedback', item.id, {
-			status: originalStatus,
-		} );
-
-		// Revert the count change
-		updateCountsOptimistically( newStatus, originalStatus, 1, queryParams );
-	} );
-
-	return {
-		itemsUpdated,
-		itemsFailed,
-		numberOfErrors: itemsFailed.length,
-	};
-};
-
 export const BULK_ACTIONS = {
 	markAsSpam: 'mark_as_spam',
 	markAsNotSpam: 'mark_as_not_spam',
@@ -417,6 +302,9 @@ export const markAsSpamAction: Action = {
 				} else {
 					// Remove the info notice when undo completes successfully
 					removeNotice( 'mark-as-spam-action' );
+					items.forEach( item => {
+						optimisticallyUpdateUnreadCount( 'spam', 'publish', item.is_unread );
+					} );
 				}
 			} else {
 				// There is at least one failure.
@@ -551,6 +439,9 @@ export const markAsNotSpamAction: Action = {
 					} );
 				} else {
 					removeNotice( 'mark-as-not-spam-action' );
+					items.forEach( item => {
+						optimisticallyUpdateUnreadCount( 'publish', 'spam', item.is_unread );
+					} );
 				}
 			} else {
 				// There is at least one failure.
@@ -678,6 +569,10 @@ export const restoreAction: Action = {
 					} );
 				} else {
 					removeNotice( 'restore-action' );
+					items.forEach( item => {
+						// Since you can send an item to Trash from both Spam and Inbox, we need to restore the unread count based on the new status.
+						optimisticallyUpdateUnreadCount( newStatus, 'trash', item.is_unread );
+					} );
 				}
 
 				return;
@@ -818,6 +713,11 @@ export const moveToTrashAction: Action = {
 					} );
 				} else {
 					removeNotice( 'move-to-trash-action' );
+					// This is an undo action that moves the item from and is triggered by Undo in restore.
+					// Which means that we are on the Trash view and wanted to restore the item back to 'publish' but then decided to undo that.
+					items.forEach( item => {
+						optimisticallyUpdateUnreadCount( 'trash', 'publish', item.is_unread );
+					} );
 				}
 
 				return;