-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Small change for #5999 #6047
Conversation
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`.
@@ -1014,15 +1014,16 @@ async fn add_parts( | |||
} | |||
} | |||
|
|||
state = if (seen && replace_msg_id.is_none()) | |||
state = if replace_msg_id.is_some() { |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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
f2abfd5
to
1a7f108
Compare
See #5999