Hey,<br><br><div class="gmail_quote">On Sun, Mar 10, 2013 at 12:07 PM, Vishesh Handa <span dir="ltr"><<a href="mailto:me@vhanda.in" target="_blank">me@vhanda.in</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hey Martin<br><br>As discussed in the PIM Sprint - I've been reviewing the kpeople code so that we can get it ready by the end of march for the first release. I've already made some basic changes, but there are still big changes to be made -<br>
<br>1. Ontologies - Who is responsible for installing the ontologies? Considering that the telepathy nepomuk feeder is pushing the properties maybe we should only let it install the ontology? We do have a run-time dependency on the feeder.<br>
</blockquote><div><br></div><div>Actually we don't use any Tp ontologies in kpeople, just NCO and PIMO. So the obvious question is - do we need the Telepathy ontologies at all?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2. Rename PersonModelContactItem to ContactItem and PersonModelItem to PersonItem? We can currently make them abstract enough to reuse them in other places such as PersonData, so they won't exactly be tied to the model.<br>
</blockquote><div><br></div><div>Ok.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">3. Clear Separation Between Nepomuk and kpeople - I know that kpeople relies on Nepomuk heavily, but we need to have clear boundaries of how Nepomuk components are going to be used. In case it's not apparent I'm referring to the use case of using Nepomuk Ontologies as keys.<br>
<br>a. Their is a QHash<QString, QUrl> which maps the sparql variables to their respective properties. This hash is constructed by using a regex and modifying the property names.</blockquote><div><br></div><div>It's not really a regex, simple string parsing rather.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> It just happens that the property names turn out to be the exact same as those in the queries. Eg -<br>
<br> <a href="http://www.semanticdesktop.org/ontologies/2007/03/22/nco#emailAddress" target="_blank">http://www.semanticdesktop.org/ontologies/2007/03/22/nco#emailAddress</a> -> nco_emailAddress<br></blockquote><div>
<br></div><div>This does not "just happen", that's on purpose :)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">It might be simpler to construct all the mappings ourselves - hash.insert( NCO::emailAddress(), "nco_emailAddress" ); That way we can avoid using such long binding names if we want.</blockquote>
<div><br></div><div>The original idea was that there would be different "facets" for different usages - IM, PIM etc - and each would have defined a set of properties (ontologies) it is interested in and the query was supposed to be built up from that, but that happened only half-way as you can see.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">b. We shouldn't be using Nepomuk properties as keys - In the ContactItem we have a QMultiHash<QUrl, QVariant>. They QUrl over here is a nepomuk property. When we are asked of a particular role say emailAddress we translate it to its corresponding property - nco:emailAddress and then return the value. This is confusing cause nco:emailAddress has a domain of a nco:EmailAddress not of a Contact. Ditto in the case of contact group names.<br>
</blockquote><div><br></div><div>I don't think this is necessarily a problem on the client side though.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>Instead it would be better to have a QHash<Role, Variant> and directly use the Roles as keys, that way we wouldn't even have to have switch case statements.<br><br>The hash in (a) would then be of the form QHash<Role, QString>. And we could manually map the binding names to the roles - hash.insert( EmailRole, "nco_emailAddress" ).<br>
</blockquote><div><br></div><div>That does sound sensible. And I like it.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>With this the only Nepomuk code in the model would be that of the query. A new comer trying to contribute would find it a LOT simpler, cause now they don't need to understand the ontologies and wonder why they are being mapped around.<br>
</blockquote><div><br></div><div>Tbh. I don't want anyone not understanding the ontologies around ;)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
c. If (b) is implemented, then we cannot use the Nepomuk Resource class as an argument to the ContactItem. It would be a lot simpler to just issue a small query and fetch the required properties and map them with the hash described above.<br>
<br>4. PersonsData - It can just be a light wrapper over PersonItem and ContactItem. They already have all the data. No point duplicate code.<br></blockquote><div><br></div><div>As David pointed out - this is a nice API wrapper-class around one single contact. And I like it that way. Mixing it with ContactItem would be a mess.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">5. Loading properties with cardinality > 1 - In the PersonModel you currently have a check to make sure the same contact uri is not returned twice - They will get returned twice if the contact contains multiple emails/photos/groups - Each time with a different combination of any of those 3. We need to handle that case as well.<br>
<br>It seems quite simple. We can have a list of bindings whose cardinality > 1 and insert only those if the uri appears twice.<br></blockquote><div><br></div><div>Ok, I will take a look.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
6. ResourceWatcher - The watcher will fail for the cases when a contact group, email, or photo is changed. Anything that is not a direct property of the nco:PersonContact - Mainly cause it will return -<br>
<br>propertyChanged - nco:photo<br>from: nepomuk:/res/oldUri<br>to: nepomuk:/res/newUri<br><br>and the code does this - hash.replace( prop, to ); which will replace file://location/something to nepomuk:/res/newUri. Also if we go ahead with step 3 (b), then we will need to map these properties to the roles. I'm not sure if this is a good idea. Maybe we should just drop live updates for now? Not sure.<br>
</blockquote><div><br></div><div>Right, this is a problem. I don't want to drop live updates. We need smarter updates handling.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
7. Realtime loading of the contact list - Currently the query constructs all the ContactItems when iterating over the query results, but it only adds them at the end cause we need to group them properly. It might be better from a user point of view to add them the moment they are constructed. That way the user gets to see the contact list being populated instead of having to stare at a blank screen for some time. In my case it is about 7 seconds. Once the query is finished, then the contacts can be re-arranged into people.<br>
</blockquote><div><br></div><div>I'm against this. I've tried this with the per-contact query and it just looks horrible.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
8. ContactType and ContactTypeRole - What is this? It's not used anywhere. Also how can a contact be of a single type - Email, IM, Phone, MobilePhone or Postal? Can't it have a combination of these 5? What do you want to do about this?<br>
</blockquote><div><br></div><div>This is an old-ish relic, needs to be removed completely.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">9. Merging Contacts into People - The current code is quite wrong as it uses mergeResource - We just need to group them under the same person, not merge them into. I think we discussed this during the pim sprint.<br>
</blockquote><div><br></div><div>No, you missed the correct methods :) See PersonsModel::createPersonFromIndexes().</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
10. Unmerging - In that case we want to delete the pimo:Person if it only consists of one nco:PersonContact<br></blockquote><div><br></div><div>Yup, needs checking - if (Person.groundingOccurances().size() == 1) remove Person;</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>11. Automatic Merging - Let me know if you want me to look into that code. I haven't really taken a look. From what my notes say, we had marked this as optional for the next release.<br>
</blockquote><div><br></div><div>Later stuff.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>----<br><br>I'm willing to implement most of the points above. Points 1-5 to quite simple and shouldn't take more an hour or two + testing. <br></blockquote><div><br></div><div>Perfect. Let's get on with 3 and 6, the rest is quite easy to do.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>Let me know what you think. We have about 3 weeks.</blockquote><div><br></div><div>David - Yes, the old plan still holds - release separately after a week or more after 0.6 is released.</div>
<div><br></div></div><div>Cheers</div>-- <br><div><span style="color:rgb(102,102,102)">Martin Klapetek | KDE Developer</span></div>