Details

Reviewers

Group Reviewers

Maniphest Tasks

Commits

R224:9c1d6e43ad99: Add contacts-reading plugin (KDE side)
R224:99fa6e43afc0: Add first-cut contacts-reading plugin
R224:68014ab9bd53: Upgrade TODOs to action items

Summary

Add a plugin to KDE Connect which supports exporting the Android contacts databases to vcards on the desktop

When the devices are connected, the plugin sends a request for all timestamps and IDs

When a packet with timestamps and IDs is received, it verifies it has vcards for each ID and that the timestamps match and deletes any vcards for IDs which were not reported. It then sends a request for all vcards which were missing or need updating

When a packet with vcards is received they are unconditionally written to disk, possibly overwriting existing vcards

Provides one dbus method: contacts/synchronizeRemoteWithLocal which triggers the request for all timestamps and IDs

BUG: 367999

Test Plan

Connect the device to the desktop and verify that vcards are created in QStandardPaths::GenericDataLocation / kpeoplevcard". On my system this is ~/.local/share/kpeoplevcard

Create a dummy contact on the device and verify it is synchronized (Currently not automatic, have to disconnect and reconnect or use dbus)

Modify the dummy contact and verify the modifications are synchronized (Currently not automatic, have to disconnect and reconnect or use dbus)

Delete the dummy contact and verify the deletion is synchronized (Currently not automatic, have to disconnect and reconnect or use dbus)

Diff Detail

Repository

R224 KDE Connect

Branch

contacts-rebase

Lint

No Linters Available

Unit

No Unit Test Coverage

sredman requested review of this revision.Jan 6 2018, 1:52 AM

sredman created this revision.

sredman added a reviewer: KDE Connect.Jan 6 2018, 1:55 AM

Add dbus interface for requesting unique IDs for accessing the contacts

Attach dbus to the correct path

Add interface for requesting contact names given uIDs

Change return type of getAllContactUIDs to int instead of string

Add interface for requesting phone numbers and email addresses

Add PhoneEntry container class for all the data associated with a phone number
Add interface for requesting phone numbers given a list of uIDs
Remove old interface for requesting random jumble of contact names + phone numbers
Combine sender methods into single helper
Add locks when handling a phone number response packet
Stop manually setting up DBus connection, since it is already handled
Add package type and handler for requesting contact phone numbers given a list of uIDs
Reorder methods

sredman retitled this revision from Add first-cut contacts-reading plugin to Add contacts-reading plugin.Jan 16 2018, 1:41 AM

sredman edited the summary of this revision. (Show Details)

sredman edited the test plan for this revision. (Show Details)

The DBus API seems too convoluted, I suggest using vcard for now, which will be understood by every PIM suite in existance.

apol requested changes to this revision.Mar 16 2018, 10:23 PM

This revision now requires changes to proceed.Mar 16 2018, 10:23 PM

This revision was not accepted when it landed; it landed in state Needs Revision.Mar 24 2018, 1:20 PM

Closed by commit R224:99fa6e43afc0: Add first-cut contacts-reading plugin (authored by sredman). · Explain Why

This revision was automatically updated to reflect the committed changes.

sredman reopened this revision.Mar 25 2018, 5:16 PM

Update contacts plugin for VCard API

Combine sender methods into single helper
Strip down contacts plugin for new VCards api
Merge remote-tracking branch 'origin/master' into contacts
Write VCards to local files
Move vcards storage path to class-level
Implment reading local cache for missing and remove unreported contacts
Update locally-cached vcards if the timestamp differs
Synchronize contacts when plugin loads

Cleanup TODOs

This revision was not accepted when it landed; it landed in state Needs Review.Mar 25 2018, 6:28 PM

Closed by commit R224:68014ab9bd53: Upgrade TODOs to action items (authored by sredman). · Explain Why

This revision was automatically updated to reflect the committed changes.

sredman added a task: T8283: Sync Contacts.Mar 27 2018, 4:39 PM

Restricted Application added a project: KDE Connect. · View Herald TranscriptMar 27 2018, 4:39 PM

sredman reopened this revision.Mar 27 2018, 4:41 PM

