Ticket #8426: 8426.10.patch

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

@@ -247 +247 @@
         $message_ids = array_intersect( $unread_message_ids, wp_list_pluck( $thread_template->thread->messages, 'id' ) );
 
         // Mark each notification for each PM message as read.
-        foreach ( $message_ids as $message_id ) {
-                bp_notifications_mark_notifications_by_item_id( bp_loggedin_user_id(), (int) $message_id, buddypress()->messages->id, 'new_message' );
-        }
+        bp_notifications_mark_notifications_by_item_ids( bp_loggedin_user_id(), $message_ids, 'messages', 'new_message', false );
 }
 add_action( 'thread_loop_start', 'bp_messages_screen_conversation_mark_notifications', 10 );
 
@@ -261 +259 @@
  * @since 3.0.0
  *
  * @param int $thread_id ID of the thread being marked as read.
+ * @param int $user_id   ID of the user thread will be marked as unread.
+ * @param int $num_rows  The number of affected rows by the "mark read" update query.
  */
-function bp_messages_mark_notification_on_mark_thread( $thread_id ) {
+function bp_messages_mark_notification_on_mark_thread( $thread_id, $user_id = 0, $num_rows = 0 ) {
+        if ( ! $num_rows ) {
+                return;
+        }
+
         $thread_messages = BP_Messages_Thread::get_messages( $thread_id );
+        $message_ids     = wp_list_pluck( $thread_messages, 'id' );
 
-        foreach ( $thread_messages as $thread_message ) {
-                bp_notifications_mark_notifications_by_item_id( bp_loggedin_user_id(), $thread_message->id, buddypress()->messages->id, 'new_message' );
-        }
+        bp_notifications_mark_notifications_by_item_ids( $user_id, $message_ids, 'messages', 'new_message', false );
 }
-add_action( 'messages_thread_mark_as_read', 'bp_messages_mark_notification_on_mark_thread' );
+add_action( 'messages_thread_mark_as_read', 'bp_messages_mark_notification_on_mark_thread', 10, 3 );
 
 /**
  * When a message is deleted, delete corresponding notifications.
@@ -283 +286 @@
         // 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 ) {
-                        if ( ! empty( $recipient->user_id ) ) {
-                                bp_notifications_delete_notifications_by_item_id(
-                                        $recipient->user_id,
-                                        (int) $message_id,
-                                        buddypress()->messages->id,
-                                        'new_message'
-                                );
-                        }
+                if ( ! isset( $recipient->user_id ) || ! $recipient->user_id ) {
+                        continue;
                 }
+
+                bp_notifications_delete_notifications_by_item_ids( $recipient->user_id, $message_ids, buddypress()->messages->id, 'new_message' );
         }
 }
 add_action( 'bp_messages_thread_after_delete', 'bp_messages_message_delete_notifications', 10, 2 );

src/bp-messages/classes/class-bp-messages-thread.php

@@ -766 +766 @@
                                 bp_loggedin_user_id();
                 }
 
-                $bp     = buddypress();
-                $retval = $wpdb->query( $wpdb->prepare( "UPDATE {$bp->messages->table_name_recipients} SET unread_count = 0 WHERE user_id = %d AND thread_id = %d", $user_id, $thread_id ) );
+                $bp       = buddypress();
+                $num_rows = $wpdb->query( $wpdb->prepare( "UPDATE {$bp->messages->table_name_recipients} SET unread_count = 0 WHERE user_id = %d AND thread_id = %d", $user_id, $thread_id ) );
 
                 wp_cache_delete( 'thread_recipients_' . $thread_id, 'bp_messages' );
                 wp_cache_delete( $user_id, 'bp_messages_unread_count' );
@@ -777 +777 @@
                  *
                  * @since 2.8.0
                  * @since 9.0.0 Added the `user_id` parameter.
+                 * @since 10.0.0 Added the `$num_rows` parameter.
                  *
                  * @param int $thread_id The message thread ID.
                  * @param int $user_id   The user the thread will be marked as read.
+                 * @param bool|int $num_rows    Number of threads marked as unread or false on error.
                  */
-                do_action( 'messages_thread_mark_as_read', $thread_id, $user_id );
+                do_action( 'messages_thread_mark_as_read', $thread_id, $user_id, $num_rows );
 
-                return $retval;
+                return $num_rows;
         }
 
         /**
@@ -810 +812 @@
                                 bp_loggedin_user_id();
                 }
 
-                $bp     = buddypress();
-                $retval = $wpdb->query( $wpdb->prepare( "UPDATE {$bp->messages->table_name_recipients} SET unread_count = 1 WHERE user_id = %d AND thread_id = %d", $user_id, $thread_id ) );
+                $bp       = buddypress();
+                $num_rows = $wpdb->query( $wpdb->prepare( "UPDATE {$bp->messages->table_name_recipients} SET unread_count = 1 WHERE user_id = %d AND thread_id = %d", $user_id, $thread_id ) );
 
                 wp_cache_delete( 'thread_recipients_' . $thread_id, 'bp_messages' );
                 wp_cache_delete( $user_id, 'bp_messages_unread_count' );
@@ -821 +823 @@
                  *
                  * @since 2.8.0
                  * @since 9.0.0  Added the `$user_id` parameter.
-                 * @since 10.0.0 Added the `$retval` parameter.
+                 * @since 10.0.0 Added the `$num_rows` parameter.
                  *
                  * @param int      $thread_id The message thread ID.
                  * @param int      $user_id   The user the thread will be marked as unread.
-                 * @param bool|int $retval     =Number of threads marked as unread or false on error.
+                 * @param bool|int $num_rows  Number of threads marked as unread or false on error.
                  */
