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

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

Comments

@iequidoo
Copy link
Collaborator

iequidoo commented Nov 15, 2024

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 added a commit that referenced this issue Nov 15, 2024
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 added a commit that referenced this issue Nov 18, 2024
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
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
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
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
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
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
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
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
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
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
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
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
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
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
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant