Review Request 112987: Use nco:contactUID instead of nco:imID to identify a contact [part 2/2]

[David Edmundson]

> On Sept. 29, 2013, 3:24 a.m., David Edmundson wrote:
> > KTp/im-persons-data-source.cpp, line 84
> > <http://git.reviewboard.kde.org/r/112987/diff/1/?file=193067#file193067line84>
> >
> >     this can crash.
> >     
> >     if the account is offline, accountForContact won't work, at best it will return a null accountptr, which will then crash.
> >     
> >     This is important as it is called from onContactInvalidated.
> 
> Daniele E. Domenichelli wrote:
>     I'm not sure what to return here when the account is null... something like ktp://invalid_account#<contact_id> would work?
>     Can the contact be invalid as well?

no, you _need_ it to return the correct account ID otherwise you won't delete the contact from the hash map correctly.

It will return the wrong account because the code for matching contacts --> account is 

foreach(account, allAccounts) {
  if (account->connection == contact->contactManager->connection) {
    return account;
  }
}

This works whilst you're online, but as soon as you're offline both of those are null. 
(EDIT: from the above snippit it makes it sound like that will work, but we have a check for nulls and just return a null AccountPtr)

It's pretty terrible.
I guess we need to rethink how we do that.

Long term a connection->account() method in TpQt will be ideal. 

Short term, we either need to not do this call here, or we need a better hack for doing accountForContact. Maybe a hashmap inside GlobalContactManager?


- David


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


On Sept. 29, 2013, 12:35 a.m., Daniele E. Domenichelli wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112987/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2013, 12:35 a.m.)
> 
> 
> Review request for Telepathy, David Edmundson and Martin Klapetek.
> 
> 
> Repository: ktp-common-internals
> 
> 
> Description
> -------
> 
> Use nco:contactUID instead of nco:imID to identify a contact
> 
> NOTE: Changes in this review request https://git.reviewboard.kde.org/r/112970/ are required for this to work
> 
> 
> Diffs
> -----
> 
>   KTp/im-persons-data-source.cpp ebd01bae2f029ba5335c99babedec31b88cd7620 
>   KTp/Models/kpeopletranslationproxy.cpp a9bce378271006a9003c7757e49ba5b76ea97223 
> 
> Diff: http://git.reviewboard.kde.org/r/112987/diff/
> 
> 
> Testing
> -------
> 
> Tested with contact list, changed presence a few times.
> No more duplicates, presence works as expected
> 
> 
> Thanks,
> 
> Daniele E. Domenichelli
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20130930/3d125e34/attachment-0001.html>