-                do_action( 'messages_thread_mark_as_unread', $thread_id, $user_id, $retval );
+                do_action( 'messages_thread_mark_as_unread', $thread_id, $user_id, $num_rows );
 
-                return $retval;
+                return $num_rows;
         }
 
         /**

src/bp-notifications/actions/bulk-manage.php

@@ -42 +42 @@
         // Delete, mark as read or unread depending on the user 'action'.
         switch ( $action ) {
                 case 'delete':
-                        foreach ( $notifications as $notification ) {
-                                bp_notifications_delete_notification( $notification );
-                        }
+                        bp_notifications_delete_notifications_by_ids( $notifications );
                         bp_core_add_message( __( 'Notifications deleted.', 'buddypress' ) );
                         break;
 
                 case 'read':
-                        foreach ( $notifications as $notification ) {
-                                bp_notifications_mark_notification( $notification, false );
-                        }
+                        bp_notifications_mark_notifications_by_ids( $notifications, false );
                         bp_core_add_message( __( 'Notifications marked as read', 'buddypress' ) );
                         break;
 
                 case 'unread':
-                        foreach ( $notifications as $notification ) {
-                                bp_notifications_mark_notification( $notification, true );
-                        }
+                        bp_notifications_mark_notifications_by_ids( $notifications, true );
                         bp_core_add_message( __( 'Notifications marked as unread.', 'buddypress' ) );
                         break;
         }

src/bp-notifications/bp-notifications-functions.php

@@ -384 +384 @@
         ) );
 }
 
+/**
+ * Delete notifications by notification ids.
+ *
+ * @since 10.0.0
+ *
+ * @param  array     $ids              IDs of the associated notifications.
+ * @param  string    $component_name   Name of the associated component.
+ * @param  string    $component_action Name of the associated action.
+ * @return int|false                   The number of rows updated. False on error.
+ */
+function bp_notifications_delete_notifications_by_ids( $ids ) {
+        return BP_Notifications_Notification::delete_by_id_list( 'id', $ids );
+}
+
+/**
+ * Delete notifications by item ids and user.
+ *
+ * @since 10.0.0
+ *
+ * @param  int       $user_id          ID of the user whose notifications are being deleted.
+ * @param  array     $item_ids         ID of the associated item.
+ * @param  string    $component_name   Name of the associated component.
+ * @param  string    $component_action Name of the associated action.
+ * @return int|false                   The number of rows updated. False on error.
+ */
+function bp_notifications_delete_notifications_by_item_ids( $user_id, $item_ids, $component_name, $component_action ) {
+        return BP_Notifications_Notification::delete_by_id_list(
+                'item_id',
+                $item_ids,
+                array(
+                        'user_id'          => $user_id,
+                        'component_name'   => $component_name,
+                        'component_action' => $component_action
+                )
+        );
+}
+
 /**
  * Delete all notifications by type.
  *
@@ -584 +621 @@
         );
 }
 
+/**
+ * Mark notifications read/unread by item ids and user.
+ *
+ * @since 10.0.0
+ *
+ * @param  int       $user_id          ID of the user whose notifications are being deleted.
+ * @param  array     $item_ids         ID of the associated item.
+ * @param  string    $component_name   Name of the associated component.
+ * @param  string    $component_action Name of the associated action.
+ * @param  int|false $is_new           0 for read, 1 for unread.
+ * @return int|false                   The number of rows updated. False on error.
+ */
+function bp_notifications_mark_notifications_by_item_ids( $user_id, $item_ids, $component_name, $component_action, $is_new = false ) {
+        return BP_Notifications_Notification::update_id_list(
+                'item_id',
+                $item_ids,
+                array(
+                        'is_new' => $is_new,
+                ),
+                array(
+                        'user_id'          => $user_id,
+                        'component_name'   => $component_name,
+                        'component_action' => $component_action
+                )
+        );
+}
+
+/**
+ * Mark notifications read/unread by notification ids.
+ *
+ * @since 10.0.0
+ *
+ * @param  array     $ids     IDs of the associated notification items.
+ * @param  int|false $is_new  0 for read, 1 for unread.
+ * @return int|false          The number of rows updated. False on error.
+ */
+function bp_notifications_mark_notifications_by_ids( $ids, $is_new = false ) {
+        return BP_Notifications_Notification::update_id_list(
+                'id',
+                $ids,
+                array(
+                        'is_new' => $is_new,
+                )
+        );
+}
+
 /** Helpers *******************************************************************/
 
 /**

src/bp-notifications/classes/class-bp-notifications-notification.php

@@ -910 +910 @@
                 );
         }
 
+        /**
+         * Update notifications using a list of ids/items_ids.
+         *
+         * @since 10.0.0
+         *
+         * @param string $field The name of the db field of the items to update.
+         *                      Possible values are `id` or `item_id`.
+         * @param int[]  $items The list of items to update.
+         * @param array  $data  Array of notification data to update, passed to
+         *                      {@link wpdb::update()}. Accepts any property of a
+         *                      BP_Notification_Notification object.
+         * @param array  $where The WHERE params as passed to wpdb::update().
+         *                      Typically consists of array( 'ID' => $id ) to specify the ID
+         *                      of the item being updated. See {@link wpdb::update()}.
+         * @return int|false    The number of rows updated. False on error.
+         */
+        public static function update_id_list( $field, $items = array(), $data = array(), $where = array() ) {
+                global $wpdb;
+                $bp = buddypress();
+
+                $supported_fields = array( 'id', 'item_id' );
+
+                if ( false === in_array( $field, $supported_fields, true ) ) {
+                        return false;
+                }
+
+                if ( ! is_array( $items ) || ! is_array( $data ) || ! is_array( $where ) ) {
+                        return false;
+                }
+
+                $update_args = self::get_query_clauses( $data );
+                $where_args  = self::get_query_clauses( $where );
+
+                $fields     = array();
+                $conditions = array();
+                $values     = array();
+
+                $_items       = implode( ',', wp_parse_id_list( $items ) );
+                $conditions[] = "{$field} IN ({$_items})";
+
+                foreach ( $update_args['data'] as $field => $value ) {
+                        $index  = array_search( $field, array_keys( $update_args['data'] ) );
+                        $format = $update_args['format'][ $index ];
+
+                        $fields[] = "{$field} = {$format}";
+                        $values[] = $value;
+                }
+
+                foreach ( $where_args['data'] as $field => $value ) {
+                        $index  = array_search( $field, array_keys( $where_args['data'] ) );
+                        $format = $where_args['format'][ $index ];
+
+                        $conditions[] = "{$field} = {$format}";
+                        $values[]     = $value;
+                }
+
+                $fields     = implode( ', ', $fields );
+                $conditions = implode( ' AND ', $conditions );
+
+                /** This action is documented in bp-notifications/classes/class-bp-notifications-notification.php */
+                do_action( 'bp_notification_before_update', $update_args, $where_args );
+
+                return $wpdb->query( $wpdb->prepare( "UPDATE {$bp->notifications->table_name} SET {$fields} WHERE {$conditions}", $values ) );
+        }
+
         /**
          * Delete notifications.
          *
@@ -940 +1005 @@
                 return self::_delete( $where['data'], $where['format'] );
         }
 
+        /**
+         * Delete notifications using a list of ids/items_ids.
+         *
+         * @since 10.0.0
+         *
+         * @param string $field The name of the db field of the items to delete.
+         *                      Possible values are `id` or `item_id`.
+         * @param int[]  $items The list of items to delete.
+         * @param array  $args  The WHERE params as passed to wpdb::delete().
+         *                      Typically consists of array( 'ID' => $id ) to specify the ID
+         *                      of the item being deleted. See {@link wpdb::delete()}.
+         * @return int|false    The number of rows deleted. False on error.
+         */
+        public static function delete_by_id_list( $field, $items = array(), $args = array() ) {
+                global $wpdb;
+                $bp = buddypress();
+
+                $supported_fields = array( 'id', 'item_id' );
+
+                if ( false === in_array( $field, $supported_fields, true ) ) {
+                        return false;
+                }
+
+                if ( ! is_array( $items ) || ! is_array( $args ) ) {
+                        return false;
+                }
+
+                $where = self::get_query_clauses( $args );
+
+                $conditions = array();
+                $values     = array();
+
+                $_items       = implode( ',', wp_parse_id_list( $items ) );
+                $conditions[] = "{$field} IN ({$_items})";
+
+                foreach ( $where['data'] as $field => $value ) {
+                        $index  = array_search( $field, array_keys( $where['data'] ) );
+                        $format = $where['format'][ $index ];
+
+                        $conditions[] = "{$field} = {$format}";
+                        $values[]     = $value;
+                }
+
+                $conditions = implode( ' AND ', $conditions );
+
+                /** This action is documented in bp-notifications/classes/class-bp-notifications-notification.php */
+                do_action( 'bp_notification_before_delete', $args );
+
+                if ( ! $values ) {
+                        return $wpdb->query( "DELETE FROM {$bp->notifications->table_name} WHERE {$conditions}" );
+                }
+
+                return $wpdb->query( $wpdb->prepare( "DELETE FROM {$bp->notifications->table_name} WHERE {$conditions}", $values ) );
+        }
+
         /** Convenience methods ***************************************************/
 
         /**

tests/phpunit/testcases/messages/notifications.php

@@ -11 +11 @@
         public function setUp() {
                 parent::setUp();
 
+                $this->reset_user_id = get_current_user_id();
+
                 $this->filter_fired = '';
         }
 
+        public function tearDown() {
+                parent::tearDown();
+
+                $this->set_current_user( $this->reset_user_id );
+        }
+
         /**
          * @group messages_format_notifications
          */
@@ -40 +48 @@
 
         /**
          * @group messages_format_notifications
-         * @group imath
          */
         public function test_friends_format_notifications_bp_messages_single_new_message_notification_nonstring_filter() {
                 // Dummy thread ID
@@ -145 +152 @@
                 $this->assertSame( array(), $n2 );
         }
 
+        /**
+         * @ticket BP8426
+         */
+        public function test_bp_messages_mark_notification_on_mark_thread() {
+                $u1 = self::factory()->user->create();
+                $u2 = self::factory()->user->create();
+                $m1 = self::factory()->message->create_and_get( array(
+                        'sender_id'  => $u1,
+                        'recipients' => array( $u2 ),
+                        'subject'    => 'Foo',
+                ) );
+
+                self::factory()->message->create_many(
+                        9,
+                        array(
+                                'thread_id' => $m1->thread_id,
+                                'sender_id' => $u2,
+                                'recipients' => array( $u1 ),
+                                'subject' => 'Bar',
+                        )
+                );
+
+                $unreadn = wp_list_pluck(
+                        BP_Notifications_Notification::get(
+                                array(
+                                        'user_id'           => $u1,
+                                        'component_name'    => buddypress()->messages->id,
+                                        'component_action'  => 'new_message',
+                                        'is_new'            => 1,
+                                )
+                        ),
+                        'user_id',
+                        'id'
+                );
+
+                $this->set_current_user( $u1 );
+
+                // Mark a thread read.
+                bp_messages_mark_notification_on_mark_thread( $m1->thread_id, $u1, count( $unreadn ) );
+
+                $readn = wp_list_pluck(
+                        BP_Notifications_Notification::get(
+                                array(
+                                        'user_id'           => $u1,
+                                        'component_name'    => buddypress()->messages->id,
+                                        'component_action'  => 'new_message',
+                                        'is_new'            => 0,
+                                )
+                        ),
+                        'user_id',
+                        'id'
+                );
+
+                $this->assertSame( $unreadn, $readn );
+        }
+
+        /**
+         * @ticket BP8426
+         * @group message_delete_notifications
+         */
+        public function test_bp_messages_message_delete_notifications() {
+                $u1 = self::factory()->user->create();
+                $u2 = self::factory()->user->create();
+                $m1 = self::factory()->message->create_and_get( array(
+                        'sender_id'  => $u1,
+                        'recipients' => array( $u2 ),
+                        'subject'    => 'Foo',
+                ) );
+
+                $message_ids = self::factory()->message->create_many(
+                        9,
+                        array(
+                                'thread_id' => $m1->thread_id,
+                                'sender_id' => $u2,
+                                'recipients' => array( $u1 ),
+                                'subject' => 'Bar',
+                        )
+                );
+
+                $message_ids = wp_list_pluck(
+                        BP_Notifications_Notification::get(
+                                array(
+                                        'user_id'           => $u1,
+                                        'component_name'    => buddypress()->messages->id,
+                                        'component_action'  => 'new_message',
+                                        'is_new'            => 1,
+                                )
+                        ),
+                        'item_id'
+                );
+
+                $test = bp_messages_message_delete_notifications( $m1->thread_id, $message_ids );
+
+                $deleted = BP_Notifications_Notification::get(
+                        array(
+                                'user_id'           => $u1,
+                                'component_name'    => buddypress()->messages->id,
+                                'component_action'  => 'new_message',
+                                'is_new'            => 1,
+                        )
+                );
+
+                $this->assertEmpty( $deleted );
+        }
+
         public function notification_filter_callback( $value ) {
                 $this->filter_fired = current_filter();
                 return $value;

tests/phpunit/testcases/notifications/functions.php

@@ -541 +541 @@
                 $found = bp_notifications_get_notifications_for_user( $u, 'object' );
                 $this->assertEqualSets( [ $n1 ], wp_list_pluck( $found, 'id' ) );
         }
