Allow for message threads to be deleted for users other than the current user

[jdgrimes]

Message threads are deleted on a per-user basis, and then only removed from the database completely once all recipients have marked the thread as deleted. Currently message threads can only be deleted for the user who is the current user, because the use of bp_loggedin_user_id() is hard-coded in BP_Messages_Thread::delete(). There is an accompanying todo comment suggesting that this should be fixed. As I just ran into an issue with this in the context of unit testing some code that uses the messages API, I thought I'd work up a patch.

[jdgrimes]

7235.diff​ is a first pass. I've added an optional $user_id parameter to BP_Messages_Thread::delete() and messages_delete_thread(), and to action hooks that they call where I felt that it was appropriate.

While working on this I realized that there could be back-compat issues with any functions hooked to those actions that will be assuming that the messages are being deleted for the logged-in user. I guess the only way to work around that would be to hack the current user when calling those actions, and then reset it afterward. Possibly then we'd also want to deprecate them and introduce new actions that preserve the current user when they are called. I guess this really could be an insurmountable obstacle to ever fixing this without introducing new functions with new actions.

[DJPaul]

Hi @jdgrimes, thanks for the patch -- really good work! 👏

Your point about the risk of change in having these functions no longer rely on the logged-in user is a good one. I am undecided how much I think we should care. A new pair of actions is not going to significantly complicate the function, and I guess we could even move the current actions into /bp-core/deprecated/ and hook them to the new actions (to keep the function neat and tidy). But the current actions have pretty good/logical names.

I think on balance, delete is our lowest level function, and should never have been relying on the global logged-in user at all. Upstream in messages_delete_thread etc is where that should have been handled (and where it is now, in your patch!).

Let's await a second opinion but I think we should probably just commit as-is and not worry about the potential break.

[DJPaul]

Oh, I suppose for bonus points, we should add a @since 2.7.0 line to the functions' docblocks explaining we added a new default parameter. :)

[jdgrimes]

Add @since docs for new parameter

[jdgrimes]

Slightly better @since explanations for new $user_id parameter

[boonebgorges]

Let's await a second opinion but I think we should probably just commit as-is and not worry about the potential break.

I agree with this. The current behavior doesn't make much sense. Let's make sure that the change is documented in a bit more detail in the hook docblock - maybe a separate paragraph in the long description. We might also prepare a bpdevel post for near the end of the dev cycle to explain some breaking changes (this and the unit test factory come to mind).

[boonebgorges]

In 11153:

Accept a $user_id parameter in messages_delete_thread().

Previously, this stack assumed the logged-in user, which made it very
difficult to delete threads for arbitrary users without touching the
database directly.

Props jdgrimes.
Fixes #7235.