[Merge] lp:~renatofilho/address-book-service/fix-vcard-import into lp:address-book-service

Michael Sheldon michael.sheldon at canonical.com
Wed Oct 28 12:52:21 UTC 2015


Review: Needs Fixing

Added a couple of diff comments

Diff comments:

> === modified file 'lib/detail-context-parser.cpp'
> --- lib/detail-context-parser.cpp	2014-07-08 19:24:25 +0000
> +++ lib/detail-context-parser.cpp	2015-10-27 15:15:49 +0000
> @@ -75,6 +75,10 @@
>              break;
>          case QContactDetail::TypeAddress:
>              context << parseAddressContext(detail);
> +            // Setting more than one context causes folks to freeze
> +            if (!context.isEmpty()) {
> +                return context.mid(0,1);

context.first() would seem a bit more readable to me

> +            }
>              break;
>          case QContactDetail::TypeOnlineAccount:
>              context << parseOnlineAccountContext(detail);
> 
> === modified file 'tests/unittest/qcontacts-test.cpp'
> --- tests/unittest/qcontacts-test.cpp	2015-09-24 17:47:49 +0000
> +++ tests/unittest/qcontacts-test.cpp	2015-10-27 15:15:49 +0000
> @@ -574,6 +574,21 @@
>          QCOMPARE(updatedName.lastName(), name.lastName());
>      }
>  
> +    void testAddressWithMultipleSubTypes()
> +    {
> +        // create a contact
> +        QContact contact = testContact();
> +        QContactAddress addr;
> +        addr.setLocality("8777 West Six Mile Road");
> +        addr.setContexts(QList<int>() << QContactDetail::ContextWork);

Shouldn't this list have more than one context in it to properly test the issue?

> +        addr.setSubTypes(QList<int>() << QContactAddress::SubTypeParcel << QContactAddress::SubTypeDomestic);
> +        contact.saveDetail(&addr);
> +
> +        QSignalSpy spyContactAdded(m_manager, SIGNAL(contactsAdded(QList<QContactId>)));
> +        bool result = m_manager->saveContact(&contact);
> +        QCOMPARE(result, true);
> +        QTRY_COMPARE(spyContactAdded.count(), 1);
> +    }
>  };
>  
>  QTEST_MAIN(QContactsTest)


-- 
https://code.launchpad.net/~renatofilho/address-book-service/fix-vcard-import/+merge/272310
Your team Ubuntu Phablet Team is subscribed to branch lp:address-book-service.



More information about the Ubuntu-reviews mailing list