Cannot reproduce Conversation::recipientRemoved & Finder compatibility issue

[Xon]

Affected version

2.2.6

Conversation::recipientRemoved requires the User relationship which is a key'ed FinderCollection to be loaded with all records, except if the conversation was loading with a finder Conversation.User|{$userId} this can result in the recipientRemoved function unexpectedly deleting the conversation.

It looks like the fix is simple;

/** @var \XF\Mvc\Entity\FinderCollection $users */
$users = $this->Users;
$users->populate();
$users->forceRemove($recipient->user_id);

This thankfully can't happen with stock configuration, but add-ons could easily tickle this behaviour without meaning to.

 

[Chris D]

If needed, do you have a more concrete example of what the actual bug is here?

public function recipientRemoved(ConversationRecipient $recipient)
{
   /** @var \XF\Mvc\Entity\FinderCollection $users */
   $users = $this->Users;
   $users->forceRemove($recipient->user_id);

   if (!$users->count())
   {
      $this->delete();
   }
}

The above is the full recipientRemoved method of the ConversationMaster entity. The ConversationMaster entity is deleted if the $users->count() call is 0.

Your proposed fix is to do $users->populate() but the count method in the AbstractCollection class already calls populate prior to counting.

So I'm not seeing a clear case for calling populate separately or exactly what issue this is trying to solve.

 

[Xon]

My add-on code was adding 'Conversation.User|'.\XF::visitor()->user_id to the ConversationUser with clause (as it removed extra queries).

Then when the user kicked themselves or left the conversation, recipientRemoved would see the user list as a list of 0 items and try to delete the conversation.

 

[Chris D]

Then when the user kicked themselves or left the conversation, recipientRemoved would see the user list as a list of 0 items and try to delete the conversation.

The proposed solution of adding $users->populate() wouldn't solve this though because that runs already when we call $users->count() but equally if you're still seeing that behaviour then there must be something else you were running into.