Merge lp:~diegosarmentero/pay-ui/pay-errors into lp:pay-ui

Proposed by Diego Sarmentero
Status: Merged
Approved by: dobey
Approved revision: 7
Merged at revision: 5
Proposed branch: lp:~diegosarmentero/pay-ui/pay-errors
Merge into: lp:pay-ui
Diff against target: 495 lines (+158/-23)
11 files modified
app/payui.qml (+76/-14)
app/ui/CheckoutPage.qml (+22/-1)
app/ui/DirectPurchase.qml (+19/-0)
app/ui/UbuntuPurchaseWebkit.qml (+1/-5)
backend/modules/payui/network.cpp (+6/-0)
backend/modules/payui/network.h (+1/-0)
backend/modules/payui/purchase.cpp (+11/-1)
backend/modules/payui/purchase.h (+2/-0)
backend/tests/mock_click_server.py (+8/-0)
backend/tests/test_network.cpp (+11/-1)
manifest.json (+1/-1)
To merge this branch: bzr merge lp:~diegosarmentero/pay-ui/pay-errors
Reviewer Review Type Date Requested Status
Alejandro J. Cura (community) Approve
dobey (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+225397@code.launchpad.net

Commit message

- Showing errors and give the user the chance to recover from them.
- Improve logging

Description of the change

Not quit on any error, but instead show a message to the user, and let the user decide the action to follow.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alejandro J. Cura (alecu) wrote :

Please commit this branch with --fixes lp:1333394

Why are there no unit tests for the qml bits?

review: Needs Fixing
7. By Diego Sarmentero

linking bug

Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

> Please commit this branch with --fixes lp:1333394

Done

>
> Why are there no unit tests for the qml bits?

As I mentioned yesterday, we can't do qml tests YET, because we need to load the Purchase plugin, and there is a bug with autopilot which I already mentioned to elopio and he confirmed it that for some reason is breaking autopilot on loading those qml plugins.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alejandro J. Cura (alecu) wrote :

On Thu, Jul 3, 2014 at 8:29 AM, Diego Sarmentero
<email address hidden> wrote:
>> Please commit this branch with --fixes lp:1333394
>
> Done
>
>>
>> Why are there no unit tests for the qml bits?
>
> As I mentioned yesterday, we can't do qml tests YET, because we need to load the Purchase plugin, and there is a bug with autopilot which I already mentioned to elopio and he confirmed it that for some reason is breaking autopilot on loading those qml plugins.

Please link here the autopilot bug, and please open a new bug for the
missing tests in pay-ui.

Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

> Please commit this branch with --fixes lp:1333394
>
> Why are there no unit tests for the qml bits?

Bug created for this: https://bugs.launchpad.net/pay-ui/+bug/1337311

Revision history for this message
dobey (dobey) :
review: Approve
Revision history for this message
Alejandro J. Cura (alecu) wrote :

Code looks good, and tested on mako, seems to be working ok.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'app/payui.qml'
2--- app/payui.qml 2014-06-25 20:58:29 +0000
3+++ app/payui.qml 2014-07-03 11:28:29 +0000
4@@ -6,9 +6,12 @@
5 import "ui"
6
7 /*!
8- \brief MainView with Tabs element.
9- First Tab has a single Label and
10- second Tab has a single ToolbarAction.
11+ states:
12+ - add-payment
13+ - direct
14+ - buy-interaction
15+ - checkout
16+ - no-credentials
17 */
18
19 MainView {
20@@ -32,6 +35,8 @@
21
22 property bool loading: true
23 property bool purchasing: false
24+ property string errorTitle: ""
25+ property string errorMessage: ""
26
27
28 AccountServiceModel {
29@@ -55,8 +60,10 @@
30 if (payments.length == 0) {
31 webkit.title = i18n.tr("Add Payment");
32 webkit.url = purchase.getAddPaymentUrl();
33+ mainView.state = "add-payment";
34 pageStack.push(webkit);
35 } else {
36+ mainView.state = "direct";
37 pageStack.push(direct);
38 }
39 hideLoading();
40@@ -65,26 +72,37 @@
41 onBuyItemFailed: {
42 mainView.purchasing = false;
43 hideLoading();
44- purchase.quitCancel();
45+ var title = i18n.tr("Purchase failed");
46+ var message = i18n.tr("The purchase couldn't be completed.");
47+ mainView.showErrorDialog(title, message);
48 }
49
50 onBuyItemSucceeded: {
51- Qt.quit();
52+ purchase.quitSuccess();
53 }
54
55 onBuyInterationRequired: {
56 mainView.purchasing = false;
57 webkit.title = "";
58 webkit.url = url;
59+ mainView.state = "buy-interaction";
60 pageStack.push(webkit);
61 }
62
63 onError: {
64- purchase.quitCancel();
65- hideLoading();
66+ hideLoading();
67+ var title = i18n.tr("Error contacting the server");
68+ var message = i18n.tr("Do you want to try again?");
69+ mainView.showErrorDialog(title, message);
70+ }
71+
72+ onAuthenticationError: {
73+ hideLoading();
74+ pageStack.currentPage.showWrongPassword();
75 }
76
77 onCredentialsNotFound: {
78+ mainView.state = "no-credentials";
79 pageStack.push(noCredentials);
80 hideLoading();
81 }
82@@ -99,6 +117,12 @@
83 mainView.loading = false;
84 }
85
86+ function showErrorDialog(title, message) {
87+ mainView.errorTitle = title;
88+ mainView.errorMessage = message;
89+ PopupUtils.open(errorDialogContainer);
90+ }
91+
92 Component {
93 id: loadingDialogContainer
94 Dialog {
95@@ -127,6 +151,33 @@
96 }
97 }
98
99+ Component {
100+ id: errorDialogContainer
101+ Dialog {
102+ id: errorDialog
103+ title: mainView.errorTitle
104+ text: mainView.errorMessage
105+
106+ Button {
107+ text: i18n.tr("Retry")
108+ color: UbuntuColors.orange
109+ onClicked: {
110+ showLoading();
111+ purchase.checkCredentials();
112+ PopupUtils.close(errorDialog);
113+ }
114+ }
115+ Button {
116+ text: i18n.tr("Exit")
117+ color: UbuntuColors.coolGrey
118+ onClicked: {
119+ PopupUtils.close(errorDialog);
120+ purchase.quitCancel();
121+ }
122+ }
123+ }
124+ }
125+
126 PageStack {
127 id: pageStack
128 Component.onCompleted: {
129@@ -140,6 +191,7 @@
130 account: accounts
131
132 onShowPaymentTypes: {
133+ mainView.state = "checkout";
134 pageStack.push(checkout);
135 }
136
137@@ -172,6 +224,7 @@
138 onAddCreditCard: {
139 webkit.title = i18n.tr("Add Payment");
140 webkit.url = purchase.getAddPaymentUrl();
141+ mainView.state = "add-payment";
142 pageStack.push(webkit);
143 }
144 }
145@@ -180,16 +233,25 @@
146 id: webkit
147 visible: false
148
149- onPayTypeAdded: {
150- showLoading();
151- purchase.getPaymentTypes();
152- pageStack.push(checkout);
153- }
154 onPurchaseCanceled: {
155- purchase.quitCancel();
156+ hideLoading();
157+ if (mainView.state == "add-payment") {
158+ var title = i18n.tr("Adding Credit Card failed");
159+ var message = i18n.tr("Do you want to try again?");
160+ mainView.showErrorDialog(title, message);
161+ } else {
162+ var title = i18n.tr("Purchase failed");
163+ var message = i18n.tr("The purchase couldn't be completed.");
164+ mainView.showErrorDialog(title, message);
165+ }
166 }
167 onPurchaseSucceeded: {
168- pageStack.push(success);
169+ if (mainView.state == "add-payment") {
170+ showLoading();
171+ purchase.getPaymentTypes();
172+ } else {
173+ purchase.quitSuccess();
174+ }
175 }
176
177 onLoading: {
178
179=== modified file 'app/ui/CheckoutPage.qml'
180--- app/ui/CheckoutPage.qml 2014-06-25 20:58:29 +0000
181+++ app/ui/CheckoutPage.qml 2014-07-03 11:28:29 +0000
182@@ -24,6 +24,7 @@
183
184 title: selector.currentlyExpanded ? i18n.tr("Payment type") : i18n.tr("Checkout")
185
186+ property bool _errorVisible: false
187 property int keyboardSize: Qt.inputMethod.visible ? Qt.inputMethod.keyboardRectangle.height : 0
188 property alias itemTitle: titleLabel.text
189 property alias itemSubtitle: subtitleLabel.text
190@@ -37,11 +38,16 @@
191 signal addCreditCard
192
193 function launchPurchase() {
194+ pageCheckout._errorVisible = false;
195 var pay = selector.model[selector.selectedIndex];
196 var email = accountView.currentItem.email;
197 pageCheckout.buy(email, passwordField.text, pay.paymentId, pay.backendId);
198 }
199
200+ function showWrongPassword() {
201+ pageCheckout._errorVisible = true;
202+ }
203+
204 Flickable {
205 id: checkoutFlickable
206 anchors {
207@@ -261,13 +267,28 @@
208 Keys.onReturnPressed: launchPurchase();
209 }
210
211+ Label {
212+ id: errorPasswordLabel
213+ objectName: "errorPasswordLabel"
214+ color: "red"
215+ text: i18n.tr("Incorrect Password, please try again.")
216+ wrapMode: Text.WordWrap
217+ visible: selector.currentlyExpanded ? false : pageCheckout._errorVisible
218+ anchors {
219+ left: parent.left
220+ right: parent.right
221+ top: passwordField.bottom
222+ margins: units.gu(1)
223+ }
224+ }
225+
226 Rectangle {
227 id: separator
228 color: "#dedede"
229 anchors {
230 left: parent.left
231 right: parent.right
232- top: selector.currentlyExpanded ? manageCardLabel.bottom : passwordField.bottom
233+ top: selector.currentlyExpanded ? manageCardLabel.bottom : errorPasswordLabel.bottom
234 topMargin: units.gu(2)
235 }
236 height: 2
237
238=== modified file 'app/ui/DirectPurchase.qml'
239--- app/ui/DirectPurchase.qml 2014-06-25 20:58:29 +0000
240+++ app/ui/DirectPurchase.qml 2014-07-03 11:28:29 +0000
241@@ -27,10 +27,15 @@
242 property alias account: accountView.model
243
244 function launchPurchase() {
245+ errorPasswordLabel.visible = false;
246 var email = accountView.currentItem.text;
247 pageDirectPayment.buy(email, passwordField.text);
248 }
249
250+ function showWrongPassword() {
251+ errorPasswordLabel.visible = true;
252+ }
253+
254 Column {
255 spacing: units.gu(2)
256 anchors.fill: parent
257@@ -96,6 +101,19 @@
258 }
259
260 Label {
261+ id: errorPasswordLabel
262+ objectName: "errorPasswordLabel"
263+ color: "red"
264+ text: i18n.tr("Incorrect Password, please try again.")
265+ wrapMode: Text.WordWrap
266+ visible: false
267+ anchors {
268+ left: parent.left
269+ right: parent.right
270+ }
271+ }
272+
273+ Label {
274 id: paymentTypeLabel
275 objectName: "paymentTypeLabel"
276 textFormat: Text.RichText
277@@ -106,6 +124,7 @@
278 right: parent.right
279 }
280 onLinkActivated: {
281+ errorPasswordLabel.visible = false;
282 pageDirectPayment.showPaymentTypes();
283 }
284 }
285
286=== modified file 'app/ui/UbuntuPurchaseWebkit.qml'
287--- app/ui/UbuntuPurchaseWebkit.qml 2014-06-23 18:20:45 +0000
288+++ app/ui/UbuntuPurchaseWebkit.qml 2014-07-03 11:28:29 +0000
289@@ -17,7 +17,7 @@
290 import QtQuick 2.0
291 import Ubuntu.Components 0.1
292 import Ubuntu.Components.Popups 0.1
293-import QtWebKit 3.0
294+import com.canonical.Oxide 1.0
295
296
297 Page {
298@@ -26,7 +26,6 @@
299
300 signal purchaseSucceeded()
301 signal purchaseCanceled()
302- signal payTypeAdded()
303 signal loading(bool value)
304
305 property int keyboardSize: Qt.inputMethod.visible ? Qt.inputMethod.keyboardRectangle.height : 0
306@@ -75,9 +74,6 @@
307 pageWebkit.closePurchase();
308 pageWebkit.purchaseCanceled();
309 return;
310- } else if(/^(purchase):\/\/payAdded/.test(request.url)) {
311- pageWebkit.payTypeAdded();
312- return;
313 }
314
315 request.action = WebView.AcceptRequest;
316
317=== modified file 'backend/modules/payui/network.cpp'
318--- backend/modules/payui/network.cpp 2014-06-25 20:58:29 +0000
319+++ backend/modules/payui/network.cpp 2014-07-03 11:28:29 +0000
320@@ -34,6 +34,7 @@
321 #define PAY_BASE_URL "https://sc.ubuntu.com/api/2.0/click/"
322 #define ACCOUNT_CREDS_URL "https://login.ubuntu.com/api/v2/tokens/oauth"
323 #define ADD_PAYMENT_URL "https://sc.ubuntu.com/api/2.0/click/paymentmethods/add/"
324+//<matiasb> gatox: this is the url, in staging: https://sc.staging.ubuntu.com/click/payment-method/add/
325
326 #define PREFERED_PAYMENT_TYPE "0"
327 #define PAYMENT_TYPES "1"
328@@ -159,11 +160,14 @@
329 QString state = object.value("state").toString();
330
331 if (state == BUY_COMPLETE) {
332+ qDebug() << "BUY STATE: complete";
333 Q_EMIT buyItemSucceeded();
334 } else if (state == BUY_IN_PROGRESS) {
335+ qDebug() << "BUY STATE: in progress";
336 QString redirect_to = object.value("redirect_to").toString();
337 Q_EMIT buyInteractionRequired(redirect_to);
338 } else {
339+ qDebug() << "BUY STATE: failed";
340 Q_EMIT buyItemFailed();
341 }
342 } else if (state->operation.contains(ITEM_INFO) && document.isObject()) {
343@@ -190,6 +194,8 @@
344 Q_EMIT error(message);
345 }
346
347+ } else if (httpStatus == 401 || httpStatus == 403) {
348+ Q_EMIT authenticationError();
349 } else {
350 Q_EMIT error(QString::number(httpStatus));
351 }
352
353=== modified file 'backend/modules/payui/network.h'
354--- backend/modules/payui/network.h 2014-06-25 20:58:29 +0000
355+++ backend/modules/payui/network.h 2014-07-03 11:28:29 +0000
356@@ -70,6 +70,7 @@
357 void buyItemFailed();
358 void buyInteractionRequired(QString url);
359 void error(QString message);
360+ void authenticationError();
361 void credentialsNotFound();
362 void credentialsFound();
363 void credentialsUpdated();
364
365=== modified file 'backend/modules/payui/purchase.cpp'
366--- backend/modules/payui/purchase.cpp 2014-06-23 18:20:45 +0000
367+++ backend/modules/payui/purchase.cpp 2014-07-03 11:28:29 +0000
368@@ -26,6 +26,8 @@
369 this, &Purchase::buyInterationRequired);
370 QObject::connect(&m_network, &Network::error,
371 this, &Purchase::error);
372+ QObject::connect(&m_network, &Network::authenticationError,
373+ this, &Purchase::authenticationError);
374 QObject::connect(&m_network, &Network::credentialsNotFound,
375 this, &Purchase::credentialsNotFound);
376 QObject::connect(&m_network, &Network::credentialsFound,
377@@ -55,13 +57,21 @@
378
379 void Purchase::quitCancel()
380 {
381+ qDebug() << "Purchase Canceled: exit code 1";
382 QCoreApplication::exit(1);
383 }
384
385+void Purchase::quitSuccess()
386+{
387+ qDebug() << "Purchase Succeeded: exit code 0";
388+ QCoreApplication::exit(0);
389+}
390+
391 void Purchase::checkCredentials()
392 {
393 if (m_appid.isEmpty() && m_itemid.isEmpty()) {
394- QCoreApplication::exit(1);
395+ qDebug() << "AppId or ItemId not provided";
396+ quitCancel();
397 } else {
398 qDebug() << "Getting Item Details";
399 getItemDetails(m_appid, m_itemid);
400
401=== modified file 'backend/modules/payui/purchase.h'
402--- backend/modules/payui/purchase.h 2014-06-23 18:20:45 +0000
403+++ backend/modules/payui/purchase.h 2014-07-03 11:28:29 +0000
404@@ -21,6 +21,7 @@
405 Q_INVOKABLE void checkCredentials();
406 Q_INVOKABLE QString getAddPaymentUrl();
407
408+ Q_INVOKABLE void quitSuccess();
409 Q_INVOKABLE void quitCancel();
410
411 Q_SIGNALS:
412@@ -30,6 +31,7 @@
413 void buyItemFailed();
414 void buyInterationRequired(QString url);
415 void error(QString message);
416+ void authenticationError();
417 void credentialsNotFound();
418 void credentialsFound();
419
420
421=== modified file 'backend/tests/mock_click_server.py'
422--- backend/tests/mock_click_server.py 2014-06-23 18:20:45 +0000
423+++ backend/tests/mock_click_server.py 2014-07-03 11:28:29 +0000
424@@ -114,6 +114,12 @@
425 self.end_headers()
426 self.wfile.write(bytes(json.dumps(response), 'UTF-8'))
427
428+ def response_auth_error(self):
429+ self.send_response(401)
430+ self.send_header("Content-type", "application/json")
431+ self.end_headers()
432+ self.wfile.write(bytes(json.dumps(dict()), 'UTF-8'))
433+
434 def do_POST(self):
435 """Respond to a POST request."""
436 #content = self.rfile.read(int(self.headers.get('content-length')))
437@@ -135,6 +141,8 @@
438 elif self.path.find("iteminfo/") != -1:
439 fail = self.path.find("/fail/") != -1
440 self.response_item_info(fail)
441+ elif self.path.find("/authError/") != -1:
442+ self.response_auth_error()
443 elif self.path.find("shutdown") != -1:
444 global KEEP_ALIVE
445 KEEP_ALIVE = False
446
447=== modified file 'backend/tests/test_network.cpp'
448--- backend/tests/test_network.cpp 2014-06-25 20:58:29 +0000
449+++ backend/tests/test_network.cpp 2014-07-03 11:28:29 +0000
450@@ -40,6 +40,7 @@
451
452 private slots:
453 void initTestCase();
454+ void testNetworkAuthenticationError();
455 void testNetworkGetPaymentTypes();
456 void testNetworkGetPaymentTypesFail();
457 void testNetworkBuyItem();
458@@ -62,7 +63,7 @@
459 void TestNetwork::initTestCase()
460 {
461 qDebug() << "Starting Server";
462-// network.setCredentials(Token("token_key", "token_secret", "consumer_key", "consumer_secret"));
463+ network.setCredentials(Token("token_key", "token_secret", "consumer_key", "consumer_secret"));
464 process = new QProcess(this);
465 QSignalSpy spy(process, SIGNAL(started()));
466 process->setWorkingDirectory(QDir::currentPath() + "/backend/tests/");
467@@ -80,6 +81,15 @@
468 QTRY_COMPARE(spy2.count(), 1);
469 }
470
471+void TestNetwork::testNetworkAuthenticationError()
472+{
473+ setenv("ACCOUNT_CREDS_URL", "http://localhost:8000/authError/", 1);
474+ setenv("PAY_BASE_URL", "http://localhost:8000/", 1);
475+ QSignalSpy spy(&network, SIGNAL(authenticationError()));
476+ network.buyItem("email", "password", "appid", "itemid", "paymentid", "backendid");
477+ QTRY_COMPARE(spy.count(), 1);
478+}
479+
480 void TestNetwork::testNetworkGetPaymentTypes()
481 {
482 setenv("PAY_BASE_URL", "http://localhost:8000/", 1);
483
484=== modified file 'manifest.json'
485--- manifest.json 2014-06-25 20:58:29 +0000
486+++ manifest.json 2014-07-03 11:28:29 +0000
487@@ -10,6 +10,6 @@
488 "desktop": "payui.desktop"
489 }
490 },
491- "version": "0.2",
492+ "version": "0.2.5",
493 "maintainer": "Sarmentero Diego <diego.sarmentero@canonical.com>"
494 }
495\ No newline at end of file

Subscribers

People subscribed via source and target branches

to all changes: