[buddypress-trac] [BuddyPress] #5273: Error sending private messages in 1.9 Beta 2

[buddypress-trac]

#5273: Error sending private messages in 1.9 Beta 2
-----------------------------------------+---------------------------
 Reporter:  henrywright                  |       Owner:  boonebgorges
     Type:  defect (bug)                 |      Status:  reopened
 Priority:  normal                       |   Milestone:  1.9
Component:  Messaging                    |     Version:
 Severity:  normal                       |  Resolution:
 Keywords:  has-patch reporter-feedback  |
-----------------------------------------+---------------------------

Comment (by r-a-y):

 Okay, thanks for explaining that you're attempting to use the
 `'bp_messages_single_new_message_notification'` filter, henrywright.

 We should probably create a new ticket for this, but the problem you state
 is not a regression.

 BP 1.8 also set the `'item_id'` to the `$message->id` (see the
 [http://buddypress.trac.wordpress.org/browser/tags/1.8.1/bp-messages/bp-
 messages-functions.php#L125 source]).

 There are two approaches to address this:

 '''1) Change the notification `'item_id'` to `$message->thread_id`'''

 If we use henrywright's patch so the notification`'item_id'` is set to
 `$message->thread_id`, current plugins that are already using the
 '`item_id'` look up to check for the `$message->id` will no longer work.
 (I think the chances of this happening are pretty slim, but just stating
 the consequences.)

 On the other hand, this would also fix
 `bp_messages_screen_conversation_mark_notifications()` as that is
 referencing the thread ID and not the message ID.

 '''2) Grab the thread ID when formatting notifications'''

 Ideally, when formatting single message notifications, we should be using
 the message thread permalink so we can take advantage of the new mark as
 read feature like you stated, henrywright.

 I've attached an initial patch with this in mind.  I also changed the
 strings a bit, which is kind of a no-no since we're so close to release,
 but if this isn't desired, we can remove the string changes.

 `bp_messages_screen_conversation_mark_notifications()` also works with the
 message ID now, but is a little hacky and I don't like the usage of the
 foreach loop.  `BP_Notification_Notification::update()` (and consequently
 wpdb::update()) is a little limited as we can't pass `item_id` as an
 array, which is why I had to resort to this.

 I'm not a fan of the multiple DB queries in the foreach loop as well as
 when using `new BP_Messages_Message()` to get the thread ID.

 Until I get some feedback from an additional dev, what you could do in the
 meantime is grab the message thread ID by looking up the message in your
 filter override function.  Something like this:

 {{{
 $message = new BP_Messages_Message( $message->id );
 $thread_id = $message->thread_id;
 }}}

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5273#comment:33>
BuddyPress <http://buddypress.org/>
BuddyPress