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

Small change for #5999 #6047

Conversation

Hocuri
Copy link
Collaborator

@Hocuri Hocuri commented Oct 15, 2024

See #5999

Add a test on what happens currently when apps call `markseen_msgs()` for not downloaded
messages. Such messages are marked as seen, but MDNs aren't sent for them. This is probably correct,
but when a message is downloaded, its state should be downgraded to `InNoticed` and when the user
really sees it, the app should issue `markseen_msgs()` again and that should trigger sending an
MDN. Currently when such a message is downloaded, it remains `InSeen`.
@Hocuri Hocuri marked this pull request as draft October 15, 2024 16:02
@link2xt link2xt changed the base branch from main to iequidoo/markseen_not_downloaded_msg October 15, 2024 20:27
@@ -1014,15 +1014,16 @@ async fn add_parts(
}
}

state = if (seen && replace_msg_id.is_none())
state = if replace_msg_id.is_some() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure that the code can be rewritten this way. If a message is replaced, that doesn't mean it's already noticed. Maybe it's replaced because DownloadLimit was changed and the message was resent. But i didn't test such a scenario, so i decided not to change the code here too much

@@ -1564,7 +1565,7 @@ INSERT INTO msgs
ON CONFLICT (id) DO UPDATE
SET rfc724_mid=excluded.rfc724_mid, chat_id=excluded.chat_id,
from_id=excluded.from_id, to_id=excluded.to_id, timestamp_sent=excluded.timestamp_sent,
type=excluded.type, state=min(state,max(?,13)), msgrmsg=excluded.msgrmsg,
type=excluded.type, state=min(state,excluded.state), msgrmsg=excluded.msgrmsg,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for this, i agree that excuded.state should be used instead of ?, but i prefer to have max(...,13) here because we never want to downgrade the state to InFresh. So this is a bit cumbersome, but more straightforward

@iequidoo iequidoo force-pushed the iequidoo/markseen_not_downloaded_msg branch 2 times, most recently from f2abfd5 to 1a7f108 Compare October 17, 2024 01:17
@Hocuri Hocuri closed this Oct 17, 2024
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 this pull request may close these issues.

2 participants