Changeset 9744

Timestamp:

04/12/2015 07:42:38 PM (10 years ago)

Author:

boonebgorges

Message:

Ensure that corresponding notifications are deleted when a message thread is deleted.

This means deleting the notifications for each message in the thread, for each
recipient on the thread. Previously, only a subset of these were being deleted.

This changeset removes a test that used a dummy thread_id, a testing
technique that doesn't work with the new looping technique used for
notification deletion. The new unit test is a proper replacement.

Props pareshradadiya for an initial patch.
Fixes #6329.

Location:

trunk

Files:

• src/bp-messages/bp-messages-notifications.php (modified) (1 diff)
• tests/phpunit/testcases/messages/notifications.php (modified) (2 diffs)

trunk/src/bp-messages/bp-messages-notifications.php

@@ -298 +298 @@
  * @since BuddyPress (2.0.0)
  *
- * @param int $message_id ID of the message.
- */
-function bp_messages_message_delete_notifications( $message_id = 0 ) {
-    if ( bp_is_active( 'notifications' ) && ! empty( $message_id ) ) {
-        bp_notifications_delete_notifications_by_item_id( bp_loggedin_user_id(), (int) $message_id, buddypress()->messages->id, 'new_message' );
-    }
-}
-add_action( 'messages_thread_deleted_thread', 'bp_messages_message_delete_notifications', 10, 1 );
+ * @param int   $message_id  ID of the thread.
+ * @param array $message_ids IDs of the messages.
+ */
+function bp_messages_message_delete_notifications( $thread_id, $message_ids ) {
+    if ( ! bp_is_active( 'notifications' ) ) {
+        return;
+    }
+
+    // For each recipient, delete notifications corresponding to each message.
+    $thread = new BP_Messages_Thread( $thread_id );
+    foreach ( $thread->get_recipients() as $recipient ) {
+        foreach ( $message_ids as $message_id ) {
+            bp_notifications_delete_notifications_by_item_id( $recipient->user_id, (int) $message_id, buddypress()->messages->id, 'new_message' );
+        }
+    }
+}
+add_action( 'bp_messages_thread_after_delete', 'bp_messages_message_delete_notifications', 10, 2 );

trunk/tests/phpunit/testcases/messages/notifications.php

@@ -13 +13 @@
 
         $this->filter_fired = '';
-    }
-    /**
-     * @group bp_messages_message_delete_notifications
-     */
-    public function test_bp_messages_message_delete_notifications() {
-        $current_user = get_current_user_id();
-        $u = $this->factory->user->create();
-        $this->set_current_user( $u );
-
-        // Dummy thread ID
-        $t = 12;
-
-        // Admin
-        $n = $this->factory->notification->create( array(
-            'component_name' => 'messages',
-            'user_id' => $u,
-            'item_id' => $t,
-            'component_action' => 'new_message',
-        ) );
-
-        $notifications = BP_Notifications_Notification::get( array(
-            'user_id' => $u,
-        ) );
-
-        // Double check it's there
-        $this->assertEquals( array( $n ), wp_list_pluck( $notifications, 'id' ) );
-
-        // fire the hook
-        do_action( 'messages_thread_deleted_thread', $t );
-
-        $notifications = BP_Notifications_Notification::get( array(
-            'user_id' => $u,
-        ) );
-
-        $this->assertEmpty( $notifications );
-
-        $this->set_current_user( $current_user );
     }
 
@@ -148 +111 @@
     }
 
+    /**
+     * @ticket BP6329
+     */
+    public function test_messages_notifications_should_be_deleted_when_corresponding_message_is_deleted() {
+        if ( ! bp_is_active( 'messages' ) ) {
+            $this->markTestSkipped( __METHOD__ . ' requires the Messages component.' );
+        }
+
+        $u1 = $this->factory->user->create();
+        $u2 = $this->factory->user->create();
+
+        $t1 = messages_new_message( array(
+            'sender_id'  => $u1,
+            'recipients' => array( $u2 ),
+            'subject'    => 'A new message',
+            'content'    => 'Hey there!',
+        ) );
+
+        // Verify that a notification has been created for the message.
+        $n1 = BP_Notifications_Notification::get( array(
+            'component' => 'messages',
+            'user_id' => $u2,
+        ) );
+        $this->assertNotEmpty( $n1 );
+
+        $this->assertTrue( messages_delete_thread( $t1 ) );
+
+        $n2 = BP_Notifications_Notification::get( array(
+            'component' => 'messages',
+            'user_id' => $u2,
+        ) );
+        $this->assertSame( array(), $n2 );
+    }
+
     public function notification_filter_callback( $value ) {
         $this->filter_fired = current_filter();