sredman retitled this revision from Add contacts-reading plugin to Add contacts-reading plugin (KDE side).Mar 27 2018, 5:21 PM

sredman edited the summary of this revision. (Show Details)

sredman edited the test plan for this revision. (Show Details)

sredman added a subscriber: KDE Connect.

Same here.

Update diff with ALL changes. Arc is too powerful for me!

Add first-cut contacts-reading plugin
Add dbus interface for requesting unique IDs for accessing the contacts
Attach dbus to the correct path
Change return type of getAllContactUIDs to int instead of string
Add interface for requesting contact names given uIDs
Add PhoneEntry container class for all the data associated with a phone number
Add interface for requesting phone numbers given a list of uIDs
Remove old interface for requesting random jumble of contact names + phone numbers
Combine sender methods into single helper
Add locks when handling a phone number response packet
Stop manually setting up DBus connection, since it is already handled
Add package type and handler for requesting contact phone numbers given a list of uIDs
Reorder methods
Change NetworkPackage to NetworkPacket
Strip down contacts plugin for new VCards api
Print VCards
Add dependency on KPeople for contacts plugin
Revert "Add dependency on KPeople for contacts plugin"
Write VCards to local files
Move vcards storage path to class-level
Implment reading local cache for missing and remove unreported contacts
Update locally-cached vcards if the timestamp differs
Synchronize contacts when plugin loads
Upgrade TODOs to action items

In D9691#235506, @mtijink wrote:

Same here.

Okay. I am now more confident that I have done this correctly :)

Some nitpicking... looking much better.
Please extrapolate the comments to similar situations.

plugins/contacts/contactsplugin.cpp
71	in KDE Connect we don't add a new line after the if: `} else if () {` Fix others too.
102	mark uIDs const
109	const QString & ID
180	auto &
plugins/contacts/contactsplugin.h
80	How about: typedef qint64 UID; typedef QList<uID_t> uIDList_t;
144	const QString &
154	const QString & and uIDs

This revision now requires changes to proceed.Mar 28 2018, 12:08 PM

mtijink added inline comments.Mar 29 2018, 7:45 PM

plugins/contacts/contactsplugin.cpp
70	It's not really common in C++ to write `this->`.
plugins/contacts/contactsplugin.h
69	You can get the uid's by calling `map.keys()`, so the separate list is not needed.
75	Wouldn't `kdeconnect/vcard` make more sense?
122	Why protected? We're not going to inherit from this, right?
plugins/contacts/kdeconnect_contacts.json
13	How will the contact integrate with the desktop? E.g. will I see the contacts in KAddressBook? Or can I sent people e-mails using this? If the contacts conflict with what the user has already set up, I'd disable it by default. Otherwise, this is fine.

Update ContactsPlugin to use LOOKUP_KEY instead of RAW_CONTACT_ID since that is what LOOKUP_KEY is for and address reviewer comments

In D9691#235891, @apol wrote:

Some nitpicking... looking much better.
Please extrapolate the comments to similar situations.

Nitpicking is good. That is how I learn!

plugins/contacts/contactsplugin.h
75	The KPeople plugin KPeopleVCard expects vcards to be in that folder
122	The future is uncertain... Maybe someone wants to make a BetterContactsPlugin which supports more packets but lives alongside this plugin? I don't see any reason to stop them.
plugins/contacts/kdeconnect_contacts.json
13	vcards are written to the folder where KPeopleVCard expects to find them. Then, once KPeople and KPeopleVCard are properly configured, any application which uses KPeople will automatically find the contacts Writing contacts back to the phone is not (yet) supported Kpeople supports merging contacts, and each KDE Connect device writes to its own folder, so if multiple contacts with the same information exist they can be merged within KPeople

sredman marked 6 inline comments as done.Mar 30 2018, 5:09 AM

sredman added inline comments.Mar 30 2018, 5:13 AM

plugins/contacts/contactsplugin.cpp
70	Does it do any harm? It's a habit from writing too much Python (which I'd rather keep so I can still writing Python) and being overly clear never hurt anybody
plugins/contacts/contactsplugin.h
69	I believe this comment meant to apply somewhere else in the code? But in general I like to give names to things so that other lines are shorter and clearer

