[buddypress-trac] [BuddyPress Trac] #8508: Paginate messages and recipients in a thread

[buddypress-trac]

#8508: Paginate messages and recipients in a thread
-------------------------------------------------+-------------------------
 Reporter:  espellcaste                          |       Owner:
                                                 |  espellcaste
     Type:  enhancement                          |      Status:  accepted
 Priority:  high                                 |   Milestone:  10.0.0
Component:  Messages                             |     Version:  1.0
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests dev-        |
  reviewed                                       |
-------------------------------------------------+-------------------------
Changes (by imath):

 * keywords:  has-patch has-unit-tests => has-patch has-unit-tests dev-
     reviewed
 * version:  8.0.0 => 1.0


Comment:

 Thanks a lot for your work about this improvement.

 1) I'm unsure we should paginate the recipients list the way you did it as
 `messages_check_thread_access()` ends up getting all recipients of the
 thread to check the current user can access it. I believe paginating
 recipients using an `array_slice( $recipients, $offset, $length )` of the
 cached list is a better option.
 2) Is this the only step or will there be another one to add default
 values for the new arguments to the `bp_thread_has_messages()` function?
 It's not required (I believe) to pass these arguments, but it can help
 developers to save some time finding these 😉.
 3) Small suggestion, I think the inline comment to explain why the
 messages are not cached would be better if it was phrased the other way
 around as you're checking `! empty()`, eg: `// If 'per_page' is not empty,
 this is a paginated request and messages should not be fetched from the
 cache.`

 Otherwise I confirm unit tests are Ok (thanks for building these 😍) and I
 haven't seen any regression into the BP Nouveau Messages UI.

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