Review Request: Synchronize tp-integration-daemon with subscription/publication states and newly added contacts

[Dario Freddi]

> On 2010-04-05 14:41:10, George Goldberg wrote:
> > In general, this all looks fine to me (usual disclaimer - I haven't compiled or tested it yet). 
> > 
> > The one area that seems to be a bit problematic is that when the TelepathyContact constructor is run, and doNepomukSetup() method is called. I see that you call the slots to handle subscription state and blocked status after calling doNepomukSetup(). Could perhaps some of the redundant code in doNepomukSetup() (at least the bit creating the isBuddyOf relationship) be removed?
> 
> Dario Freddi wrote:
>     Good catch - it probably is. I'll check it out later.
> 
> Dario Freddi wrote:
>     I added it to my local copy, but git-svn is not really rb-friendly. Consider that as addressed, though.

After the comments below, is this patch good to get in as well?


> On 2010-04-05 14:41:10, George Goldberg wrote:
> > /trunk/playground/network/telepathy-integration-daemon/telepathycontact.cpp, line 355
> > <http://reviewboard.kde.org/r/3468/diff/2/?file=22396#file22396line355>
> >
> >     Is there a bug report against Nepomuk for that? Or is it not as simple as that?
> 
> Dario Freddi wrote:
>     I don't know - I really wanted to check as usual if it was me doing something totally wrong or not.
> 
> George Goldberg wrote:
>     Looks to me like it should work, so best to check with #nepomuk-kde people.

Ok


> On 2010-04-05 14:41:10, George Goldberg wrote:
> > /trunk/playground/network/telepathy-integration-daemon/telepathycontact.cpp, line 364
> > <http://reviewboard.kde.org/r/3468/diff/2/?file=22396#file22396line364>
> >
> >     Umm, I'm not sure, for now. Let's leave a FIXME there and figure out what to do later.
> >     
> >     It'll probably be some way for the UI to know the subscription state is ASK because it'll want to have some "resend subscription request" button.
> 
> Dario Freddi wrote:
>     Ok - in this case we probably also have to notify the UI about capabilities (are you able to cancel a subscription request, etc).
> 
> George Goldberg wrote:
>     Yeah. Let's leave that for now, but file a bug report or make a note or something so this issue doesn't get forgotten·

I'll leave a TODO in the code, maybe with a more explainatory description.


> On 2010-04-05 14:41:10, George Goldberg wrote:
> > /trunk/playground/network/telepathy-integration-daemon/telepathycontact.cpp, line 91
> > <http://reviewboard.kde.org/r/3468/diff/2/?file=22396#file22396line91>
> >
> >     m_contact->publishState() would do I think.
> >     
> >     Same applies to the two lines below.
> 
> Dario Freddi wrote:
>     Are we using this as "coding style"? Here I'm following the latest trend in Qt API, aka QWeakPointer, where operator-> is no longer defined and you explicitely have to access the pointer using .data(). I can switch back using -> though.
> 
> George Goldberg wrote:
>     I hadn't thought of that - to be honest I don't think it matters, although I personally perfer the shorter version, and since TpQt4 doesn't use QWeakPointers in its API, this would do fine. I'll leave it up to you whether you care enough to change them ;)

Ok, I'll try to change them back to -> then :)


> On 2010-04-05 14:41:10, George Goldberg wrote:
> > /trunk/playground/network/telepathy-integration-daemon/telepathycontact.cpp, line 325
> > <http://reviewboard.kde.org/r/3468/diff/2/?file=22396#file22396line325>
> >
> >     I'm not sure what the possibilities are here. Can you explain please?
> 
> Dario Freddi wrote:
>     Basically - do we want to record if the contact is no longer subscribed to us? Random example: in MSN, if a contact unsubscribes from you but still does not block you, you can see in your client (if it supports it, like KMess) "This contact has no longer you in his contact list".
> 
> George Goldberg wrote:
>     That would be a nice feature.
>     
>     Overall, I believe the possible things we can record are: 
>     - Is Contact Blocked?
>     - Can I see the Contacts Presence?
>     - Is the contact subscribed to me?
>     - What outstanding invitations are there?
>     
>     That list is off the top of my head - it would be necessary to check with the spec (and probably #telepathy wizards) to find out exactly what stuff like this is supported, and whether yes/no is enough or if yes/no/unknown is needed (where e.g. protocol X doesn't support bool Y).

I can't think about anything else to record as well - about seeing what is supported, IIRC tp-qt4 can already give out account's capabilities - we probably want to map them in the API of the other review request.


- Dario


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


On 2010-04-05 22:38:42, Dario Freddi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3468/
> -----------------------------------------------------------
> 
> (Updated 2010-04-05 22:38:42)
> 
> 
> Review request for telepathy and George Goldberg.
> 
> 
> Summary
> -------
> 
> This patch makes tp-integration-daemon pick up the changes done at a subscription/publication level to telepathy contacts. Bottom line: now adding/removing contacts from tp-contactlist no longer screws up your nepomuk database.
> 
> Please note that tp-contactlist won't pick up changes in realtime: you have to restart it to see the effect. This is because tp-contactlist does not watch nepomuk resources for changes (my next target, after this patch and the other one will make it in)
> 
> 
> Diffs
> -----
> 
>   /trunk/playground/network/telepathy-integration-daemon/telepathy.trig 1111532 
>   /trunk/playground/network/telepathy-integration-daemon/telepathyaccount.h 1111532 
>   /trunk/playground/network/telepathy-integration-daemon/telepathyaccount.cpp 1111532 
>   /trunk/playground/network/telepathy-integration-daemon/telepathycontact.h 1111532 
>   /trunk/playground/network/telepathy-integration-daemon/telepathycontact.cpp 1111532 
> 
> Diff: http://reviewboard.kde.org/r/3468/diff
> 
> 
> Testing
> -------
> 
> Works, as shown in tp-contactlist.
> 
> 
> Thanks,
> 
> Dario
> 
>