address-book-service

Merge lp:~renatofilho/address-book-service/fix-addr-update into lp:address-book-service

  1. fix-addr-update
  2. Merge into trunk
Proposed by Renato Araujo Oliveira Filho
Status: Merged
Approved by: Bill Filler
Approved revision: 146
Merged at revision: 146
Proposed branch: lp:~renatofilho/address-book-service/fix-addr-update
Merge into: lp:address-book-service
Diff against target: 211 lines (+49/-22)
3 files modified
lib/qindividual.cpp (+27/-22)
lib/qindividual.h (+2/-0)
tests/unittest/qcontacts-test.cpp (+20/-0)
To merge this branch: bzr merge lp:~renatofilho/address-book-service/fix-addr-update
Reviewer Review Type Date Requested Status
Bill Filler (community) Approve
PS Jenkins bot continuous-integration Approve
Review via email: mp+240587@code.launchpad.net

Commit message

Save address fields in single lines. To avoid problems with folks.

To post a comment you must log in.
lp:~renatofilho/address-book-service/fix-addr-update updated
145. By Renato Araujo Oliveira Filho

Remove spaces at the end and new line symbols from street field.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:144
http://jenkins.qa.ubuntu.com/job/address-book-service-ci/314/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-service-utopic-armhf-ci/86
        deb: http://jenkins.qa.ubuntu.com/job/address-book-service-utopic-armhf-ci/86/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/address-book-service-utopic-i386-ci/86/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/6453
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/4281/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/6051
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/7705
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/7705/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15409
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/3634/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4675
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4675/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-service-ci/314/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:145
http://jenkins.qa.ubuntu.com/job/address-book-service-ci/316/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-service-utopic-armhf-ci/88
        deb: http://jenkins.qa.ubuntu.com/job/address-book-service-utopic-armhf-ci/88/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-service-utopic-i386-ci/88
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/6457
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/4285/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/6056
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/7709
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/7709/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15416
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/3638/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4679
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4679/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-service-ci/316/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Bill Filler (bfiller) wrote :

shouldn't we do this with all fields, not just address fields?

review: Needs Fixing
Revision history for this message
Bill Filler (bfiller) wrote :

also, please add some unit tests to test for this case of unexpected characters in the fields

lp:~renatofilho/address-book-service/fix-addr-update updated
146. By Renato Araujo Oliveira Filho

Convert any multi line field into a single line value.
Create unit test.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:146
http://jenkins.qa.ubuntu.com/job/address-book-service-ci/317/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-service-vivid-armhf-ci/1
        deb: http://jenkins.qa.ubuntu.com/job/address-book-service-vivid-armhf-ci/1/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-service-vivid-i386-ci/1
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/24
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid/16
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/28
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/24
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/24/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15504
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-vivid/18
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/17
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/17/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-service-ci/317/rebuild

review: Approve (continuous-integration)
Revision history for this message
Bill Filler (bfiller) wrote :

