OK, I've managed to compile the 2014 GSoC code, it indeed required minimal porting (mostly CMakeLists.txt and headers).
The code works; however, apparently, several view widgets (kpeople/widgets/person{email,note,chat,event,post,files}view.h) went from KPeople right into the abyss, and the only thing left is persondetailsview.h.
The current code there is able to show only general details about the contact, such as avatar, name, online status. Yeah, and it works only with Telepathy contacts, since Akonadi plugin is disabled.

Next I'm going to look into the Akonadi plugin for KPeople, which should feed KPeople with Akonadi contacts. There is indeed some disabled code in KPeople, I'm going to look into it next
NB: Daniel told that, since KPeople is inside Frameworks, it cannot depend on Akonadi, so we'll have to move it to Akonadi code base eventually.

In T9595#158350, @poboiko wrote:

Next I'm going to look into the Akonadi plugin for KPeople, which should feed KPeople with Akonadi contacts. There is indeed some disabled code in KPeople, I'm going to look into it next
NB: Daniel told that, since KPeople is inside Frameworks, it cannot depend on Akonadi, so we'll have to move it to Akonadi code base eventually.

If possible I would prefer if we do not move KPeople from Frameworks -> Applications. It is quite better to separate the Akonadi based part into an own repository or merge it into akonadi-{notes,calendar,...} this way makes it easier for other applications to use KPeople without a build dependency to Akonadi. Or use a plugin system for KPeople where the akonadi plugin is built within KDE PIM.

If possible I would prefer if we do not move KPeople from Frameworks -> Applications. It is quite better to separate the Akonadi based part into an own repository or merge it into akonadi-{notes,calendar,...} this way makes it easier for other applications to use KPeople without a build dependency to Akonadi. Or use a plugin system for KPeople where the akonadi plugin is built within KDE PIM.

Sure, that's what I meant. It seems like KPeople architecture allows that (i.e., KTp plugin lives inside ktp-common-internals, so it should be possible to just land Akonadi plugin inside akonadi-contacts).

I started digging into Akonadi plugin itself. Again, after minimal porting it seems to be operating. However, it's able to fetch Akonadi::Items with KPeople::AbstractContact payload (which are non-existent, since all contacts are actually KContact::Addressee's). Apparently, we need conversion between them. Shouldn't be that hard.

I guess either in that time KContacts framework was non-existent, or KPeople was intended to replace it.
Side thought: does it really make sense to have two frameworks with this kind of functionality? Maybe KContacts may be absorbed inside KPeople at some point?
Doesn't need to be done now, for sure - it may create too much overhead, need to perform migration and related problems.

Indeed, we don't want to move KPeople from Frameworks, what I and @poboiko meant was moving the Akonadi code from KPeople to akonadi-contacts or somewhere similar.

However, it's able to fetch Akonadi::Items with KPeople::AbstractContact payload (which are non-existent, since all contacts are actually KContact::Addressee's). Apparently, we need conversion between them. Shouldn't be that hard

We should probably have a KPeople::AbstractContact subclass that is really just a wrapper around KContact::Addressee returned from Akonadi (rather than to convert between them). In fact you probably even want it to be a wrapper around Akonadi::Item, or at least keeping reference to Item ID. I don't know why the code assumed that it would obtain a KPeople::AbstractContact from Akonadi Items.

I guess either in that time KContacts framework was non-existent, or KPeople was intended to replace it.
Side thought: does it really make sense to have two frameworks with this kind of functionality? Maybe KContacts may be absorbed inside KPeople at some point?

KContacts is waaaaaay older than KPeople :) There was never an intention for KPeople to replace KContacts as those two libraries have a completely different purpose. KContacts is a very complete implementation of the vCard format (RFC 6350) which is the most widely used format to exchange contact information online. KPeople is just an abstraction over the concepts of contacts and personas.

We should probably have a KPeople::AbstractContact subclass that is really just a wrapper around KContact::Addressee returned from Akonadi (rather than to convert between them). In fact you probably even want it to be a wrapper around Akonadi::Item, or at least keeping reference to Item ID. I don't know why the code assumed that it would obtain a KPeople::AbstractContact from Akonadi Items.

You're right. AbstractContact itself isn't capable of storing anything anyways, it indeed is supposed to be a wrapper around some backend-dependent data.
KPeople uses URIs as identifiers, and we can just plug Akonadi::Item::url() there (those are not supposed to change, I hope). The only difference is that KPeople expects URI to look like akonadi://?item=1234 (namely, it uses everything to the left of "://" to determine the backend plugin), while Akonadi gives URLs in a form akonadi:?item=1234. Which is not a big deal.

I've modified the Akonadi backend for KPeople accordingly, it seems to be working so far. There are few side notes, however:

• One of default properties for AbstractContact is PhoneNumberProperty, which is supposed to be a default phone number. KContact::Addressee has this kind of functionality for email, but not for phone number. Which doesn't look like a big deal, since there is also AllPhoneNumbersProperty.
• Another property is GroupsProperty, which is QStringList of groups to which this contact belong. Not quite sure how it should be done in Akonadi, but this is a pretty nice functionality. It should be somehow done in a unified fashion, because:
  • There is Akonadi::Tag, which seems to be capable of it
  • There is also ContactGroup class
  • However, in Google Contacts resource (which I use), this is done through virtual collections
