[Kde-pim] Review Request 113180: kmail: do not duplicate contacts when saving distribution list

[Kevin Krammer]

> On Oct. 13, 2013, 3:53 p.m., Kevin Krammer wrote:
> > messagecomposer/recipient/distributionlistdialog.cpp, line 206
> > <http://git.reviewboard.kde.org/r/113180/diff/2/?file=200984#file200984line206>
> >
> >     Hmm, I wonder if we need to iterate at all.
> >     I mean we are not creating new DistributionListItem instances for each, right?
> >     If we only need one hit, maybe we could even limit the search job to one result?
> 
> Jonathan Marten wrote:
>     I wasn't sure about this either, but having thought about it the situation could potentially arise.  A case from my own address book: there are some couples who have 2 distinct entries (because they have their own mobile phone number, work details etc) but who share a home email address.  The initial query will return both their entries (because the email address matches), and the list confirmation will be populated with both of them but with only the first entry checked.  The list can then be adusted by selecting/deselecting to suit before finally creating the list.
>     
>     It's not a particularly complex or expensive operation to support the multiple entries, so we should really leave it in to cover all cases.
>
> 
> Jonathan Marten wrote:
>     Have just looked at the code again, and would need to rewrite the loop to cover the above case.  Will investigate.
>

Yes, that makes a lot of sense. Maybe check the file's history, the item creation might have accidentally been moved out of the loop.


- Kevin


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


On Oct. 13, 2013, 3:28 p.m., Jonathan Marten wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113180/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2013, 3:28 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Bugs: 325303
>     http://bugs.kde.org/show_bug.cgi?id=325303
> 
> 
> Repository: kdepim
> 
> 
> Description
> -------
> 
> When saving a distribution list from the KMail composer, each recipient address is searched to see if they already exist.  As described in the referenced bug and at http://lists.kde.org/?l=kde-pim&m=138020586810635&w=2, if an address is not found a new contact is added to the Akonadi database and the distribution list references it by Akonadi ID.  If an address is found then their name and email address is added to the distribution list.
> 
> This is surely wrong (logic reversed), an existing contact should be referenced to avoid duplication and an unknown contact added by name/email so as not to add unwanted entries to the address book (and also to avoid a nested event loop).
> 
> This change implements that by retaining the Akonadi ID of a searched contact, and using it to create the distribution list (and also recording the email address if that is not the same as the contact's preferred email).
> 
> 
> Diffs
> -----
> 
>   messagecomposer/recipient/distributionlistdialog.cpp f03aaec 
> 
> Diff: http://git.reviewboard.kde.org/r/113180/diff/
> 
> 
> Testing
> -------
> 
> Build kdepim with this change, checked saving of distribution lists with known and unknown contacts.  No unwanted contacts added to address book, distribution list entry stored as:
> 
> Known contact, using preferred email:
>     <contactReference uid="10104" gid=""/>
> 
> Known contact but using secondary email:
>     <contactReference uid="345" gid="" preferredEmail="xxx at somedomain.com"/>
> 
> Unknown contact:
>     <contactData name="Anonymous Person" email="their.email at yahoo.co.co"/>
> 
> 
> Thanks,
> 
> Jonathan Marten
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/