musonza / chat

A Laravel chat package. You can use this package to create a chat/messaging Laravel application.
MIT License
1.12k stars 308 forks source link

ReadAll sets status for all participants #210

Open hirbod opened 4 years ago

hirbod commented 4 years ago

Calling Chat::conversation($conversation)->setParticipant($request->user())->readAll(); sets the status of all messages to "seen", but this should not (or at least there should be another method) set this for own messages. I am displaying Chat-Bubbles with 1 and 2 ticks inside the bubble (like WhatsApp). 1 means its saved to the server, 2 means its read by the user.

But if I send the messages (User A) and open the chat after a while (again with USER A), I call the function readAll but this sets also the status to read for my last message, which only should be "read_at", if someone else have seen it.

hirbod commented 4 years ago

Looks like this is only the case when I grab the messages, it works well on the last_message in my inbox (so the read status is correct in the conversations but not in the message-return)

hirbod commented 4 years ago

@musonza any eta on this? Thanks!

hirbod commented 4 years ago

Friendly ping @musonza

hirbod commented 4 years ago

I found out that readAll actually sets the read_at value in the messages response, while inside the database everything is marked correctly (seen for me, but not for the receiver). So read_at should not be set, if not all other participants have seen it (could be an issue on group chats but a no brainer on 1:1)

musonza commented 4 years ago

Ok Thanks, I will take a look at this over the weekend

musonza commented 4 years ago

@Hirbod can you paste a snippet of code you are using for the above?

hirbod commented 4 years ago
    public function getMessages(Request $request, Conversation $conversation)
    {
        Chat::conversation($conversation)->setParticipant($request->user())->readAll();
        $messages = Chat::conversation($conversation)->setParticipant($request->user())->setPaginationParams([
            'perPage' => 30,
            'page' => $request->page ? $request->page : 1,
            'sorting' => "desc",
        ])->getMessages();

        return fractal($messages, $this->messageTransformer)->respond();

        //return response()->json($messages);
    }

And in the response, I check the read_at value to determine, if the message was read. But its already set when I open the chat once again. If I remove readAll, the value is empty and correct but will never update ofc.

hirbod commented 4 years ago

If I check the notification table, everything is right. The read status is also correct when I get all conversations, but the read_at value is wrongly set for the current user requesting all the messages. read_at should be null if the other party didn't read it, but currently is set when I read it calling readAll. (my own messages)

hirbod commented 4 years ago

....

musonza commented 4 years ago

@Hirbod looking at this again, the read_at column only works for the recipient (whether you have seen the message). You are trying to track who has read/seen messages you send?

hirbod commented 4 years ago

If I send a message I want to know if the recipient read it. I am checking for the read_at. But read_at gets a timestamp when I call Chat::conversation($conversation)->setParticipant($request->user())->readAll() (I call this everytime I grab a chat). But read_at only should be filled when the recipient triggered readAll, not when I did. (only his/her messages should receive a read_at, not mine) Vice versa

musonza commented 4 years ago

That's not the use of this column. The readAll() is more like an inbox when you select messages and mark them as read. There is no functionality that exists right now to do this. What would you do for group messages? However, I'm sure you can make it work by writing a query for that.

hirbod commented 4 years ago

readAll sets the messages of a conversation to read. Why would I want to mark my own messages as read? This makes no sense. But this is exactly whats happening right now. I only have 1:1 case right now, but yes, group messages should deliver a read status per message per user (same as whatsapp).

In whatsapp, you can watch which user has seen your message and the ticks turn blue when all users of the group have seen them.

musonza commented 4 years ago

Yes but that functionality in whatsapp is different from what read_at is doing here. It can pretty much be ignored when is_sender is true as it's meaningless for own messages.

hirbod commented 4 years ago

So how can I determine the status using my snippet from above?

musonza commented 4 years ago

You can start by looking at this query https://github.com/musonza/chat/blob/e60fcbdfa1c6f5e08d743142fd8e804bb8ec6cf4/src/Models/Conversation.php#L319-L343 and see how you can have a variation of that