• Different backends store different information inside a contact. There are several common properties specified in the KPeople documentation, which are clearly not sufficient for vCards. So inside KAddressBook we either should show only this common information (that sucks), or be aware of all possible backends (that seems to be impossible). Or it can care only for Akonadi-based contacts (however, it would be nice to show also i.e. Telepathy contacts inside KAddressBook...).

It might be logical for backends to specify their own widgets/views for their contacts. But then there is a question how to merge them and show in a single view, if we have metacontacts from different backends...
Or we can make KPeople aware of all possibilities, and have a Big Nice Person View, which supports everything. Which means that if we add another backend, which will have even more fields, we'll need to adapt KPeople accordingly in order to support them. Which doesn't make much sense.
Not sure what to do with that for now.

KContacts is waaaaaay older than KPeople :) There was never an intention for KPeople to replace KContacts as those two libraries have a completely different purpose. KContacts is a very complete implementation of the vCard format (RFC 6350) which is the most widely used format to exchange contact information online. KPeople is just an abstraction over the concepts of contacts and personas.

Oh. Now it makes sense. Forgive me my ignorance :(

Nice progress!

The only difference is that KPeople expects URI to look like akonadi://?item=1234 (namely, it uses everything to the left of "://" to determine the backend plugin), while Akonadi gives URLs in a form akonadi:?item=1234. Which is not a big deal.

Hmm, from QUrl point of view, those URIs should be identical I think (schema = akonadi, path = /empty/, query = { item: 1234 }). If KPeople is parsing them as regular QStrings then that should be fixed in KPeople...

One of default properties for AbstractContact is PhoneNumberProperty, which is supposed to be a default phone number. KContact::Addressee has this kind of functionality for email, but not for a phone number. Which doesn't look like a big deal, since there is also AllPhoneNumbersProperty.

KContact::PhoneNumber::type() returns a bit flag - if the KContact::PhoneNumber::Pref bit is set, the number is considered default. We could probably extend the KContact::Addressee class to have defaultPhoneNumber() getter that would lookup and return such number I suppose.

Another property is GroupsProperty, which is QStringList of groups to which this contact belong. Not quite sure how it should be done in Akonadi, but this is a pretty nice functionality. It should be somehow done in a unified fashion, because:
There is Akonadi::Tag, which seems to be capable of it

Akonadi::Tag could be considered a group in the sense of KPeople groups, yes. The original KPeople "group" concept comes from the IM contact list groups (you would have e.g. "Work" and "Close friends" groups etc. in your IM), so tags are indeed the closest representation of that in Akonadi.

There is also ContactGroup class

KContacts::ContactGroup is more like a couple of contacts linked together so that you can send email to all of them easily. KContacts::Addressee and KContacts::ContactGroup should be treated as equal entities, in fact, you should have a support for retrieving ContactGroups from Akonadi as well.

However, in Google Contacts resource (which I use), this is done through virtual collections

Indeed. We can, however, change that quite easily (well, not so easily, but we can) to better fit the KPeople model. We could only sync the contacts in a single collection and tag them for the group, instead of putting them into virtual sub-collections. This would actually make KAddressBook quite powerful I think because it would mean the contacts from "Best Friends" group from your Google Contacts and your e.g. Outlook address books would all appear under the same "Best Friends" group in KAB.

Different backends store different information inside a contact. There are several common properties specified in the KPeople documentation, which are clearly not sufficient for vCards. So inside KAddressBook we either should show only this common information (that sucks), or be aware of all possible backends (that seems to be impossible).

For displaying information in the contact list, the common properties provided by AbstractContact should enough - picture, name, email, phone is all we want there

Or it can care only for Akonadi-based contacts (however, it would be nice to show also i.e. Telepathy contacts inside KAddressBook...).

Right, we could probably start by only allowing the Akonadi backend and eventually re-enable other backends later on. To be honest, not many people use KTp these days anyway and I'm not aware of any other KPeople backend, so we don't need to stress about this too much.

But then there is a question how to merge them and show in a single view if we have metacontacts from different backends...Or we can make KPeople aware of all possibilities, and have a Big Nice Person View, which supports everything. Which means that if we add another backend, which will have even more fields, we'll need to adapt KPeople accordingly in order to support them. Which doesn't make much sense. Not sure what to do with that for now.

So KAddressBook uses Grantlee templates to display the contact details. Grantlee templates are just plain simple HTML files with placeholder variables that get replaced with actual values during processing. We already have a template for KContacts::Addressee, so we would just add a simple template just for the default properties from KPeople::AbstractContact (in case it comes from a backend we don't know). When the user selects a metacontact, we could just concatenate an output from the templates of each of the merged contacts.

We could also try to build a super-KContact::Addressee containing aggregated values from all the sub-entries, but that also requires some logic to de-duplicate values etc., so probably something for later...

Hmm, from QUrl point of view, those URIs should be identical I think (schema = akonadi, path = /empty/, query = { item: 1234 }). If KPeople is parsing them as regular QStrings then that should be fixed in KPeople...

They use them as unique identifiers i.e. to look them up in QMaps or database. I'm not even sure they want those to be proper URLs, they just have look-alike schemas (i.e. <backend>://<whatever-contact-id>).

KContact::PhoneNumber::type() returns a bit flag - if the KContact::PhoneNumber::Pref bit is set, the number is considered default. We could probably extend the KContact::Addressee class to have defaultPhoneNumber() getter that would lookup and return such number I suppose.

Lovely, thanks! For now I can do it inside the backend, doesn't matter that much. However, it looks to be better fitting inside KContact::Addresse, so we can put it there at some point.

KContacts::ContactGroup is more like a couple of contacts linked together so that you can send email to all of them easily. KContacts::Addressee and KContacts::ContactGroup should be treated as equal entities, in fact, you should have a support for retrieving ContactGroups from Akonadi as well.

Hmm, hat doesn't look good, it doesn't look like KPeople has its counterpart. In principle, we could create some sort of virtual AbstractContact for a group, but those are different entities with different behavior. It might get weird if someone would want to create a metacontact with ordinary contacts as well as other groups.
On the other hand, I guess no one would actually need that functionality, so we can just fetch contacts from KPeople and ContactGroups from Akonadi, and show them in a single Model.

Indeed. We can, however, change that quite easily (well, not so easily, but we can) to better fit the KPeople model. We could only sync the contacts in a single collection and tag them for the group, instead of putting them into virtual sub-collections. This would actually make KAddressBook quite powerful I think because it would mean the contacts from "Best Friends" group from your Google Contacts and your e.g. Outlook address books would all appear under the same "Best Friends" group in KAB.

That sounds cool!

Right, we could probably start by only allowing the Akonadi backend and eventually re-enable other backends later on. To be honest, not many people use KTp these days anyway and I'm not aware of any other KPeople backend, so we don't need to stress about this too much.

So KAddressBook uses Grantlee templates to display the contact details. Grantlee templates are just plain simple HTML files with placeholder variables that get replaced with actual values during processing. We already have a template for KContacts::Addressee, so we would just add a simple template just for the default properties from KPeople::AbstractContact (in case it comes from a backend we don't know).

Right, let's stick to it for now.

When the user selects a metacontact, we could just concatenate an output from the templates of each of the merged contacts.
We could also try to build a super-KContact::Addressee containing aggregated values from all the sub-entries, but that also requires some logic to de-duplicate values etc., so probably something for later...

But that sounds exactly like what KPeople should do! It even has basic functionality for now - it has KPeople::MetaContact class, which disguise itself as a AbstractContact. When asked for a List property (i.e. phone numbers, emails, ...), it just merges everything from all the subcontacts and shows them; when asked for a singleton property (i.e. photo, name), it picks one that pops up first (although, it would be nice to allow users to choose what they want to see, but... Well, that's for later).

And now for something completely different - I'm starting to look into the Model part.
It looks like EntityTreeModel is specifically used to watch stuff inside Akonadi collections and represent them. In our case, that is done by KPeople, i.e. plugin has its own Akonadi::Monitor and takes care of everything on its own. So it seems like we don't really need to be to implement ETM subclass, just an ordinary QAbstractItemModel proxy should be sufficient.
However, if we don't find a way to fit ContactGroups inside KPeople logic and want to fetch them separately, we might need both (i.e. an ETM for ContactGroups, QAbstractItemModel for Persons, and somehow merge them inside a single model). Sounds a bit weird, but might work. What do you think?

Hmm, from QUrl point of view, those URIs should be identical I think (schema = akonadi, path = /empty/, query = { item: 1234 }). If KPeople is parsing them as regular QStrings then that should be fixed in KPeople...

They use them as unique identifiers i.e. to look them up in QMaps or database. I'm not even sure they want those to be proper URLs, they just have look-alike schemas (i.e. <backend>://<whatever-contact-id>).

Yeah, and KPeople keeps the URL around as QStrings instead of QUrls, which is bad. It should be a QUrl. Unfortunately, this probably cannot be fixed in KPeople without breaking API compatibility :(`

KContacts::ContactGroup is more like a couple of contacts linked together so that you can send email to all of them easily. KContacts::Addressee and KContacts::ContactGroup should be treated as equal entities, in fact, you should have a support for retrieving ContactGroups from Akonadi as well.

Hmm, hat doesn't look good, it doesn't look like KPeople has its counterpart. In principle, we could create some sort of virtual AbstractContact for a group, but those are different entities with different behavior. It might get weird if someone would want to create a metacontact with ordinary contacts as well as other groups.

On the other hand, I guess no one would actually need that functionality, so we can just fetch contacts from KPeople and ContactGroups from Akonadi, and show them in a single Model.

We should wrap KContacts::ContactGroup into a single KPeople::AbstractContact the same way we do with KContacts::Addressee. ContactGroup is basically still just a contact: it has a name and multiple email addresses. In the contact list, this is fine. Yes, someone could try to "merge" two contact groups into one metacontact, but I *think* there's a way in KPeople to disallow merging conditionally (I'm not sure though).

When the user selects a metacontact, we could just concatenate an output from the templates of each of the merged contacts. We could also try to build a super-KContact::Addressee containing aggregated values from all the sub-entries, but that also requires some logic to de-duplicate values etc., so probably something for later...

But that sounds exactly like what KPeople should do! It even has basic functionality for now - it has KPeople::MetaContact class, which disguise itself as a AbstractContact. When asked for a List property (i.e. phone numbers, emails, ...), it just merges everything from all the subcontacts and shows them; when asked for a singleton property (i.e. photo, name), it picks one that pops up first (although, it would be nice to allow users to choose what they want to see, but... Well, that's for later).

I think there may be a bit of misunderstanding about the cases we are addressing here.

For the contact list, the merged data provided by KPeople::MetaContact is enough, no problem here.
For the contact view (the big thing on the right where the full contact appears when you select it the contact list) the merged data from KPeople::MetaContact will not be enough - KPeople is not aware about geographical addresses, birthdays, occupations and all the crazy stuff that KContacts::Addressee can support, which we want to display in the contact view. Just try to create a local contact (not on Google, but add a new vCard addressbook) and try to fill in all the fields in the contact editor to get an idea about how much stuff there is.

And now for something completely different - I'm starting to look into the Model part. It looks like EntityTreeModel is specifically used to watch stuff inside Akonadi collections and represent them. In our case, that is done by KPeople, i.e. plugin has its own Akonadi::Monitor and takes care of everything on its own. So it seems like we don't really need to be to implement ETM subclass, just an ordinary QAbstractItemModel proxy should be sufficient. However, if we don't find a way to fit ContactGroups inside KPeople logic and want to fetch them separately, we might need both (i.e. an ETM for ContactGroups, QAbstractItemModel for Persons, and somehow merge them inside a single model). Sounds a bit weird, but might work. What do you think?

This will be a bit wider explanation so you get a better idea about how things work in KAddressBook. For the sake of simplicity, by "contact" I mean both KContacts::Addressee and KContacts::ContactGroup as this does not make any difference in the discussion here.

In KAB there's one central ETM, which contains a tree of addressbooks and contacts. The ETM is plugged into two proxy models: one that filters out the contacts and only keeps the addressbooks (let's call it CollectionFilterProxy) and one that flattens the ETM into a list and only displays contacts (let's call it ContactListProxy). This CollectionFilterProxy model is then plugged into the addresbook tree view (a QTreeView) on the left side of KAB window. The QTreeView has a QItemSelectionModel, which provides information about which items in the view are /selected/ (for example by their checkbox being checked). This QItemSelectionModel is plugged into another proxy model. Let's call it SelectionProxyModel. This proxy model sits on top of the CollectionListProxy and it filters out contacts that belong to collections that are not selected (through the QItemSelectionModel). This model is then plugged in to the contact list view in the middle of the KAB window.

Here's an attempt for a diagram on how this looks like:

                     [ ETM ]
                        |
            ,------------------------------,
            v                              v
[ CollectionFilterProxy ]         [ ContactListProxy ]
            |                              |
            |                              v
            |-->[SelectionModel]-->[ SelectionProxy ]
            |       ^                      |
            v       |                      v 
  [ AddressbookTreeView ]         [ ContactsListView ]

(Note that in reality there are more models involved, but this schema captures only those that are relevant to our discussion here).

What you propose is to get rid of the entire right side of the schema (except for the ContactsListView) and replace it with KPeople. I don't think that's a good idea, because it will be rather complicated to keep the filtering based on selected addressbooks. It is also an unnecessary overhead to have to Akonadi::Monitors and additional book-keeping that ETM already does for us. What I propose then is this:

                     [ ETM ]
                        |
            ,------------------------------,
            v                              v
[ CollectionFilterProxy ]         [ ContactListProxy ]
            |                              |
            |                              v
            |-->[SelectionModel]-->[ SelectionProxy ] --------> [ KPeople::AkonadiBackend ]
            |       ^                                                       |
            v       |                                                       v
  [ AddressbookTreeView ]         [ ContactsListView ] <---------- [ KPeople::Model ]

As you can see this removes the direct connection between ContactListView and ETM, but it keeps the backend connected to it, which still allows for various filter proxies to be put before that. The KPeople::Model will then be populated by KPeople::AbstractContacts from the AkonadiBackend. This will even simplify the backend code quite a lot, because you don't need to bother yourself with initial retrieval of contacts from Akonadi: they will already be in the source model, so you just iterate over all rows in the source model and create KPeople::AbstractContact for each row. Any other change: contact being added, removed or changed - whether this is due to sync with backend or because user applies some filter or because user checks/unchecks an addressbook in the tree view on the left - will be completely opaque to you. will then come to you through the regular QAbstractItemModel signals. In fact, this will make the backend almost Akonadi-free, only working with Akonadi::Items, but not having to do any direct interaction with Akonadi itself.

Does this sound sensible? You've looked into the code more closer, I only skimmed through it, so maybe there's something that would make this approach unfeasible and we will have to go with a dedicated Akonadi::Monitor in the backend. But the general rule is: try to avoid interacting with Akonadi directly in your code as much as you can, ETM is your friend :-)

We should wrap KContacts::ContactGroup into a single KPeople::AbstractContact the same way we do with KContacts::Addressee. ContactGroup is basically still just a contact: it has a name and multiple email addresses. In the contact list, this is fine. Yes, someone could try to "merge" two contact groups into one metacontact, but I *think* there's a way in KPeople to disallow merging conditionally (I'm not sure though).

I think KPeople was created with intention for Person to represent... Well, a person. Which might be a quantum superposition of various AbstractContacts. So making ContactGroups an AbstractContact and furthermore a Person feels just a bit weird to me. Anyway, as far as I can see, for now there is no "mergeable" flag, or "virtual" contacts not really representing a Person. But:

1. It shouldn't be too hard to add this functionality, but we'll have to touch KPeople code.
2. We might not need it, if we adopt your schema (if I'll find out how to do it). We can add something like a AddresseeProxy, which we'll feed to KPeople backend, and ContactGroupProxy, which we don't need to.

I think there may be a bit of misunderstanding about the cases we are addressing here.

For the contact list, the merged data provided by KPeople::MetaContact is enough, no problem here.
For the contact view (the big thing on the right where the full contact appears when you select it the contact list) the merged data from KPeople::MetaContact will not be enough - KPeople is not aware about geographical addresses, birthdays, occupations and all the crazy stuff that KContacts::Addressee can support, which we want to display in the contact view. Just try to create a local contact (not on Google, but add a new vCard addressbook) and try to fill in all the fields in the contact editor to get an idea about how much stuff there is.

Yeah, managing a contact list with minmial information is not a big deal. I was referring to a contact view. I'm just saying that it would be really nice if we implement this functionality inside KPeople, because it's sort of a purpose of this framework.

Actually, it might be doable. As I've written above, an AbstractContact is just a wrapper, which can return whatever we want for whatever properties we want (via contactCustomProperty, which returns a QVariant). In principle, we can expose everything from Addressee to AbstractContact, and make KPeople do its magic. That's hell a lot of properties, but might be doable. I can play with that and see how it works.

[...]
Does this sound sensible? You've looked into the code more closer, I only skimmed through it, so maybe there's something that would make this approach unfeasible and we will have to go with a dedicated Akonadi::Monitor in the backend. But the general rule is: try to avoid interacting with Akonadi directly in your code as much as you can, ETM is your friend :-)

Uhm...
On one hand, there are general purpose KPeople backends, which are supposed to be stand-alone plugins and might be used from whatever application people mightwant (which in principle might not be even aware of EntityTreeModel). That thing is already implemented and working (well, almost), it has its own Akonadi::Monitor and so on. PersonsModel then uses static PersonPluginManager class, which loads all those general purpose plugins. Model then creates AllContactsMonitors for them, retrieves all the data, and does the managing. There is currently no way to pass an EntityTreeModel to our plugin.

On the other hand, it shouldn't be hard to create custom BasePersonsDataSource subclass, which would wrap around an ETM. We won't announce it as a general purpose plugin, it will just serve our purpose. Then we need to somehow substitute an "akonadi" general purpose plugin by ours, custom one. It cannot be done without changing KPeople code, but it might be done without breaking existing functionality.
I propose to introduce a PersonPluginManager::setCustomDataSource(QString name, BasePersonsDataSource* dataSource), to which we'll feed our custom DataSource. And then, when PersonPluginManager loads plugins, it can just load general purpose plugin if one was not set before.

It won't break existing code, but will allow us to use ETM. Needs some coding though
(in principle, we don't even need a general purpose plugin then; since no one uses it - there's not much point in maintaining it. Or we can leave both and hope it might come in handy some day. Or we can merge them in a single one, and then for general purpose we will just create an ETM if we were not given one)

KPeople also does not export PersonPluginManager (i.e. headers), its purely private and used only inside the framework.

We should wrap KContacts::ContactGroup into a single KPeople::AbstractContact the same way we do with KContacts::Addressee. ContactGroup is basically still just a contact: it has a name and multiple email addresses. In the contact list, this is fine. Yes, someone could try to "merge" two contact groups into one metacontact, but I *think* there's a way in KPeople to disallow merging conditionally (I'm not sure though).

I think KPeople was created with intention for Person to represent... Well, a person. Which might be a quantum superposition of various AbstractContacts. So making ContactGroups an AbstractContact and furthermore a Person feels just a bit weird to me. Anyway, as far as I can see, for now there is no "mergeable" flag, or "virtual" contacts not really representing a Person. But:

• It shouldn't be too hard to add this functionality, but we'll have to touch KPeople code.

A ContactGroup does not map to a MetaContact: it's not a merge of multiple contacts. Hence the only think we are left is is an AbstractContact. Introducing an AbstractContactGroup into KPeople would only make the KPeople code unnecessary complicated to represent a very special case for a single particular usecase (ours :)). It's not hard to map ContactGroup to an AbstractContact - if you just forget about the name of the class, a ContactGroup is nothing more than a a single contact with multiple email addresses (that happen to belong to different people instead of a single person).

• We might not need it, if we adopt your schema (if I'll find out how to do it). We can add something like a AddresseeProxy, which we'll feed to KPeople backend, and ContactGroupProxy, which we don't need to.

Let's try not to have to merge two models together :)

...

...
Actually, it might be doable. As I've written above, an AbstractContact is just a wrapper, which can return whatever we want for whatever properties we want (via contactCustomProperty, which returns a QVariant). In principle, we can expose everything from Addressee to AbstractContact, and make KPeople do its magic. That's hell a lot of properties, but might be doable. I can play with that and see how it works.

Sure, we could expose all of the KContacts::Addressee properties through contactCustomProperty for third-party applications. However since there's no-one else using KPeople right now and since I'm not a fan of magical string properties, I would much more prefer to expose the actual KContacts::Addressee object through one of the contactCustomProperty, so that callers can do for example

const auto addresee = abstractContact->contactCustomProperty(QStringLiteral("addressee")).value<KContacts::AddresseePtr>();
... = addressee->address(KContacts::Address::Home);

to get a contact's address without having to guess the name of the string property ("home-address", "address-home", "homeAddress" ....?)

Uhm...
On one hand, there are general purpose KPeople backends, which are supposed to be stand-alone plugins and might be used from whatever application people mightwant (which in principle might not be even aware of EntityTreeModel). That thing is already implemented and working (well, almost), it has its own Akonadi::Monitor and so on. [....]
On the other hand, it shouldn't be hard to create custom BasePersonsDataSource subclass, which would wrap around an ETM. [...] I propose to introduce a PersonPluginManager::setCustomDataSource(QString name, BasePersonsDataSource* dataSource), to which we'll feed our custom DataSource. And then, when PersonPluginManager loads plugins, it can just load general purpose plugin if one was not set before.
Needs some coding though (in principle, we don't even need a general purpose plugin then; since no one uses it - there's not much point in maintaining it. Or we can leave both and hope it might come in handy some day. Or we can merge them in a single one, and then for general purpose we will just create an ETM if we were not given one)

Regarding the current existing plugin in KPoeple, I'm all for removing it from there for good - nobody can ever use it, because KPeople can't depend on Akonadi. It's there because initially, KPeople was part of KTp, which allowed for such dependency. I think was a good starting point but long term we want something better :)

You are right that injecting the ETM into the backend plugin is non-trivial as it's a plugin. I like your idea about instantiating the BasePersonsDataSource ourselves and injecting it into PersonPluginManager. Architecturally it would probably be bast to have a PersonDataSourceManager as a public API which gets internally populated through PersonPluginManager but also offers API to add/remove data sources manually through API. Right now we really only want to use the Akonadi backend, and maybe allow KTp at some point and then, if more sources appear in the future, we may need to implement support for them first in KAB before we start displaying their data. So having a manual control over the data sources is a good idea.

You are right that injecting the ETM into the backend plugin is non-trivial as it's a plugin. I like your idea about instantiating the BasePersonsDataSource ourselves and injecting it into PersonPluginManager. Architecturally it would probably be bast to have a PersonDataSourceManager as a public API which gets internally populated through PersonPluginManager but also offers API to add/remove data sources manually through API. Right now we really only want to use the Akonadi backend, and maybe allow KTp at some point and then, if more sources appear in the future, we may need to implement support for them first in KAB before we start displaying their data. So having a manual control over the data sources is a good idea.

Actually, I don't think we need two entitites in the KPeople: PersonPluginManager actually manages DataSources. If we split those, there won't be much left inside PersonPluginManager (i.e. single call to KPluginLoader::findPlugins() and loading those). So I propose simply making PersonPluginManager public, and adding static method setDataSource there (we might also want something like PersonPluginManager::setLoadSystemDataSourcePlugins(bool load) to explicitly show KPeople we only want to work with our plugin).

OK, I did those changes inside KPeople, so I was able to feed it with our custom AkonadiDataSource, which I started to implement (based on ETM - I used GlobalContactModel adopted from KAddressBook).
Now I have the same issue I've began with. Inside the model, there are duplicated contacts. It's fairly easy to filter those inside QAbstractItemModel::rowsInserted signal and don't add them multiple times, but we clearly don't want to remove them when we catch QAbstractItemModel::rowsRemoved signal (since it might be just a single item out of ten duplicates).
The only thing that comes to my mind is refcounting, but something inside my head says that it's not right. Or is it?

Awesome, you rock! Feel free to upload your KPeople changes to Phabricator for review (and link it from here or so).

Regarding the duplicated contacts I have to admit I fell behind a little and did not yet start to modify the Google Contacts resource as we agreed above. I'll jump at it tonight and make branch with Google Contacts resource having only a single Collection and using Tags for groups instead of the virtual collections. This will solve the problem with contacts being duplicated in the ETM.

In T9595#159788, @dvratil wrote:

Awesome, you rock! Feel free to upload your KPeople changes to Phabricator for review (and link it from here or so).

Thanks! I've posted it: D15431: Make PersonPluginManager API public.

Regarding the duplicated contacts I have to admit I fell behind a little and did not yet start to modify the Google Contacts resource as we agreed above. I'll jump at it tonight and make branch with Google Contacts resource having only a single Collection and using Tags for groups instead of the virtual collections. This will solve the problem with contacts being duplicated in the ETM.

Anyway, there could be other plugins with virtual collections leading to duplicate contacts inside EntityTreeModel, and we definitely don't want our code to break on that case. So, whatever, I've added refcounting code, it seems to be working (it needs some testing for sure, but that's for later).

I've decided to make a scratch repo and to publish there what is done so far. It includes ported viewer (based on 2014-dated GSoC project), which is a bit broken, but whatever - it still can be used for debugging.
TODO: it would be nice to view all internal data of AbstractContacts there; as well as fancy tree-view with metacontacts & contacts. But that's for the future.

So far so good: it works, contacts get fetched, updated and removed correctly. Yay!
There's still a lot to be done, though.

Just some thoughts:
Apparently, when the KPeople was developed, they actually had a nice plugin system used for viewing: PersonDetailsView loads available plugins installed system-wide, which are able to show different data (i.e.: dedicated plugins to display name, emails, phones, chat log, etc.).
Maybe we can utilize it somehow, i.e. just write own plugin for viewing KContacts::Addressee and then just use PersonDetailsView in KAddressBook?

It's all QWidget-based, though. Hard to combine with HTML/Grantlee, which is used inside KAB :(

Apparently, when the KPeople was developed, they actually had a nice plugin system used for viewing: PersonDetailsView loads available plugins installed system-wide, which are able to show different data (i.e.: dedicated plugins to display name, emails, phones, chat log, etc.).

This was mostly done for the use case in KDE Telepathy, where you can display "Contact Info" dialog for each contact.

Maybe we can utilize it somehow, i.e. just write own plugin for viewing KContacts::Addressee and then just use PersonDetailsView in KAddressBook? It's all QWidget-based, though. Hard to combine with HTML/Grantlee, which is used inside KAB :(

I don't know. For now, I'd prefer to leave the contact viewer in Grantlee (since we already have all the code to display KContacts::Addressee) and aim tentatively at porting it to QML rather than PersonDetailsView.

Sorry for long delay, was busy with other stuff...

OK, I've modified my test application so it now has the similar behavior to KAddressBook. Namely, all those proxies works, with ability to select addressbooks to be used, etc.
Now it crashes when I try to show a contact, but the crash is somewhat KTp-related, have to look into it. It also crashes due to bug in KPeople, which I tried to address in D15812: Don't crash if person gets removed.

ContactGroups are yet to be implemented, and also the main feature - i.e. contact merging - still has to be tested.

Merge/Unmerge functionality appeared to be fairly simple, it's done via KPeople::mergeContacts(QStringList &uris) and KPeople::unmergeContact(QString uri). It kinda already works (as simple as that), see the repo.

However, there is apparently no simple way to access the subcontacts. We can get their IDs via PersonData::contactUris(), but if we try to access such uri i.e. via PersonData(uri), we end up having parent contact.
Yet we might need it in KAB, because we would like to expose it to user (I guess). Another place where we need it is i.e. when editing contact - we should ask user which one he would like to modify.
I'll think about convenient API...

Isn't the URI just an Akonadi ID? That should be enough to retrieve the Item from the ETM manually (build ID-only Item with Item::formUrl() and pass it to EntityTreeModel::modelIndexForItem() which will give you QModelIndex for the actual fully-populated Item) - for the editor and viewer you want to manipulate the Akonadi Item anyway, not the KContacts::Contact itself.

Great job btw, thanks for fixing the issues along the way!

Just fyi, I'm still working on the improved Google Contacts resource, but I've got caught by the fact that the tags system in Akonadi is not much used and thus kinda broken, so I had to fix that first :-D

So far merging works, but occasionally it crashes (didn't find reliable way to reproduce). It also works weirdly when I try to merge two (already) metacontacts.
Interestingly enough, it doesn't crash the testing application, but plasmashell. My guess that it's due to KTp Instant Messanging plasmoid, which interacts with Plasma.
Will investigate...

In T9595#162009, @dvratil wrote:

Isn't the URI just an Akonadi ID? That should be enough to retrieve the Item from the ETM manually (build ID-only Item with Item::formUrl() and pass it to EntityTreeModel::modelIndexForItem() which will give you QModelIndex for the actual fully-populated Item) - for the editor and viewer you want to manipulate the Akonadi Item anyway, not the KContacts::Contact itself.

Currently, the KPeople plugin operates with Akonadi::Items (there is no code to fetch it yet, but it's simply two lines of code), so it's fine.
So in principle - yes, we can fetch those from ETM. I just thought that since this KPeople is already aware of this information - why don't just use it?

Great job btw, thanks for fixing the issues along the way!

Thanks :)

Just fyi, I'm still working on the improved Google Contacts resource, but I've got caught by the fact that the tags system in Akonadi is not much used and thus kinda broken, so I had to fix that first :-D

Cool, that's great news! ( not about the broken stuff, but the stuff that gets repaired :] )

I've implemented ContactGroup support, so now the plugin seems to be fully functional, so I'm switching to KAddressBook side of the story. Apart from adopting View to work with PersonModel, we also need few other stuff.

• As far as I understood, we want to ship the plugin with akonadi-contacts. Yet if we want to feed an EntityTreeModel from KAddressBook, we need AkonadiDataSource to be accessible from there. That would mean that we need to:
  • Export those headers & link KAddressBook to plugin library. Sounds a bit weird to me, those plugins are supposed to be runtime dependencies, and we shouldn't break if those are removed.
  • Or we can just ship that plugin with KAddressBook. Which is also a bit weird, since it's a general-purpose plugin, which in principle can be used within any application (if there will be anything that supports KPeople...)
  • We can also add another API for PersonPluginManager to feed arguments (QVariantList args) to a plugin (we don't know the dataSourceId before loading it, but we know KPluginLoader::pluginName, which I believe should be akonadi_kpeople_plugin), without having an access to a class itself.
• UI for contacts merging and unmerging (a simple button in context menu / toolbar?)
• Code to merge several KContacts::Addressee into a single one, to feed it to the Contact Details Viewer when user selects a metacontact
  • (?) Might be useful to be implemented inside KContacts framework
  • (?) In principle, several contacts might have different names / avatars (and other unique fields), and it would be nice if user could specify which one he would like to use for metacontact. For example, KPeople does it basically randomly - first contact that appears in its internal list is returned. Don't really now how to do it the simple way.
• UI for editing a metacontact. I guess we can just ask user which of the subcontacts he wants to modify, and then just show him existing dialog for a single contact.
  • An "Edit [...]" item for each subcontact in the context menu?
  • A separate dialog that asks user which of subcontacts he would like to modify? (gosh, I hate UIs which show dozens of nested sub-dialogs...)
  • A drop-down menu (or any other UI element) inside an already existing Contact Edit Dialog, that allows user to specify the subcontact?
• UI for duplicates suggestions - that's a must-have feature (based on names / phones / emails, maybe something else). There is some UI for it inside KPeople, but I haven't looked into it yet.

What do you think about all that?

Cool! Just tried playing around with the test app a little, looks pretty good :)

• As far as I understood, we want to ship the plugin with akonadi-contacts. Yet if we want to feed an EntityTreeModel from KAddressBook, we need AkonadiDataSource to be accessible from there. That would mean that we need to:
  • Export those headers & link KAddressBook to plugin library. Sounds a bit weird to me, those plugins are supposed to be runtime dependencies, and we shouldn't break if those are removed.

We don't need to link a plugin library, we just make the class a part of the AkonadiContacts library. Call it Akonadi::KPeopleDataSource or something like that and make it part of akonadi-contacts public interface. Depending on KPeople from akonadi-contacts is not a problem either since it's a KDE Framework (and we already depend on most of them anyway :))

• UI for contacts merging and unmerging (a simple button in context menu / toolbar?)

Yes, an action in contact's context menu that's available only when multiple contacts are selected (adding the same QAction to toolbar is trivial thanks to KXmlGui).

• Code to merge several KContacts::Addressee into a single one, to feed it to the Contact Details Viewer when user selects a metacontact
  • (?) Might be useful to be implemented inside KContacts framework

The templates and base code for this in KAddressBook right now, so I think we should add it there for now - generally, I'm not opposed to moving this to moving the entire viewer code to akonadi-contacts, but that's out-of-scope for this task. KContacts is not a good target for this, it's supposed to be domain-specific library (implementing the respective RFCs).

*UI for duplicates suggestions - that's a must-have feature (based on names / phones / emails, maybe something else). There is some UI for it inside KPeople, but I haven't looked into it yet.

There's also the "Search duplicate contacts" action in KAddressBook menu which is a part of the "Merging" plugin probably (don't ask me why it's a plugin), maybe the UI could be repurposed for that.

• UI for editing a metacontact. I guess we can just ask user which of the subcontacts he wants to modify, and then just show him existing dialog for a single contact.
  • An "Edit [...]" item for each subcontact in the context menu?

Edit action with a submenu for individual contacts

• A separate dialog that asks user which of subcontacts he would like to modify? (gosh, I hate UIs which show dozens of nested sub-dialogs...)

You double-click a merged contact, you get a dialog with a list of the subcontacts, you select the one you want to edit - I don't see anything problematic here (and I have no idea how to do it better)

• A drop-down menu (or any other UI element) inside an already existing Contact Edit Dialog, that allows user to specify the subcontact?

I would avoid touching the contact editor right now.

@poboiko hi Igor, how's the progress on this? Do you need any help?

Sorry, I've postponed this one for some time :(

I hope I'll be able to get back to this next week