+
+        /**
+         * @ticket BP8426
+         */
+        public function test_bp_notifications_mark_notifications_by_ids() {
+                $u = self::factory()->user->create();
+
+                $n = self::factory()->notification->create(
+                        array(
+                                'component_name'    => 'barfoo',
+                                'component_action'  => 'new_bar',
+                                'item_id'           => 98,
+                                'user_id'           => $u,
+                        )
+                );
+
+                for ( $i = 101; $i < 111; ++$i ) {
+                        self::factory()->notification->create(
+                                array(
+                                        'component_name'    => 'foobar',
+                                        'component_action'  => 'new_foo',
+                                        'item_id'           => $i,
+                                        'user_id'           => $u,
+                                )
+                        );
+                }
+
+                $unread = wp_list_pluck(
+                        BP_Notifications_Notification::get(
+                                array(
+                                        'user_id'           => $u,
+                                        'component_name'    => 'foobar',
+                                        'component_action'  => 'new_foo',
+                                        'is_new'            => 1,
+                                )
+                        ),
+                        'id'
+                );
+
+                bp_notifications_mark_notifications_by_ids( $unread );
+
+                $read = wp_list_pluck(
+                        BP_Notifications_Notification::get(
+                                array(
+                                        'user_id'           => $u,
+                                        'component_name'    => 'foobar',
+                                        'component_action'  => 'new_foo',
+                                        'is_new'            => 0,
+                                )
+                        ),
+                        'id'
+                );
+
+                $n_get = BP_Notifications_Notification::get(
+                        array(
+                                'id' => $n,
+                                'component_name'    => 'barfoo',
+                                'component_action'  => 'new_bar',
+                        )
+                );
+
+                $n_obj = reset( $n_get );
+
+                $this->assertEquals( $unread, $read );
+                $this->assertEquals( $n, $n_obj->id );
+                $this->assertTrue( 1 === (int) $n_obj->is_new );
+        }
+
+        /**
+         * @ticket BP8426
+         * @group delete_notifications_by_ids
+         */
+        public function test_bp_notifications_delete_notifications_by_ids() {
+                $u = self::factory()->user->create();
+
+                $n = self::factory()->notification->create(
+                        array(
+                                'component_name'    => 'barfoo',
+                                'component_action'  => 'new_bar',
+                                'item_id'           => 98,
+                                'user_id'           => $u,
+                        )
+                );
+
+                for ( $i = 101; $i < 111; ++$i ) {
+                        self::factory()->notification->create(
+                                array(
+                                        'component_name'    => 'foobar',
+                                        'component_action'  => 'new_foo',
+                                        'item_id'           => $i,
+                                        'user_id'           => $u,
+                                )
+                        );
+                }
+
+                $unread = wp_list_pluck(
+                        BP_Notifications_Notification::get(
+                                array(
+                                        'user_id'           => $u,
+                                        'component_name'    => 'foobar',
+                                        'component_action'  => 'new_foo',
+                                        'is_new'            => 1,
+                                )
+                        ),
+                        'id'
+                );
+
+                bp_notifications_delete_notifications_by_ids( $unread );
+
+                $deleted = wp_list_pluck(
+                        BP_Notifications_Notification::get(
+                                array(
+                                        'user_id'           => $u,
+                                        'component_name'    => 'foobar',
+                                        'component_action'  => 'new_foo',
+                                        'is_new'            => 1,
+                                )
+                        ),
+                        'id'
+                );
+
+                $n_get = BP_Notifications_Notification::get(
+                        array(
+                                'id' => $n,
+                                'component_name'    => 'barfoo',
+                                'component_action'  => 'new_bar',
+                        )
+                );
+
+                $n_obj = reset( $n_get );
+
+                $this->assertEmpty( $deleted );
+                $this->assertEquals( $n, $n_obj->id );
+                $this->assertTrue( 1 === (int) $n_obj->is_new );
+        }
 }