-
-
Notifications
You must be signed in to change notification settings - Fork 111
feat: Add import_vcard() (#5202) #5582
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
Conversation
Hocuri
left a comment
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.
Looks very good already! Lots of small comments, most of them pretty unimportant, feel free to ignore those.
a948302 to
b9cefff
Compare
853d29d to
a08e643
Compare
b855038 to
9924e0b
Compare
deltachat-jsonrpc/src/api.rs
Outdated
| async fn import_vcard(&self, account_id: u32, path: String) -> Result<Vec<u32>> { | ||
| let ctx = self.get_context(account_id).await?; | ||
| let mut f = fs::File::open(&path).await?; | ||
| let mtime = f |
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.
I think it is better to use current time as a timestamp instead of mtime and not pass the timestamp around as arguments. mtime is always tricky, e.g. if you receive a vCard and then transfer a backup, second device may have have newer timestamp for the same vCard than the first device. Also regarding the time of contact modification, I think it makes more sense to store the date contact was modified if we merge with the changes that come from the network.
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.
For me using the current time is even worse as it changes every time the function is called, while file mtime usually doesn't. But this timestamp anyway was used as a fallback if we don't have timestamps in the vCard. DC doesn't create such vCards anyway. So i decided to use 0 (i.e. UNIX epoch) for vCards w/o timestamps. This means that contacts are created from such vCards, but if a contact already exists and has a gossip key, it is not updated
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.
The only exception if one such vCards is imported after another, then the gossip key is updated because the timestamp is 0 for both
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.
Also regarding the time of contact modification, I think it makes more sense to store the date contact was modified if we merge with the changes that come from the network.
Agree, but currently we don't have a place to store the contact modification time. last_seen isn't suited for this. Probably need to add another one contacts table column
Add a function importing contacts from the given vCard.
Hocuri
left a comment
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.
Ta-daaaaaa!!
Add a function importing contacts from the given vCard.
Close #5202