Emit signal reporting changed vcards

sredman marked 11 inline comments as done.Apr 18 2018, 7:04 PM

Patch looks good to me, I'd say let's merge it so we can start testing.

plugins/contacts/contactsplugin.h
122	Well that's not a great reason. If we ever need more abstraction we'll have to modify the code, and that's fine. I'm not concerned about this though. Note that public before Q_SIGNAL it does nothing and can be removed.

apol accepted this revision.Apr 18 2018, 10:44 PM

This revision is now accepted and ready to land.Apr 18 2018, 10:44 PM

nicolasfella mentioned this in T8555: [RFC] Import contacts from KDE Connect.Apr 19 2018, 12:04 PM

Closed by commit R224:9c1d6e43ad99: Add contacts-reading plugin (KDE side) (authored by sredman, committed by nicolasfella). · Explain WhyApr 23 2018, 8:30 PM

This revision was automatically updated to reflect the committed changes.

		Path
M		plugins/CMakeLists.txt (1 line)
A	M	plugins/contacts/CMakeLists.txt (11 lines)
A	M	plugins/contacts/README (3 lines)
A	M	plugins/contacts/contactsplugin.h (157 lines)
A	M	plugins/contacts/contactsplugin.cpp (229 lines)
A	M	plugins/contacts/kdeconnect_contacts.json (33 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	24807	1f6544e		Jan 6 2018, 1:52 AM	★	★
Diff 2	25361	1f6544e	Add dbus interface for requesting unique IDs for accessing the contacts	Jan 15 2018, 2:27 AM	★	★
Diff 3	25363	1f6544e	Add interface for requesting contact names given uIDs	Jan 15 2018, 7:35 AM	★	★
Diff 4	25435	1f6544e	Add interface for requesting phone numbers and email addresses	Jan 16 2018, 1:15 AM	★	★
Diff 5	30389	0c560d9	R224:99fa6e43afc089d995762356f1ee26a8ad400fda	Mar 23 2018, 10:09 PM	★	★
Diff 6	30533	33c2a10	Update contacts plugin for VCard API	Mar 25 2018, 5:18 PM	★	★
Diff 7	30539	581b41b	Cleanup TODOs	Mar 25 2018, 6:28 PM	★	★
Diff 8	30540	581b41b	R224:68014ab9bd53536ae060eac8c102b36838aaadd0	Mar 25 2018, 6:27 PM	★	★
Diff 9	30769	bf327e5	Update diff with ALL changes. Arc is too powerful for me!	Mar 28 2018, 12:43 AM	★	★
Diff 10	30898	bf327e5	Update ContactsPlugin to use LOOKUP_KEY instead of RAW_CONTACT_ID since that is…	Mar 30 2018, 5:07 AM	★	★
Diff 11	31071	bf327e5	Emit signal reporting changed vcards	Mar 31 2018, 9:42 PM	★	★
Diff 12	32908	634d33b	R224:9c1d6e43ad99f51fbf0619e1ca7fca4283f122f0	Apr 23 2018, 8:30 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
b73f56e2aa9d	495bc056d7fc	e03b01146f10	Simon Redman	Upgrade TODOs to action items	Mar 25 2018, 6:27 PM
e03b01146f10	db4005f264dc	f4e9a22d34e3	Simon Redman	Synchronize contacts when plugin loads	Mar 25 2018, 5:11 PM
f4e9a22d34e3	b3791eaae5a9	40977a1f5c76	Simon Redman	Update locally-cached vcards if the timestamp differs	Mar 25 2018, 5:03 PM
40977a1f5c76	3b4b035c2343	9c71292b876e	Simon Redman	Implment reading local cache for missing and remove unreported contacts (Show More…)	Mar 25 2018, 4:41 PM
9c71292b876e	87ba4dd667ca	d650bd016d2c	Simon Redman	Move vcards storage path to class-level	Mar 25 2018, 4:39 PM
d650bd016d2c	e6036ed73448	b2107b5e55df	Simon Redman	Write VCards to local files	Mar 25 2018, 1:59 PM
b2107b5e55df	8781b61560a8	5df02f5c732e	Simon Redman	Revert "Add dependency on KPeople for contacts plugin" (Show More…)	Mar 25 2018, 12:37 PM
5df02f5c732e	749ec892dae6	ac36efeb892c	Simon Redman	Add dependency on KPeople for contacts plugin	Mar 25 2018, 12:18 PM
ac36efeb892c	8781b61560a8	a82d3944fab0	Simon Redman	Print VCards	Mar 25 2018, 11:59 AM
a82d3944fab0	22823e1e4fbb	44060da99e2d	Simon Redman	Strip down contacts plugin for new VCards api	Mar 25 2018, 9:32 AM
44060da99e2d	e0f282fb6eac	78fd60aa7a78	Simon Redman	Change NetworkPackage to NetworkPacket	Mar 24 2018, 1:19 PM
78fd60aa7a78	3db1dd368f74	40c042ef037d	Simon Redman	Reorder methods	Jan 16 2018, 1:09 AM
40c042ef037d	de0b46392728	dc13e18b8877	Simon Redman	Add package type and handler for requesting contact phone numbers given a list… (Show More…)	Jan 16 2018, 1:05 AM
dc13e18b8877	5e1c55704dec	a1abd5bc0bcd	Simon Redman	Stop manually setting up DBus connection, since it is already handled	Jan 16 2018, 1:03 AM
a1abd5bc0bcd	71518e47379b	f662ee259647	Simon Redman	Add locks when handling a phone number response packet	Jan 16 2018, 12:06 AM
f662ee259647	590f261537d7	aea472633f69	Simon Redman	Combine sender methods into single helper	Jan 16 2018, 12:03 AM
aea472633f69	ef12e2a8e0e6	abcddee52732	Simon Redman	Remove old interface for requesting random jumble of contact names + phone… (Show More…)	Jan 15 2018, 11:01 PM
abcddee52732	8e80ffe3218b	c6f3a032eb2c	Simon Redman	Add interface for requesting phone numbers given a list of uIDs	Jan 15 2018, 10:51 PM
c6f3a032eb2c	aa7d064ccb35	4dde2a947f93	Simon Redman	Add PhoneEntry container class for all the data associated with a phone number	Jan 15 2018, 10:46 PM
4dde2a947f93	9f2839516492	7345d52b4a24	Simon Redman	Add interface for requesting contact names given uIDs	Jan 15 2018, 7:35 AM
7345d52b4a24	312d4a9b51e5	f7878cda168f	Simon Redman	Change return type of getAllContactUIDs to int instead of string	Jan 15 2018, 4:41 AM
f7878cda168f	94c9d423a9f0	525ef57595c9	Simon Redman	Attach dbus to the correct path	Jan 15 2018, 2:26 AM
525ef57595c9	9af0ba05b3cc	a93c5ef3f9c5	Simon Redman	Add dbus interface for requesting unique IDs for accessing the contacts	Jan 15 2018, 2:26 AM
a93c5ef3f9c5	8dea8c4c86d2	bf327e5b8c6c	Simon Redman	Add first-cut contacts-reading plugin (Show More…)	Jan 6 2018, 1:51 AM

Add contacts-reading plugin (KDE side)
ClosedPublic
Actions

Details

Diff Detail

Revision Contents
Changeset List

Diff 30769

plugins/CMakeLists.txt

plugins/contacts/CMakeLists.txt

plugins/contacts/README

plugins/contacts/contactsplugin.h

plugins/contacts/contactsplugin.cpp

plugins/contacts/kdeconnect_contacts.json

Add contacts-reading plugin (KDE side)ClosedPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 30769

plugins/CMakeLists.txt

plugins/contacts/CMakeLists.txt

plugins/contacts/README

plugins/contacts/contactsplugin.h

plugins/contacts/contactsplugin.cpp

plugins/contacts/kdeconnect_contacts.json

Add contacts-reading plugin (KDE side)
ClosedPublic
Actions

Revision Contents
Changeset List