Paginate messages and recipients in a thread

[espellcaste]

Currently, the BP_Messages_Thread class grabs all messages created for a thread. So if a thread has 1k messages, it tries to grab all of them and caches them, but there is no way to paginate those messages.

I suggest we refactor that part of the class to not cache the messages but to set a default number of messages, like 10, and add a parameter to paginate the rest of the messages.

[imath]

I agree it would be a nice improvement 👍

[imath]

We won't have time for 9.0.0 unfortunately, so let's work on it early for BP 10.0.0 :)

[imath]

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.

[espellcaste]

@imath

1 - I reviewed other query classes and it seems almost all of them use bp_core_get_incremented_cache even when querying a paginated page. So your suggestion makes sense. I'll work on a update.

Just a note that the example you provided would get the recipients from cache instead of directly querying the database.

2 - Good catch. I'll update those with the new parameters.

3 - I'll update as suggested.

Tks for the feedback. Very useful! :)

[espellcaste]

@imath Could you check again?

[imath]

Sure, I will do asap 👌

[imath]

@espellcaste 8508-2.diff looks good to me. After applying your patch, I just added an optional suggestion to save a foreach loop (see ​8508.2.suggestion.patch). Thanks again for this improvement.

[espellcaste]

Thank you! Will proceed with the commit (including your patch). :)

[espellcaste]

In 13102:

Adding support to the offset pagination to Messages and Recipients in a thread.

Previously, the BP_Messages_Thread query class would grab ALL messages created for a thread and ALL its recipients. This commit introduces support to the offset pagination
allowing one to paginate both messages and recipients using parameters with a specific limit and offset.

Props imath
Fixes #8508

[espellcaste]

Related: #8597