add send_draft() to JSON-RPC API #4839
Conversation
deltachat-jsonrpc/src/api/mod.rs
Outdated
| @@ -1848,6 +1848,22 @@ impl CommandApi { | |||
| } | |||
| } | |||
|
|
|||
| async fn send_draft(&self, account_id: u32, chat_id: u32) -> Result<u32> { | |||
There was a problem hiding this comment.
I did it this way because it was the simplest way to implement it with my limited Rust copy+paste skills, but probably it is more bug-free to pass the draft/msg id instead to be sure you are sending the draft you are expecting to send and not another draft that replaced the draft you think you are sending in the mean time
anyway for bots this is good enough, desktop is maybe more sensitive, feel free to pick up this pull request and improve
There was a problem hiding this comment.
I would be for providing just msg id of draft and the function tries to load it and if it can then it sends it.
There was a problem hiding this comment.
also if the api is not final you can make it start with misc_. BTW the current api for setting drafts is also not quite ready as it does not return the message id of the draft yet and has no option to keep the same msg id even when you change the text it will create a new message.
There was a problem hiding this comment.
the danger of an just send whatever draft function is that it could produce race conditions like:
- create draft
- send draft because user clicked on send
- send draft because user clicked on send multiple times (this one could be delayed for some reason
- user creates new draft for new message
- Unfinished message is send due to the delayed send draft call
Admittedly this is theoretically, but I still think a api where you create a draft then send it by it's id is better.
Anyways we should really think about this, also how to implement it nicely in desktop.
There was a problem hiding this comment.
yes, it absolutely makes sense to pass msg id, but if no one will take care of this PR soon, I would like to have this api at least for bots, where that is not a problem, to at least have something to work with in bots
|
tbh, i do not really get why a on low-level rust and cffi, a draft is send as every other message using sure, things can always be done differently, however, unless there are good reason, it makes sense to do things in a similar way, with similar functions and names EDIT: answering myself: there is just no |
There is a There is currently no Sending a draft will then be no different from sending a normal message. |
sending preserving the message ID etc. without recreating a new message and re-setting all status updates is better IMHO, that is set_draft(), get msg-id returned by set_draft(), send status updates, send the original message draft, at least that is what android does |
| let ctx = self.get_context(account_id).await?; | ||
| if let Some(draft) = ChatId::new(chat_id).get_draft(&ctx).await? { | ||
| let mut draft = draft; | ||
| let msg_id = chat::send_msg(&ctx, ChatId::new(chat_id), &mut draft) |
There was a problem hiding this comment.
does sending the draft make it disappear from draft? or it most be manually removed after sending?
There was a problem hiding this comment.
If it keeps the msg_id, how can it be kept? Should be transformed into the same message.
Anyway, this PR needs a test checking that msg_id is kept and that no new message is created as a result.
|
So how about |
There was a problem hiding this comment.
looks good for now, it is marked as experimental via the misc_ prefix.
We can discuss this at length next week, I also discussed a bit with @r10s personally already.
I vote for merging this so adb can do what he wants and then breaking it in the next update where we do everything "right"
close #4643