-
-
Notifications
You must be signed in to change notification settings - Fork 84
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
feat: Add import_vcard() (#5202) #5582
Conversation
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.
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