[buddypress-trac] [BuddyPress Trac] #7130: bp_notifications_get_all_notifications_for_user gets all records and can cause memory leak

[buddypress-trac]

#7130: bp_notifications_get_all_notifications_for_user gets all records and can
cause memory leak
-------------------------------------+------------------
 Reporter:  hberberoglu              |       Owner:
     Type:  defect (bug)             |      Status:  new
 Priority:  normal                   |   Milestone:  3.0
Component:  Toolbar & Notifications  |     Version:
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch                |
-------------------------------------+------------------

Comment (by m_uysl):

 Hi,

 @boonebgorges thanks for the separated patch.

 > My impression is that the costly part of
 bp_notifications_get_notifications_for_user() is the formatting, not the
 grouping. Does that seem correct?

 Yeah, seems correct. But, this part:
 {{{#!php
         for ( $i = 0, $count = count( $notifications ); $i < $count; ++$i
 ) {
                 $notification = $notifications[$i];
 $grouped_notifications[$notification->component_name][$notification->component_action][]
 = $notification;
         }
 }}}

 a little annoying and the rest of the patch is almost same with the core.
 (code formatting could be the worst thing for a patch)

 > since we don't pass $args to this function anywhere in BP
 We can use filter instead but the grouped notification messages would be
 still incorrect.


 What if we change the behavior of `bp_notifications_toolbar_menu()`. I've
 checked various social networks and the basic approach is:
 * Show the unread notification count
 * Show the latest `$x` amount notification

 (IMHO) that would make notifications more streamlined ( and totally
 deserves to be an another ticket :) )

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/7130#comment:12>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac