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.

Sign up for GitHub

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

Jump to bottom

Make it possible to mark reactions as "seen" #6210

Open
iequidoo opened this issue Nov 15, 2024 · 0 comments · May be fixed by #6213
Open

Make it possible to mark reactions as "seen" #6210

iequidoo opened this issue Nov 15, 2024 · 0 comments · May be fixed by #6213
Assignees
@iequidoo

Comments

@iequidoo
Copy link
Collaborator

iequidoo commented Nov 15, 2024 •
edited
Loading

There is deltachat/deltachat-android#3377 which adds notifications for reactions. But it's blocked because when the user receives a reaction notification on multiple devices and clicks "Mark read" on one device (or, equivalently, enters the chat), then the notification won't be removed from the other devices.
This should be fixed by remembering the rfc724_mid of reaction emails and marking them as seen on the server just as normal messages. Then, the second device can notice that they were marked as seen and send a MsgsNoticed event. I started implementing this in hoc/reaction-notifications-2, but got distracted (I didn't get very far, just half an hour or so, and I don't even know whether the approach I started with was the right one).

I've taken a look at this, but for me it seems simpler to add reactions as hidden=1 messages to the msgs table as well, then the existing mechanism of "marking seen" works for them. Also this allows to have Param::OverrideSenderDisplayname for reactions which may be useful for bridge bots, that was mentioned in the closed #5184. Currently reactions just go to the trash chat. Not sure if this is actually simpler because many changes to the tests may be needed. CC @Hocuri @r10s @hpk42 @link2xt

EDIT: So the suggestion is to mark all hidden messages as seen when the chat is noticed. Currently there are no hidden incoming messages, so making reactions hidden messages doesn't intersect with anything.

EDIT: At least no so many Rust tests are failing with this approach:

        FAIL [   0.989s] deltachat reaction::tests::test_partial_download_and_reaction
        FAIL [   0.828s] deltachat reaction::tests::test_reaction_status_multidevice
        FAIL [   0.677s] deltachat reaction::tests::test_reaction_summary
        FAIL [   2.118s] deltachat reaction::tests::test_send_reaction
        FAIL [   2.067s] deltachat reaction::tests::test_send_reaction_multidevice

EDIT: The reason is that they check that reactions go to the trash chat, so the fix is trivial.
@iequidoo iequidoo self-assigned this Nov 15, 2024
@iequidoo iequidoo mentioned this issue Nov 15, 2024
Merged
iequidoo added a commit that referenced this issue Nov 15, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
a089199 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, moreover there are no `msgs` rows for reactions in the core, so
the UIs just call `marknoticed_chat()` in this case. We don't want to introduce reactions message
ids to the UIs (at least currently), but let's make reactions usual messages, just hidden, so that
the existing `\Seen` flag synchronisation mechanism works for them, and mark all reactions in the
chat as seen in marknoticed_chat().
@iequidoo iequidoo linked a pull request Nov 15, 2024 that will close this issue
Open
iequidoo added a commit that referenced this issue Nov 18, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
b71c681 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, moreover there are no `msgs` rows for reactions in the core, so
the UIs just call `marknoticed_chat()` in this case. We don't want to introduce reactions message
ids to the UIs (at least currently), but let's make received reactions usual messages, just hidden,
so that the existing `\Seen` flag synchronisation mechanism works for them, and mark all incoming
hidden messages in the chat as seen in `marknoticed_chat()`.

It's interesting that sent out reactions are already hidden messages, so this change mostly just
unifies things.
iequidoo added a commit that referenced this issue Nov 22, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
eabf313 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, moreover there are no `msgs` rows for reactions in the core, so
the UIs just call `marknoticed_chat()` in this case. We don't want to introduce reactions message
ids to the UIs (at least currently), but let's make received reactions usual messages, just hidden,
so that the existing `\Seen` flag synchronisation mechanism works for them, and mark all incoming
hidden messages in the chat as seen in `marknoticed_chat()`.

It's interesting that sent out reactions are already hidden messages, so this change mostly just
unifies things.
iequidoo added a commit that referenced this issue Nov 22, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
de2b5a1 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, moreover there are no `msgs` rows for reactions in the core, so
the UIs just call `marknoticed_chat()` in this case. We don't want to introduce reactions message
ids to the UIs (at least currently), but let's make received reactions usual messages, just hidden,
so that the existing `\Seen` flag synchronisation mechanism works for them, and mark all incoming
hidden messages in the chat as seen in `marknoticed_chat()`.

It's interesting that sent out reactions are already hidden messages, so this change mostly just
unifies things.
iequidoo added a commit that referenced this issue Nov 22, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
d9482dc 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, moreover there are no `msgs` rows for reactions in the core, so
the UIs just call `marknoticed_chat()` in this case. We don't want to introduce reactions message
ids to the UIs (at least currently), but let's make received reactions usual messages, just hidden,
so that the existing `\Seen` flag synchronisation mechanism works for them, and mark all incoming
hidden messages in the chat as seen in `marknoticed_chat()`.

It's interesting that sent out reactions are already hidden messages, so this change mostly just
unifies things.
iequidoo added a commit that referenced this issue Nov 26, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
cdc0326 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, moreover there are no `msgs` rows for incoming reactions in the
core, so the UIs just call `marknoticed_chat()` in this case. We don't want to introduce reactions
message ids to the UIs (at least currently), but let's make received reactions usual messages, just
hidden and `InFresh`, so that the existing `\Seen` flag synchronisation mechanism works for them,
and mark all incoming hidden messages in the chat as seen in `marknoticed_chat()`.

It's interesting that sent out reactions are already hidden messages, so this change mostly just
unifies things.
iequidoo added a commit that referenced this issue Nov 26, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
fb1fbe4 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, moreover there are no `msgs` rows for incoming reactions in the
core, so the UIs just call `marknoticed_chat()` in this case. We don't want to introduce reactions
message ids to the UIs (at least currently), but let's make received reactions usual messages, just
hidden and `InFresh`, so that the existing `\Seen` flag synchronisation mechanism works for them,
and mark all incoming hidden messages in the chat as seen in `marknoticed_chat()`.

It's interesting that sent out reactions are already hidden messages, so this change mostly just
unifies things.
iequidoo added a commit that referenced this issue Nov 28, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
aaa0b58 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, moreover there are no `msgs` rows for incoming reactions in the
core, so the UIs just call `marknoticed_chat()` in this case. We don't want to introduce reactions
message ids to the UIs (at least currently), but let's make received reactions usual messages, just
hidden and `InFresh`, so that the existing `\Seen` flag synchronisation mechanism works for them,
and mark the last fresh hidden incoming message in the chat as seen in `marknoticed_chat()` to
trigger emitting `MsgsNoticed` on other devices.

There's a problem though that another device may have more reactions received and not yet seen
notifications are removed from it when handling `MsgsNoticed`, but the same problem already exists
for "usual" messages, so let's not solve it for now.

It's interesting that sent out reactions are already hidden messages, so this change mostly just
unifies things.
iequidoo added a commit that referenced this issue Nov 28, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
18eda00 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, moreover there are no `msgs` rows for incoming reactions in the
core, so the UIs just call `marknoticed_chat()` in this case. We don't want to introduce reactions
message ids to the UIs (at least currently), but let's make received reactions usual messages, just
hidden and `InFresh`, so that the existing `\Seen` flag synchronisation mechanism works for them,
and mark the last fresh hidden incoming message in the chat as seen in `marknoticed_chat()` to
trigger emitting `MsgsNoticed` on other devices.

There's a problem though that another device may have more reactions received and not yet seen
notifications are removed from it when handling `MsgsNoticed`, but the same problem already exists
for "usual" messages, so let's not solve it for now.

It's interesting that sent out reactions are already hidden messages, so this change mostly just
unifies things.
iequidoo added a commit that referenced this issue Dec 6, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
a69d5a5 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, moreover there are no `msgs` rows for incoming reactions in the
core, so the UIs just call `marknoticed_chat()` in this case. We don't want to introduce reactions
message ids to the UIs (at least currently), but let's make received reactions usual messages, just
hidden and `InFresh`, so that the existing `\Seen` flag synchronisation mechanism works for them,
and mark the last fresh hidden incoming message in the chat as seen in `marknoticed_chat()` to
trigger emitting `MsgsNoticed` on other devices.

There's a problem though that another device may have more reactions received and not yet seen
notifications are removed from it when handling `MsgsNoticed`, but the same problem already exists
for "usual" messages, so let's not solve it for now.

It's interesting that sent out reactions are already hidden messages, so this change mostly just
unifies things.
iequidoo added a commit that referenced this issue Dec 8, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
6c90599 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, moreover there are no `msgs` rows for incoming reactions in the
core, so the UIs just call `marknoticed_chat()` in this case. We don't want to introduce reactions
message ids to the UIs (at least currently), but let's make received reactions usual messages, just
hidden and `InFresh`, so that the existing `\Seen` flag synchronisation mechanism works for them,
and mark the last fresh hidden incoming message in the chat as seen in `marknoticed_chat()` to
trigger emitting `MsgsNoticed` on other devices.

There's a problem though that another device may have more reactions received and not yet seen
notifications are removed from it when handling `MsgsNoticed`, but the same problem already exists
for "usual" messages, so let's not solve it for now.

It's interesting that sent out reactions are already hidden messages, so this change mostly just
unifies things.
iequidoo added a commit that referenced this issue Dec 12, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
3aa1e03 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, moreover there are no `msgs` rows for incoming reactions in the
core, so the UIs just call `marknoticed_chat()` in this case. We don't want to introduce reactions
message ids to the UIs (at least currently), but let's make received reactions usual messages, just
hidden and `InFresh`, so that the existing `\Seen` flag synchronisation mechanism works for them,
and mark the last fresh hidden incoming message in the chat as seen in `marknoticed_chat()` to
trigger emitting `MsgsNoticed` on other devices.

There's a problem though that another device may have more reactions received and not yet seen
notifications are removed from it when handling `MsgsNoticed`, but the same problem already exists
for "usual" messages, so let's not solve it for now.

It's interesting that sent out reactions are already hidden messages, so this change mostly just
unifies things.
iequidoo added a commit that referenced this issue Dec 13, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
00122b8 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, so the UIs just call `marknoticed_chat()` in this case. We don't
want to introduce reactions message ids to the UIs (at least currently), but let's make received
reactions `InFresh` so that the existing `\Seen` flag synchronisation mechanism works for them, and
mark the last fresh hidden incoming message (reaction) in the chat as seen in
`chat::marknoticed_chat()` to trigger emitting `MsgsNoticed` on other devices.

There's a problem though that another device may have more reactions received and not yet seen
notifications are removed from it when handling `MsgsNoticed`, but the same problem already exists
for "usual" messages, so let's not solve it for now.
iequidoo added a commit that referenced this issue Dec 13, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
e71b17c 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, so the UIs just call `marknoticed_chat()` in this case. We don't
want to introduce reactions message ids to the UIs (at least currently), but let's make received
reactions `InFresh` so that the existing `\Seen` flag synchronisation mechanism works for them, and
mark the last fresh hidden incoming message (reaction) in the chat as seen in
`chat::marknoticed_chat()` to trigger emitting `MsgsNoticed` on other devices.

There's a problem though that another device may have more reactions received and not yet seen
notifications are removed from it when handling `MsgsNoticed`, but the same problem already exists
for "usual" messages, so let's not solve it for now.
iequidoo added a commit that referenced this issue Dec 22, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
1076f37 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, so the UIs just call `marknoticed_chat()` in this case. We don't
want to introduce reactions message ids to the UIs (at least currently), but let's make received
reactions `InFresh` so that the existing `\Seen` flag synchronisation mechanism works for them, and
mark the last fresh hidden incoming message (reaction) in the chat as seen in
`chat::marknoticed_chat()` to trigger emitting `MsgsNoticed` on other devices.

There's a problem though that another device may have more reactions received and not yet seen
notifications are removed from it when handling `MsgsNoticed`, but the same problem already exists
for "usual" messages, so let's not solve it for now.
iequidoo added a commit that referenced this issue Dec 25, 2024
@iequidoo
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
c3c64b5 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, so the UIs just call `marknoticed_chat()` in this case. We don't
want to introduce reactions message ids to the UIs (at least currently), but let's make received
reactions `InFresh` so that the existing `\Seen` flag synchronisation mechanism works for them, and
mark the last fresh hidden incoming message (reaction) in the chat as seen in
`chat::marknoticed_chat()` to trigger emitting `MsgsNoticed` on other devices.

There's a problem though that another device may have more reactions received and not yet seen
notifications are removed from it when handling `MsgsNoticed`, but the same problem already exists
for "usual" messages, so let's not solve it for now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@iequidoo iequidoo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210) deltachat/deltachat-core-rust
1 participant
@iequidoo