tested, approved

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/qindividual.cpp'
2--- lib/qindividual.cpp 2014-10-14 15:45:14 +0000
3+++ lib/qindividual.cpp 2014-11-06 15:44:29 +0000
4@@ -230,23 +230,23 @@
5 if (sn) {
6 const char *name = folks_structured_name_get_given_name(sn);
7 if (name && strlen(name)) {
8- detail.setFirstName(QString::fromUtf8(name));
9+ detail.setFirstName(qStringFromGChar(name));
10 }
11 name = folks_structured_name_get_additional_names(sn);
12 if (name && strlen(name)) {
13- detail.setMiddleName(QString::fromUtf8(name));
14+ detail.setMiddleName(qStringFromGChar(name));
15 }
16 name = folks_structured_name_get_family_name(sn);
17 if (name && strlen(name)) {
18- detail.setLastName(QString::fromUtf8(name));
19+ detail.setLastName(qStringFromGChar(name));
20 }
21 name = folks_structured_name_get_prefixes(sn);
22 if (name && strlen(name)) {
23- detail.setPrefix(QString::fromUtf8(name));
24+ detail.setPrefix(qStringFromGChar(name));
25 }
26 name = folks_structured_name_get_suffixes(sn);
27 if (name && strlen(name)) {
28- detail.setSuffix(QString::fromUtf8(name));
29+ detail.setSuffix(qStringFromGChar(name));
30 }
31 detail.setDetailUri(QString("%1.1").arg(index));
32 }
33@@ -262,7 +262,7 @@
34 QContactDisplayLabel detail;
35 const gchar *fullName = folks_name_details_get_full_name(FOLKS_NAME_DETAILS(persona));
36 if (fullName) {
37- detail.setLabel(QString::fromUtf8(fullName));
38+ detail.setLabel(qStringFromGChar(fullName));
39 detail.setDetailUri(QString("%1.1").arg(index));
40 }
41
42@@ -278,7 +278,7 @@
43 QContactNickname detail;
44 const gchar* nickname = folks_name_details_get_nickname(FOLKS_NAME_DETAILS(persona));
45 if (nickname && strlen(nickname)) {
46- detail.setNickname(QString::fromUtf8(nickname));
47+ detail.setNickname(qStringFromGChar(nickname));
48 detail.setDetailUri(QString("%1.1").arg(index));
49 }
50 return detail;
51@@ -317,6 +317,11 @@
52 }
53 }
54
55+QString QIndividual::qStringFromGChar(const gchar *str)
56+{
57+ return QString::fromUtf8(str).remove(QRegExp("[\r\n]"));
58+}
59+
60 QtContacts::QContactDetail QIndividual::getPersonaPhoto(FolksPersona *persona, int index) const
61 {
62 QContactAvatar avatar;
63@@ -399,15 +404,15 @@
64 QContactOrganization org;
65 QString field;
66
67- field = QString::fromUtf8(folks_role_get_organisation_name(role));
68+ field = qStringFromGChar(folks_role_get_organisation_name(role));
69 if (!field.isEmpty()) {
70 org.setName(field);
71 }
72- field = QString::fromUtf8(folks_role_get_title(role));
73+ field = qStringFromGChar(folks_role_get_title(role));
74 if (!field.isEmpty()) {
75 org.setTitle(field);
76 }
77- field = QString::fromUtf8(folks_role_get_role(role));
78+ field = qStringFromGChar(folks_role_get_role(role));
79 if (!field.isEmpty()) {
80 org.setRole(field);
81 }
82@@ -447,7 +452,7 @@
83 const gchar *email = (const gchar*) folks_abstract_field_details_get_value(fd);
84
85 QContactEmailAddress addr;
86- addr.setEmailAddress(QString::fromUtf8(email));
87+ addr.setEmailAddress(qStringFromGChar(email));
88 bool isPref = false;
89 DetailContextParser::parseParameters(addr, fd, &isPref);
90 addr.setDetailUri(QString("%1.%2").arg(index).arg(fieldIndex++));
91@@ -483,7 +488,7 @@
92 const gchar *phone = (const char*) folks_abstract_field_details_get_value(fd);
93
94 QContactPhoneNumber number;
95- number.setNumber(QString::fromUtf8(phone));
96+ number.setNumber(qStringFromGChar(phone));
97 bool isPref = false;
98 DetailContextParser::parseParameters(number, fd, &isPref);
99 number.setDetailUri(QString("%1.%2").arg(index).arg(fieldIndex++));
100@@ -521,32 +526,32 @@
101 QContactAddress address;
102 const char *field = folks_postal_address_get_country(addr);
103 if (field && strlen(field)) {
104- address.setCountry(QString::fromUtf8(field));
105+ address.setCountry(qStringFromGChar(field));
106 }
107
108 field = folks_postal_address_get_locality(addr);
109 if (field && strlen(field)) {
110- address.setLocality(QString::fromUtf8(field));
111+ address.setLocality(qStringFromGChar(field));
112 }
113
114 field = folks_postal_address_get_po_box(addr);
115 if (field && strlen(field)) {
116- address.setPostOfficeBox(QString::fromUtf8(field));
117+ address.setPostOfficeBox(qStringFromGChar(field));
118 }
119
120 field = folks_postal_address_get_postal_code(addr);
121 if (field && strlen(field)) {
122- address.setPostcode(QString::fromUtf8(field));
123+ address.setPostcode(qStringFromGChar(field));
124 }
125
126 field = folks_postal_address_get_region(addr);
127 if (field && strlen(field)) {
128- address.setRegion(QString::fromUtf8(field));
129+ address.setRegion(qStringFromGChar(field));
130 }
131
132 field = folks_postal_address_get_street(addr);
133 if (field && strlen(field)) {
134- address.setStreet(QString::fromUtf8(field));
135+ address.setStreet(qStringFromGChar(field));
136 }
137
138 bool isPref = false;
139@@ -594,8 +599,8 @@
140 }
141
142 QContactOnlineAccount account;
143- account.setAccountUri(QString::fromUtf8(uri));
144- int protocolId = DetailContextParser::accountProtocolFromString(QString::fromUtf8(key));
145+ account.setAccountUri(qStringFromGChar(uri));
146+ int protocolId = DetailContextParser::accountProtocolFromString(qStringFromGChar(key));
147 account.setProtocol(static_cast<QContactOnlineAccount::Protocol>(protocolId));
148
149 bool isPref = false;
150@@ -636,7 +641,7 @@
151 const char *url = (const char*) folks_abstract_field_details_get_value(fd);
152
153 QContactUrl detail;
154- detail.setUrl(QString::fromUtf8(url));
155+ detail.setUrl(qStringFromGChar(url));
156 bool isPref = false;
157 DetailContextParser::parseParameters(detail, fd, &isPref);
158 detail.setDetailUri(QString("%1.%2").arg(index).arg(fieldIndex++));
159@@ -780,7 +785,7 @@
160 GeeIterator *iter = gee_iterable_iterator(GEE_ITERABLE(personas));
161 while(gee_iterator_next(iter)) {
162 FolksPersona *persona = FOLKS_PERSONA(gee_iterator_get(iter));
163- m_personas.insert(QString::fromUtf8(folks_persona_get_iid(persona)), persona);
164+ m_personas.insert(qStringFromGChar(folks_persona_get_iid(persona)), persona);
165 }
166
167 g_object_unref(iter);
168
169=== modified file 'lib/qindividual.h'
170--- lib/qindividual.h 2014-10-13 19:33:46 +0000
171+++ lib/qindividual.h 2014-11-06 15:44:29 +0000
172@@ -168,6 +168,8 @@
173 static void folksIndividualChanged (FolksIndividual *individual,
174 GParamSpec *pspec,
175 QIndividual *self);
176+
177+ static QString qStringFromGChar (const gchar *str);
178 };
179
180 } //namespace
181
182=== modified file 'tests/unittest/qcontacts-test.cpp'
183--- tests/unittest/qcontacts-test.cpp 2014-10-13 19:33:46 +0000
184+++ tests/unittest/qcontacts-test.cpp 2014-11-06 15:44:29 +0000
185@@ -489,6 +489,26 @@
186 }
187 QCOMPARE(address.size(), 0);
188 }
189+
190+ /*
191+ * Test saving a contact with a multi line field
192+ * BUG: #240587
193+ */
194+ void testFieldWithNewLineChar()
195+ {
196+ // create a contact
197+ QContact contact = testContact();
198+ QContactAddress addr;
199+ addr.setCountry("Line1\nLine2\r\nLine3");
200+ contact.saveDetail(&addr);
201+ QSignalSpy spyContactAdded(m_manager, SIGNAL(contactsAdded(QList<QContactId>)));
202+ bool result = m_manager->saveContact(&contact);
203+ QCOMPARE(result, true);
204+ QTRY_COMPARE(spyContactAdded.count(), 1);
205+
206+ addr = contact.detail<QContactAddress>();
207+ QCOMPARE(addr.country(), QStringLiteral("Line1Line2Line3"));
208+ }
209 };
210
211 QTEST_MAIN(QContactsTest)

Subscribers

People subscribed via source and target branches