Merge lp:~online-accounts/ubuntu-system-settings-online-accounts/master into lp:ubuntu-system-settings-online-accounts

Proposed by Alberto Mardegan
Status: Merged
Approved by: David Barth
Approved revision: no longer in the source branch.
Merged at revision: 189
Proposed branch: lp:~online-accounts/ubuntu-system-settings-online-accounts/master
Merge into: lp:ubuntu-system-settings-online-accounts
Diff against target: 299 lines (+92/-17)
8 files modified
online-accounts-ui/access-model.cpp (+25/-3)
online-accounts-ui/application-manager.cpp (+1/-0)
online-accounts-ui/module/WebView.qml (+3/-4)
online-accounts-ui/qml/AccountCreationPage.qml (+2/-3)
online-accounts-ui/qml/AuthorizationPage.qml (+1/-1)
online-accounts-ui/qml/ProviderRequest.qml (+17/-3)
tests/online-accounts-ui/tst_access_model.cpp (+22/-0)
tests/online-accounts-ui/tst_application_manager.cpp (+21/-3)
To merge this branch: bzr merge lp:~online-accounts/ubuntu-system-settings-online-accounts/master
Reviewer Review Type Date Requested Status
David Barth (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+235921@code.launchpad.net

Commit message

* Enforce the limit on single accounts even when the request comes from an app

Description of the change

* Enforce the limit on single accounts even when the request comes from an app

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
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
David Barth (dbarth) wrote :

+1

review: Approve
Revision history for this message
Alexandre Abreu (abreu-alexandre) wrote :

A few inline comments.

Also in AccessModel::setLastItemText can you clarify the rationale?

189. By Alberto Mardegan

* Enforce the limit on single accounts even when the request comes from an app
Approved by: PS Jenkins bot, David Barth

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'online-accounts-ui/access-model.cpp'
--- online-accounts-ui/access-model.cpp 2014-02-03 10:27:39 +0000
+++ online-accounts-ui/access-model.cpp 2014-09-25 13:16:18 +0000
@@ -159,7 +159,8 @@
159159
160int AccessModel::rowCount(const QModelIndex &parent) const160int AccessModel::rowCount(const QModelIndex &parent) const
161{161{
162 return QIdentityProxyModel::rowCount(parent) + 1;162 return QIdentityProxyModel::rowCount(parent) +
163 (lastItemText().isEmpty() ? 0 : 1);
163}164}
164165
165void AccessModel::setLastItemText(const QString &text)166void AccessModel::setLastItemText(const QString &text)
@@ -167,11 +168,32 @@
167 Q_D(AccessModel);168 Q_D(AccessModel);
168169
169 if (text == d->m_lastItemText) return;170 if (text == d->m_lastItemText) return;
171
172 bool wasEmpty = d->m_lastItemText.isEmpty();
173 bool insertingRow = false;
174 bool removingRow = false;
175 int oldRowCount = rowCount();
176 if (text.isEmpty() != wasEmpty) {
177 if (wasEmpty) {
178 beginInsertRows(QModelIndex(), oldRowCount, oldRowCount);
179 insertingRow = true;
180 } else {
181 beginRemoveRows(QModelIndex(), oldRowCount - 1, oldRowCount - 1);
182 removingRow = true;
183 }
184 }
185
170 d->m_lastItemText = text;186 d->m_lastItemText = text;
171 Q_EMIT lastItemTextChanged();187 Q_EMIT lastItemTextChanged();
172188
173 QModelIndex lastItemIndex = index(d->rowCount(), 0);189 if (insertingRow) {
174 Q_EMIT dataChanged(lastItemIndex, lastItemIndex);190 endInsertRows();
191 } else if (removingRow) {
192 endRemoveRows();
193 } else {
194 QModelIndex lastItemIndex = index(oldRowCount - 1, 0);
195 Q_EMIT dataChanged(lastItemIndex, lastItemIndex);
196 }
175}197}
176198
177QString AccessModel::lastItemText() const199QString AccessModel::lastItemText() const
178200
=== modified file 'online-accounts-ui/application-manager.cpp'
--- online-accounts-ui/application-manager.cpp 2014-09-16 11:35:48 +0000
+++ online-accounts-ui/application-manager.cpp 2014-09-25 13:16:18 +0000
@@ -183,6 +183,7 @@
183 info.insert(QStringLiteral("id"), providerId);183 info.insert(QStringLiteral("id"), providerId);
184 info.insert(QStringLiteral("displayName"), provider.displayName());184 info.insert(QStringLiteral("displayName"), provider.displayName());
185 info.insert(QStringLiteral("icon"), provider.iconName());185 info.insert(QStringLiteral("icon"), provider.iconName());
186 info.insert(QStringLiteral("isSingleAccount"), provider.isSingleAccount());
186187
187 /* Get Ubuntu-specific information directly from the XML file */188 /* Get Ubuntu-specific information directly from the XML file */
188 const QDomDocument doc = provider.domDocument();189 const QDomDocument doc = provider.domDocument();
189190
=== modified file 'online-accounts-ui/module/WebView.qml'
--- online-accounts-ui/module/WebView.qml 2014-09-05 09:36:27 +0000
+++ online-accounts-ui/module/WebView.qml 2014-09-25 13:16:18 +0000
@@ -1,9 +1,8 @@
1import QtQuick 2.01import QtQuick 2.0
2import Ubuntu.Components 0.12import Ubuntu.Components 0.1
3import Ubuntu.Components.Extras.Browser 0.23import Ubuntu.Web 0.2
4import com.canonical.Oxide 1.0
54
6UbuntuWebView {5WebView {
7 property QtObject signonRequest6 property QtObject signonRequest
87
9 Component.onCompleted: {8 Component.onCompleted: {
@@ -23,7 +22,7 @@
23 }22 }
24 onUrlChanged: signonRequest.currentUrl = url23 onUrlChanged: signonRequest.currentUrl = url
2524
26 context: UbuntuWebContext {25 context: WebContext {
27 dataPath: signonRequest ? signonRequest.rootDir : ""26 dataPath: signonRequest ? signonRequest.rootDir : ""
28 }27 }
2928
3029
=== modified file 'online-accounts-ui/qml/AccountCreationPage.qml'
--- online-accounts-ui/qml/AccountCreationPage.qml 2014-03-19 12:14:14 +0000
+++ online-accounts-ui/qml/AccountCreationPage.qml 2014-09-25 13:16:18 +0000
@@ -20,15 +20,14 @@
20import Ubuntu.Components 0.120import Ubuntu.Components 0.1
21import Ubuntu.OnlineAccounts 0.121import Ubuntu.OnlineAccounts 0.1
2222
23Page {23Item {
24 id: root24 id: root
2525
26 property string providerId26 property string providerId
27 property var flickable: null
2728
28 signal finished(int accountId)29 signal finished(int accountId)
2930
30 title: account.provider.displayName
31
32 Account {31 Account {
33 id: account32 id: account
34 objectHandle: Manager.createAccount(providerId)33 objectHandle: Manager.createAccount(providerId)
3534
=== modified file 'online-accounts-ui/qml/AuthorizationPage.qml'
--- online-accounts-ui/qml/AuthorizationPage.qml 2014-02-03 10:27:39 +0000
+++ online-accounts-ui/qml/AuthorizationPage.qml 2014-09-25 13:16:18 +0000
@@ -54,7 +54,7 @@
54 }54 }
55 onDelegateClicked: {55 onDelegateClicked: {
56 /* The last item in the model is the "Add another..." label */56 /* The last item in the model is the "Add another..." label */
57 if (index == model.count - 1) root.createAccount();57 if (model.lastItemText && index == model.count - 1) root.createAccount();
58 }58 }
59 }59 }
6060
6161
=== modified file 'online-accounts-ui/qml/ProviderRequest.qml'
--- online-accounts-ui/qml/ProviderRequest.qml 2014-09-17 07:47:13 +0000
+++ online-accounts-ui/qml/ProviderRequest.qml 2014-09-25 13:16:18 +0000
@@ -26,6 +26,7 @@
2626
27 property variant applicationInfo: application27 property variant applicationInfo: application
28 property variant providerInfo: provider28 property variant providerInfo: provider
29 property bool wasDenied: false
29 property int __createdAccountId: 030 property int __createdAccountId: 0
3031
31 signal denied32 signal denied
@@ -38,24 +39,32 @@
3839
39 Component.onCompleted: {40 Component.onCompleted: {
40 i18n.domain = "ubuntu-system-settings-online-accounts"41 i18n.domain = "ubuntu-system-settings-online-accounts"
42 if (accessModel.count === 0) {
43 /* No accounts to authorize */
44 denied()
45 return
46 }
41 loader.active = true47 loader.active = true
42 pageStack.push(mainPage)48 pageStack.push(mainPage)
43 }49 }
4450
45 on__CreatedAccountIdChanged: grantAccessIfReady()51 on__CreatedAccountIdChanged: grantAccessIfReady()
52 onDenied: wasDenied = true
4653
47 PageStack {54 PageStack {
48 id: pageStack55 id: pageStack
4956
50 Page {57 Page {
51 id: mainPage58 id: mainPage
52 title: i18n.tr("Accounts")59 title: providerInfo.displayName
5360
54 Loader {61 Loader {
55 id: loader62 id: loader
56 anchors.fill: parent63 anchors.fill: parent
57 active: false64 active: false
58 sourceComponent: (accessModel.count <= 1 || applicationInfo.id === "system-settings") ? accountCreationPage : authorizationPage65 sourceComponent: ((accessModel.count <= 1 && accessModel.canCreateAccounts()) ||
66 applicationInfo.id === "system-settings") ?
67 accountCreationPage : authorizationPage
59 onLoaded: {68 onLoaded: {
60 // use this trick to break the sourceComponent binding69 // use this trick to break the sourceComponent binding
61 var tmp = sourceComponent70 var tmp = sourceComponent
@@ -85,7 +94,12 @@
85 id: accessModel94 id: accessModel
86 accountModel: accountsModel95 accountModel: accountsModel
87 applicationId: applicationInfo.id96 applicationId: applicationInfo.id
88 lastItemText: i18n.tr("Add another")97 lastItemText: canCreateAccounts() ? i18n.tr("Add another") : ""
98
99 function canCreateAccounts() {
100 if (!providerInfo.isSingleAccount) return true
101 return accountsModel.count === 0
102 }
89 }103 }
90104
91 Component {105 Component {
92106
=== modified file 'tests/online-accounts-ui/tst_access_model.cpp'
--- tests/online-accounts-ui/tst_access_model.cpp 2014-02-03 10:27:39 +0000
+++ tests/online-accounts-ui/tst_access_model.cpp 2014-09-25 13:16:18 +0000
@@ -95,12 +95,25 @@
95 AccessModel *model = new AccessModel(this);95 AccessModel *model = new AccessModel(this);
96 QVERIFY(model != 0);96 QVERIFY(model != 0);
9797
98 QSignalSpy countChanged(model, SIGNAL(countChanged()));
99 QSignalSpy rowsInserted(model, SIGNAL(rowsInserted(const QModelIndex&,int,int)));
100 QSignalSpy rowsRemoved(model, SIGNAL(rowsRemoved(const QModelIndex&,int,int)));
101
98 QCOMPARE(model->lastItemText(), QString());102 QCOMPARE(model->lastItemText(), QString());
99 QCOMPARE(model->applicationId(), QString());103 QCOMPARE(model->applicationId(), QString());
100 QVERIFY(model->accountModel() == 0);104 QVERIFY(model->accountModel() == 0);
105 QCOMPARE(model->rowCount(), 0);
101106
102 model->setLastItemText("hello");107 model->setLastItemText("hello");
103 QCOMPARE(model->lastItemText(), QString("hello"));108 QCOMPARE(model->lastItemText(), QString("hello"));
109 QCOMPARE(countChanged.count(), 1);
110 countChanged.clear();
111 QCOMPARE(rowsInserted.count(), 1);
112 /* Compare the line numbers */
113 QCOMPARE(rowsInserted.at(0).at(1).toInt(), 0);
114 QCOMPARE(rowsInserted.at(0).at(2).toInt(), 0);
115 rowsInserted.clear();
116 QCOMPARE(rowsRemoved.count(), 0);
104117
105 QCOMPARE(model->rowCount(), 1);118 QCOMPARE(model->rowCount(), 1);
106 QCOMPARE(model->get(0, "displayName").toString(), QString("hello"));119 QCOMPARE(model->get(0, "displayName").toString(), QString("hello"));
@@ -108,6 +121,15 @@
108 model->setAccountModel(accountModel);121 model->setAccountModel(accountModel);
109 QCOMPARE(model->rowCount(), 1);122 QCOMPARE(model->rowCount(), 1);
110123
124 model->setLastItemText("");
125 QCOMPARE(model->lastItemText(), QString(""));
126 QCOMPARE(countChanged.count(), 1);
127 QCOMPARE(rowsInserted.count(), 0);
128 QCOMPARE(rowsRemoved.count(), 1);
129 /* Compare the line numbers */
130 QCOMPARE(rowsRemoved.at(0).at(1).toInt(), 0);
131 QCOMPARE(rowsRemoved.at(0).at(2).toInt(), 0);
132
111 delete model;133 delete model;
112 delete accountModel;134 delete accountModel;
113}135}
114136
=== modified file 'tests/online-accounts-ui/tst_application_manager.cpp'
--- tests/online-accounts-ui/tst_application_manager.cpp 2014-09-16 11:35:48 +0000
+++ tests/online-accounts-ui/tst_application_manager.cpp 2014-09-25 13:16:18 +0000
@@ -450,6 +450,7 @@
450 QTest::addColumn<QString>("contents");450 QTest::addColumn<QString>("contents");
451 QTest::addColumn<QString>("profile");451 QTest::addColumn<QString>("profile");
452 QTest::addColumn<QString>("packageDir");452 QTest::addColumn<QString>("packageDir");
453 QTest::addColumn<bool>("isSingleAccount");
453454
454 QTest::newRow("no profile") <<455 QTest::newRow("no profile") <<
455 "com.ubuntu.test_MyPlugin" <<456 "com.ubuntu.test_MyPlugin" <<
@@ -458,7 +459,19 @@
458 " <name>My Plugin</name>\n"459 " <name>My Plugin</name>\n"
459 "</provider>" <<460 "</provider>" <<
460 QString() <<461 QString() <<
461 QString();462 QString() <<
463 false;
464
465 QTest::newRow("single account") <<
466 "com.ubuntu.test_MyPlugin" <<
467 "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
468 "<provider id=\"com.ubuntu.test_MyPlugin\">\n"
469 " <name>My Plugin</name>\n"
470 " <single-account>true</single-account>\n"
471 "</provider>" <<
472 QString() <<
473 QString() <<
474 true;
462475
463 QTest::newRow("no package-dir") <<476 QTest::newRow("no package-dir") <<
464 "com.ubuntu.test_MyPlugin2" <<477 "com.ubuntu.test_MyPlugin2" <<
@@ -466,9 +479,11 @@
466 "<provider id=\"com.ubuntu.test_MyPlugin2\">\n"479 "<provider id=\"com.ubuntu.test_MyPlugin2\">\n"
467 " <name>My Plugin</name>\n"480 " <name>My Plugin</name>\n"
468 " <profile>com.ubuntu.test_MyPlugin2_0.2</profile>\n"481 " <profile>com.ubuntu.test_MyPlugin2_0.2</profile>\n"
482 " <single-account>false</single-account>\n"
469 "</provider>" <<483 "</provider>" <<
470 "com.ubuntu.test_MyPlugin2_0.2" <<484 "com.ubuntu.test_MyPlugin2_0.2" <<
471 QString();485 QString() <<
486 false;
472487
473 QTest::newRow("with package-dir") <<488 QTest::newRow("with package-dir") <<
474 "com.ubuntu.test_MyPlugin3" <<489 "com.ubuntu.test_MyPlugin3" <<
@@ -479,7 +494,8 @@
479 " <package-dir>/opt/click.ubuntu.com/something</package-dir>\n"494 " <package-dir>/opt/click.ubuntu.com/something</package-dir>\n"
480 "</provider>" <<495 "</provider>" <<
481 "com.ubuntu.test_MyPlugin3_0.2" <<496 "com.ubuntu.test_MyPlugin3_0.2" <<
482 "/opt/click.ubuntu.com/something";497 "/opt/click.ubuntu.com/something" <<
498 false;
483}499}
484500
485void ApplicationManagerTest::testProviderInfo()501void ApplicationManagerTest::testProviderInfo()
@@ -490,6 +506,7 @@
490 QFETCH(QString, contents);506 QFETCH(QString, contents);
491 QFETCH(QString, profile);507 QFETCH(QString, profile);
492 QFETCH(QString, packageDir);508 QFETCH(QString, packageDir);
509 QFETCH(bool, isSingleAccount);
493510
494 writeAccountsFile(providerId + ".provider", contents);511 writeAccountsFile(providerId + ".provider", contents);
495512
@@ -499,6 +516,7 @@
499 QCOMPARE(info.value("id").toString(), providerId);516 QCOMPARE(info.value("id").toString(), providerId);
500 QCOMPARE(info.value("profile").toString(), profile);517 QCOMPARE(info.value("profile").toString(), profile);
501 QCOMPARE(info.value("package-dir").toString(), packageDir);518 QCOMPARE(info.value("package-dir").toString(), packageDir);
519 QCOMPARE(info.value("isSingleAccount").toBool(), isSingleAccount);
502}520}
503521
504QTEST_MAIN(ApplicationManagerTest);522QTEST_MAIN(ApplicationManagerTest);

Subscribers

People subscribed via source and target branches

to all changes: