Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Indicators for Sent/Received/Read text messages #1012

Closed
GLLM opened this issue Jun 27, 2018 · 5 comments · Fixed by #4231
Closed

Indicators for Sent/Received/Read text messages #1012

GLLM opened this issue Jun 27, 2018 · 5 comments · Fixed by #4231
Assignees

Comments

@GLLM
Copy link

GLLM commented Jun 27, 2018

Hi NC devs !!

I logged a Feature request (nextcloud/talk-android#219) on Android Talk repo but I was told there's a prerequisite on the server side, hence this new issue :-)

Could the server enable Talk clients to get status indicators for message : Sent/Received/Read ?
The aim is for Clients to be able to get the same features as Signal/Whatsapp have.

Thanks.


Android: nextcloud/talk-android#219

@EdGeraghty
Copy link

I'd like to offer my help in implementing this feature.

A major blocker for my using NC Talk is its lack of vanishing messages, so I started going about that here: https://github.com/EdGeraghty/spreed/tree/expiring-messages and have now reached the part where knowing when messages have been delivered & then read is required.

I'd rather do this in tandem with this ticket than spring a pull request on you, so have there been any discussions to date on how it was planned to be implemented?

I see from the db that there's a table for it:
image

My plan is to add a reference_id column as per the comments table, and then we can show "delivered" messages as those with a NULL timestamp, with the timestamp set through the signalling when read through the client.

This raises the obvious question: how are comments going to be stored in the database in the short-medium term? Right now I see they're stored once in the comments table, with Access Control done elsewhere - if there's a move towards E2EE of the messages, then obviously each message will be stored as its own record... which will further change the logic in message expiry (for the better, I'd say, but it's certainly a change which would require wider discussion).

@nickvergessen
Copy link
Member

I'd like to offer my help in implementing this feature.

😎

A major blocker for my using NC Talk is its lack of vanishing messages, so I started going about that here: https://github.com/EdGeraghty/spreed/tree/expiring-messages and have now reached the part where knowing when messages have been delivered & then read is required.

So just for the record, there is no difference between deliver and read so far. If a device/browser fetches the messages of a conversation they are all "marked read" in terms of setting the last_read_message in oc_talk_participants to the current time.

I'd rather do this in tandem with this ticket than spring a pull request on you, so have there been any discussions to date on how it was planned to be implemented?

My current plan would be that we abuse the fact that you can not write future/past messages and therefor assume the order by time and id are the same. This way when a user marks the chat as read, we take the lowest last_read_message and store it to the conversation and everything with a lower id is "read".
The main issue is that we can't really modify the structure of the comments table in our code, as it is from the Nextcloud server and used by other apps as well. The other issue is performance. When a user reads a chat, we don't really want to have to update 200 chat messages in the database that they are not read.

This raises the obvious question: how are comments going to be stored in the database in the short-medium term? Right now I see they're stored once in the comments table, with Access Control done elsewhere - if there's a move towards E2EE of the messages, then obviously each message will be stored as its own record... which will further change the logic in message expiry (for the better, I'd say, but it's certainly a change which would require wider discussion).

Currently there are no plans to move away from the comments API. Even with E2EE we can use it. The messages would just be encrypted in this table.

We have a Talk development chat for our contributors to have a quick way of discussing/clarifying things, could even do a quick call there. If you want to be added send me an email address to <my github name>@nextcloud.com

@EdGeraghty
Copy link

EdGeraghty commented Jun 15, 2020

Thanks!

So just for the record, there is no difference between deliver and read so far. If a device/browser fetches the messages of a conversation they are all "marked read" in terms of setting the last_read_message in oc_talk_participants to the current time.

I see. Can we use the last_ping as a way to tell when read perhaps?

The main issue is that we can't really modify the structure of the comments table in our code, as it is from the Nextcloud server and used by other apps as well.

Does this include comments_read_markers ? The column I'd use is already in comments so not having it seems a little odd - does this mean I have to make a PR further up the chain?

The other issue is performance. When a user reads a chat, we don't really want to have to update 200 chat messages in the database that they are not read.

I think we can get away with that, as you say, because we can take advantage of the incrementing ID as a proxy for timestamp, matching against the last_read_message. We can then let the database do the heavy lifting rather than hitting it with a load of requests.

Currently there are no plans to move away from the comments API. Even with E2EE we can use it. The messages would just be encrypted in this table.

Sure, but then each message would be stored as (effectively) ONE2ONE rather than per-room broadcast messages, because you need to store different ciphertext per user. This obviously changes the current storage structure of the messages, and so would have a large impact on timed/vanishing messages.

@nickvergessen nickvergessen self-assigned this Jun 22, 2020
@nickvergessen
Copy link
Member

Sure, but then each message would be stored as (effectively) ONE2ONE rather than per-room broadcast messages, because you need to store different ciphertext per user. This obviously changes the current storage structure of the messages, and so would have a large impact on timed/vanishing messages.

And means that user added later to the conversation can not read messages until they all have been re-encrypted for them.

@EdGeraghty
Copy link

Would we want people to be able to read messages from before they were added to the chat, in E2EE? Anyway, this probably needs a separate ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants