Review Request: Contact Avatar support for Nepomuk Service

[Daniele Elmo Domenichelli]

> On June 15, 2011, 11:45 p.m., Daniele Elmo Domenichelli wrote:
> > telepathy.trig, line 33
> > <http://git.reviewboard.kde.org/r/101633/diff/1/?file=24529#file24529line33>
> >
> >     Another issue is that avatar can be changed, but just dropping this property means that it will be impossible to search for all the pictures that have been avatar for a certain account.
> >     To fix this, we could have a generic avatar property and a currentAvatar subProperty, but perhaps this is useless, what do you think?
> 
> George Goldberg wrote:
>     I don't think Nepomuk generally stores the history of any other property in this way, so I don't think it's appropriate to do that for an Avatar. However, I don't really know how Nepomuk takes care of that. Or does it even take care of history of properties?

I don't think that it should store the history of the property, but the history of the file containing the avatar, in the same way as we will store information about the contact that sent us a file...
Therefore if the avatar is cached/saved somewhere, also the information that tell us that that file was an avatar for a person should be stored.

Another way to solve this could be to have the avatar (or avatarForIMAccount) property with maxcardinality 1 from nco:IMAccount to nie:DataObject (or in the opposite direction) AND another avatar property from pimo:person to nie:DataObject (or in the opposite direction), that could have cardinality > 1 since telepathy allows just one avatar per account, but there is no reason why a person should have just 1 avatar.


> On June 15, 2011, 11:45 p.m., Daniele Elmo Domenichelli wrote:
> > nepomuk-storage.cpp, lines 765-766
> > <http://git.reviewboard.kde.org/r/101633/diff/1/?file=24528#file24528line765>
> >
> >     is this Q_ASSERT necessary?
> 
> George Goldberg wrote:
>     Yes. If it gets triggered that's a bug, and the best way to make developers notice is a crash in debug mode :p

I should have read the documentation of Q_ASSERT before commenting this :)


- Daniele Elmo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101633/#review3916
-----------------------------------------------------------


On June 15, 2011, 6:16 p.m., George Goldberg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101633/
> -----------------------------------------------------------
> 
> (Updated June 15, 2011, 6:16 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> This patch reinstates support for Contact Avatars in the Telepathy Nepomuk Service.
> 
> Avatar support for local accounts looks trickier so will come in a subsequent patch.
> 
> I'd particularly like comments on the changes to the Telepathy ontology. The rationale behind adding the telepathy:avatar property to nco:IMAccount is that although we can use nco:photo, this can contain multiple photos of the contact and we need to be able to identify the one that is the avatar for this IM account. The reason for it not being upstream is that it seems like telepathy-level implementation detail to me (although I will of course discuss this with upstream in due course to see whether they think it might belong there). The result is you are meant to identify one particular nco:photo of the nco:personContact to be the telepathy:avatar.
> 
> 
> This addresses bug 270864.
>     http://bugs.kde.org/show_bug.cgi?id=270864
> 
> 
> Diffs
> -----
> 
>   abstract-storage.h 7d1be9b96711500ddd2ab5b6d9aa8791e51bcea1 
>   account.h b57ebf881e1bd661bd8db069cc3f5147ce10b515 
>   account.cpp e0cfdf0931c75126339e18fb2458a93d510b6e28 
>   contact.h ccc482aa36c3d8e38cfa89608043f198ef8be674 
>   contact.cpp bc5a29100e380bbcb7ae48930813d19842b0a836 
>   controller.cpp f320326534c8b76ea73097c906d82c2b90a35f22 
>   nepomuk-storage.h 216239371b0656dff61c4a189a709703094b7418 
>   nepomuk-storage.cpp 7d154494b759e2b9bca95d086f164f1a71ab1095 
>   telepathy.trig d5a07d202185cc0d9aaa0fbd681cfd0202fa88c1 
>   tests/controller-test.h cd85a675d731b43c0960e0b5ba3fb02d8aacade7 
>   tests/storage-test.h ab5f05f2e5faf6b0710c2dfd9f8d3f21ae06d330 
>   tests/storage-test.cpp 2ff9428f57f2515c60c9fe7fb24c46d9f6907b7d 
> 
> Diff: http://git.reviewboard.kde.org/r/101633/diff
> 
> 
> Testing
> -------
> 
> Unit tests (updated appropriately) all pass.
> 
> 
> Thanks,
> 
> George
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-telepathy/attachments/20110618/93ca416d/attachment.htm