Let a message request be accepted without answering it - #141
Conversation
Approval has only ever been a side effect of messages flowing, so a client whose UI offers an Accept button had nothing to call: accepting was answering, or it was nothing. DM::approve() records it outright, through the same helper the send path uses, because it is one fact however it was arrived at. The conversation leaves message_requests() for conversations(), and the Contacts config carries the approval to our other devices, exactly as an answer would. It also tells them, which nothing here did. Session's other clients send a MessageRequestResponse on that button, and short of a message from us it is the only thing that clears the other end's awaiting_approval, while accepting is precisely the choice not to send one. An arriving one is now read as well. Nothing looked at the field before, so a peer accepting on Desktop, Android or iOS left us waiting on somebody who had already said yes. Only the acceptance is taken from it, and only for an account already a contact of ours: approval has no reverse, and a response from someone we never wrote to answers a request we never made.
Writing to someone already approves them, so the declaration is where to name the other path rather than leave a reader to find it in _send_message.
| auto now = clock_now_ms(); | ||
| SessionProtos::Content content; | ||
| content.set_sigtimestamp(static_cast<uint64_t>(epoch_ms(now))); | ||
| content.mutable_messagerequestresponse()->set_isapproved(true); | ||
|
|
||
| // Registered rather than fired blind: Core reports on every send, and a status for an id nobody | ||
| // claims would sit in _early_status for the life of the process. | ||
| _quiet_sends.insert(core.send_dm(id.session_id(), content, now)); |
There was a problem hiding this comment.
So there is a race condition with the other path of this which made a regression test on iOS really flaky - essentially when accepting a message request via the "send message" channel we should still send a MessageRequestResponse (as that's what renders the "Your message request was accepted" UI) but if it's not done as a /sequence then the messages can arrive out of order and result in the "You message request was accepted" copy not being shown
There was a problem hiding this comment.
Ok, this looks like a bigger change to libsession-util.
We need to keep track of the msg request response, so also add support for control messages, and now do a sequence for those two msgs when accepting via text.
I'll leave this to Jason and mark this PR as a draft for now.
2 participants
DM::approve(): Session's Accept button, which accepts a message request without saying anything.Approval has only ever been a side effect of messages flowing, so a client whose UI offers Accept
had nothing to call.
approve()records it outright, through the same helper_send_messageuses,so the conversation leaves
message_requests()forconversations()and the Contacts configcarries the approval to the account's other devices exactly as an answer would. Approving somebody
already approved does nothing, and there is no reverse:
delete_contactis still what un-requestsa conversation.
It also sends a
MessageRequestResponse, as Session's other clients do on the same button, and onearriving is now read. Nothing looked at that field before, so a peer accepting on Desktop, Android
or iOS left us with
awaiting_approvalset on somebody who had already said yes. OnlyisApprovedis taken from it, and only for an account that is already a contact of ours: approval has no
reverse, and a response from someone we never wrote to answers a request we never made.
Deliberately not in here
both sides. That needs a message kind the client schema does not have, and is a change of its own.
LokiProfile. Nothing this branchsends carries one yet, so this does not either.
Tests
Three cases in
tests/test_client/requests.cpp:approvedto the Contacts config, adds nomessage, and puts exactly one thing on their swarm and nothing on ours. A second accept sends
nothing and tells nobody.
approve()actually put on the wire are fed tothe client they were addressed to, and clear its
awaiting_approvalwithout touching its history.contact nor a conversation, and a refusal from a real contact is not the un-approval it reads as.
Asked for in session-foundation/session-app#4, where
approveConversationis a bridge command withno libsession counterpart and its contract scenario is skipped for that reason.