Merge pull request #560 from Automattic/fix/notifications-status

Ensure status_friendly_names are provided for notifications

modules/notifications/notifications.php

@@ -586,6 +586,28 @@ function notification_status_change( $new_status, $old_status, $post ) {
 				$current_user_display_name = __( 'WordPress Scheduler', 'edit-flow' );
 				$current_user_email = '';
 			}
+
+			$old_status_post_obj = get_post_status_object( $old_status );
+			$new_status_post_obj = get_post_status_object( $new_status );
+			$old_status_friendly_name = '';
+			$new_status_friendly_name = '';
+
+			/**
+			 * get_post_status_object will return null for certain statuses (i.e., 'new')
+			 * The mega if/else block below should catch all cases, but just in case, we 
+			 * make sure to at least set $old_status_friendly_name and $new_status_friendly_name
+			 * to an empty string to ensure they're at least set.
+			 * 
+			 * Then, we attempt to set them to a sensible default before we start the
+			 * mega if/else block
+			 */
+			if ( ! is_null( $old_status_post_obj ) ) {
+				$old_status_friendly_name = $old_status_post_obj->label;
+			}
+
+			if ( ! is_null( $new_status_post_obj ) ) {
+				$new_status_friendly_name = $new_status_post_obj->label;
+			}
 
 			// Email subject and first line of body 
 			// Set message subjects according to what action is being taken on the Post	
@@ -625,15 +647,10 @@ function notification_status_change( $new_status, $old_status, $post ) {
 				$subject = sprintf( __( '[%1$s] %2$s Status Changed for "%3$s"', 'edit-flow' ), $blogname, $post_type, $post_title );
 				/* translators: 1: post type, 2: post id, 3. post title, 4. user name, 5. user email */
 				$body .= sprintf( __( 'Status was changed for %1$s #%2$s "%3$s" by %4$s %5$s', 'edit-flow'), $post_type, $post_id, $post_title, $current_user_display_name, $current_user_email ) . "\r\n";
-				$old_status_post_obj = get_post_status_object( $old_status );
-				$old_status_friendly_name = $old_status_post_obj->label;
 			}
 
 			/* translators: 1: date, 2: time, 3: timezone */
 			$body .= sprintf( __( 'This action was taken on %1$s at %2$s %3$s', 'edit-flow' ), date_i18n( get_option( 'date_format' ) ), date_i18n( get_option( 'time_format' ) ), get_option( 'timezone_string' ) ) . "\r\n";
-
-			$new_status_post_obj = get_post_status_object( $new_status );
-			$new_status_friendly_name = $new_status_post_obj->label;
 
 			// Email body
 			$body .= "\r\n";

tests/test-edit-flow-notifications.php

@@ -0,0 +1,95 @@
+<?php
+
+class WP_Test_Edit_Flow_Notifications extends WP_UnitTestCase {
+
+	protected static $admin_user_id;
+	protected static $ef_notifications;
+
+	public static function wpSetUpBeforeClass( $factory ) {
+		global $edit_flow;
+
+		/**
+		 * `install` is hooked to `admin_init` and `init` is hooked to `init`.
+		 * This means when running these tests, you can encounter a situation
+		 * where the custom post type taxonomy has not been loaded into the database
+		 * since the tests don't trigger `admin_init` and the `install` function is where
+		 * the custom post type taxonomy is loaded into the DB. 
+		 * 
+		 * So make sure we do one cycle of `install` followed by `init` to ensure 
+		 * custom post type taxonomy has been loaded.
+		 */
+		$edit_flow->custom_status->install();
+		$edit_flow->custom_status->init();
+
+		self::$admin_user_id = $factory->user->create( array( 'role' => 'administrator' ) );
+
+		self::$ef_notifications = new EF_Notifications();
+		self::$ef_notifications->install();
+		self::$ef_notifications->init();
+	}
+
+	public static function wpTearDownAfterClass() {
+		self::delete_user( self::$admin_user_id );
+		self::$ef_notifications = null;
+	}
+
+	/**
+	 * Test that a notification status change text is accurate when no status
+	 * is provided in wp_insert_post
+	 */
+	function test_send_post_notification_no_status() {
+		global $edit_flow;
+
+		$edit_flow->notifications->module->options->always_notify_admin = 'on';
+
+		$post = array(
+			'post_author' => self::$admin_user_id,
+			'post_content' => rand_str(),
+			'post_title' => rand_str(),
+			'post_date_gmt' => '2016-04-29 12:00:00',
+		);
+
+		wp_insert_post( $post );
+
+		$mailer = tests_retrieve_phpmailer_instance();
+
+		$this->assertTrue( strpos( $mailer->get_sent()->body, 'New => Draft' ) > 0 );
+	}
+
+	/**
+	 * Test that a notification status change text is accurate when no status
+	 * is provided in wp_insert_post when the custom status module is disabled
+	 */
+	function test_send_post_notification_no_status_custom_status_disabled_for_post_type() {
+		global $edit_flow, $typenow;
+
+		/**
+		 * Prevent the registration of custom status to check if notification module will still
+		 * work when custom status module is disabled and custom statuses are not registered
+		 */
+		$typenow = 'post';
+
+		$edit_flow->custom_status->module->options->post_types = array( 'page' );
+		/**
+		 * Initiate a full cycle (install/init) to ensure the core statuses are returned
+		 * instead of custom stautses (since we're disabling the module for this post type)
+		 */
+		$edit_flow->custom_status->install();
+		$edit_flow->custom_status->init();
+
+		$edit_flow->notifications->module->options->always_notify_admin = 'on';
+
+		$post = array(
+			'post_author' => self::$admin_user_id,
+			'post_content' => rand_str(),
+			'post_title' => rand_str(),
+			'post_date_gmt' => '2016-04-29 12:00:00',
+		);
+
+		wp_insert_post( $post );
+
+		$mailer = tests_retrieve_phpmailer_instance();
+
+		$this->assertTrue( strpos( $mailer->get_sent()->body, 'New => Draft' ) > 0 );
+	}
+}