Online Accounts setup for Ubuntu Touch

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

  1. master
  2. Merge into trunk
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 :

PASSED: Continuous integration, rev:189
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-online-accounts-ci/166/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/5168
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-online-accounts-utopic-amd64-ci/59
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-online-accounts-utopic-i386-ci/59
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4913
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6420
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6420/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13732

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-online-accounts-ci/166/rebuild

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

PASSED: Continuous integration, rev:190
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-online-accounts-ci/167/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/5176
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-online-accounts-utopic-amd64-ci/60
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-online-accounts-utopic-i386-ci/60
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4921
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6428
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6428/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13746

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-online-accounts-ci/167/rebuild

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?

lp:~online-accounts/ubuntu-system-settings-online-accounts/master updated
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
1=== modified file 'online-accounts-ui/access-model.cpp'
2--- online-accounts-ui/access-model.cpp 2014-02-03 10:27:39 +0000
3+++ online-accounts-ui/access-model.cpp 2014-09-25 13:16:18 +0000
4@@ -159,7 +159,8 @@
5
6 int AccessModel::rowCount(const QModelIndex &parent) const
7 {
8- return QIdentityProxyModel::rowCount(parent) + 1;
9+ return QIdentityProxyModel::rowCount(parent) +
10+ (lastItemText().isEmpty() ? 0 : 1);
11 }
12
13 void AccessModel::setLastItemText(const QString &text)
14@@ -167,11 +168,32 @@
15 Q_D(AccessModel);
16
17 if (text == d->m_lastItemText) return;
18+
19+ bool wasEmpty = d->m_lastItemText.isEmpty();
20+ bool insertingRow = false;
21+ bool removingRow = false;
22+ int oldRowCount = rowCount();
23+ if (text.isEmpty() != wasEmpty) {
24+ if (wasEmpty) {
25+ beginInsertRows(QModelIndex(), oldRowCount, oldRowCount);
26+ insertingRow = true;
27+ } else {
28+ beginRemoveRows(QModelIndex(), oldRowCount - 1, oldRowCount - 1);
29+ removingRow = true;
30+ }
31+ }
32+
33 d->m_lastItemText = text;
34 Q_EMIT lastItemTextChanged();
35
36- QModelIndex lastItemIndex = index(d->rowCount(), 0);
37- Q_EMIT dataChanged(lastItemIndex, lastItemIndex);
38+ if (insertingRow) {
39+ endInsertRows();
40+ } else if (removingRow) {
41+ endRemoveRows();
42+ } else {
43+ QModelIndex lastItemIndex = index(oldRowCount - 1, 0);
44+ Q_EMIT dataChanged(lastItemIndex, lastItemIndex);
45+ }
46 }
47
48 QString AccessModel::lastItemText() const
49
50=== modified file 'online-accounts-ui/application-manager.cpp'
51--- online-accounts-ui/application-manager.cpp 2014-09-16 11:35:48 +0000
52+++ online-accounts-ui/application-manager.cpp 2014-09-25 13:16:18 +0000
53@@ -183,6 +183,7 @@
54 info.insert(QStringLiteral("id"), providerId);
55 info.insert(QStringLiteral("displayName"), provider.displayName());
56 info.insert(QStringLiteral("icon"), provider.iconName());
57+ info.insert(QStringLiteral("isSingleAccount"), provider.isSingleAccount());
58
59 /* Get Ubuntu-specific information directly from the XML file */
60 const QDomDocument doc = provider.domDocument();
61
62=== modified file 'online-accounts-ui/module/WebView.qml'
63--- online-accounts-ui/module/WebView.qml 2014-09-05 09:36:27 +0000
64+++ online-accounts-ui/module/WebView.qml 2014-09-25 13:16:18 +0000
65@@ -1,9 +1,8 @@
66 import QtQuick 2.0
67 import Ubuntu.Components 0.1
68-import Ubuntu.Components.Extras.Browser 0.2
69-import com.canonical.Oxide 1.0
70+import Ubuntu.Web 0.2
71
72-UbuntuWebView {
73+WebView {
74 property QtObject signonRequest
75
76 Component.onCompleted: {
77@@ -23,7 +22,7 @@
78 }
79 onUrlChanged: signonRequest.currentUrl = url
80
81- context: UbuntuWebContext {
82+ context: WebContext {
83 dataPath: signonRequest ? signonRequest.rootDir : ""
84 }
85
86
87=== modified file 'online-accounts-ui/qml/AccountCreationPage.qml'
88--- online-accounts-ui/qml/AccountCreationPage.qml 2014-03-19 12:14:14 +0000
89+++ online-accounts-ui/qml/AccountCreationPage.qml 2014-09-25 13:16:18 +0000
90@@ -20,15 +20,14 @@
91 import Ubuntu.Components 0.1
92 import Ubuntu.OnlineAccounts 0.1
93
94-Page {
95+Item {
96 id: root
97
98 property string providerId
99+ property var flickable: null
100
101 signal finished(int accountId)
102
103- title: account.provider.displayName
104-
105 Account {
106 id: account
107 objectHandle: Manager.createAccount(providerId)
108
109=== modified file 'online-accounts-ui/qml/AuthorizationPage.qml'
110--- online-accounts-ui/qml/AuthorizationPage.qml 2014-02-03 10:27:39 +0000
111+++ online-accounts-ui/qml/AuthorizationPage.qml 2014-09-25 13:16:18 +0000
112@@ -54,7 +54,7 @@
113 }
114 onDelegateClicked: {
115 /* The last item in the model is the "Add another..." label */
116- if (index == model.count - 1) root.createAccount();
117+ if (model.lastItemText && index == model.count - 1) root.createAccount();
118 }
119 }
120
121
122=== modified file 'online-accounts-ui/qml/ProviderRequest.qml'
123--- online-accounts-ui/qml/ProviderRequest.qml 2014-09-17 07:47:13 +0000
124+++ online-accounts-ui/qml/ProviderRequest.qml 2014-09-25 13:16:18 +0000
125@@ -26,6 +26,7 @@
126
127 property variant applicationInfo: application
128 property variant providerInfo: provider
129+ property bool wasDenied: false
130 property int __createdAccountId: 0
131
132 signal denied
133@@ -38,24 +39,32 @@
134
135 Component.onCompleted: {
136 i18n.domain = "ubuntu-system-settings-online-accounts"
137+ if (accessModel.count === 0) {
138+ /* No accounts to authorize */
139+ denied()
140+ return
141+ }
142 loader.active = true
143 pageStack.push(mainPage)
144 }
145
146 on__CreatedAccountIdChanged: grantAccessIfReady()
147+ onDenied: wasDenied = true
148
149 PageStack {
150 id: pageStack
151
152 Page {
153 id: mainPage
154- title: i18n.tr("Accounts")
155+ title: providerInfo.displayName
156
157 Loader {
158 id: loader
159 anchors.fill: parent
160 active: false
161- sourceComponent: (accessModel.count <= 1 || applicationInfo.id === "system-settings") ? accountCreationPage : authorizationPage
162+ sourceComponent: ((accessModel.count <= 1 && accessModel.canCreateAccounts()) ||
163+ applicationInfo.id === "system-settings") ?
164+ accountCreationPage : authorizationPage
165 onLoaded: {
166 // use this trick to break the sourceComponent binding
167 var tmp = sourceComponent
168@@ -85,7 +94,12 @@
169 id: accessModel
170 accountModel: accountsModel
171 applicationId: applicationInfo.id
172- lastItemText: i18n.tr("Add another")
173+ lastItemText: canCreateAccounts() ? i18n.tr("Add another") : ""
174+
175+ function canCreateAccounts() {
176+ if (!providerInfo.isSingleAccount) return true
177+ return accountsModel.count === 0
178+ }
179 }
180
181 Component {
182
183=== modified file 'tests/online-accounts-ui/tst_access_model.cpp'
184--- tests/online-accounts-ui/tst_access_model.cpp 2014-02-03 10:27:39 +0000
185+++ tests/online-accounts-ui/tst_access_model.cpp 2014-09-25 13:16:18 +0000
186@@ -95,12 +95,25 @@
187 AccessModel *model = new AccessModel(this);
188 QVERIFY(model != 0);
189
190+ QSignalSpy countChanged(model, SIGNAL(countChanged()));
191+ QSignalSpy rowsInserted(model, SIGNAL(rowsInserted(const QModelIndex&,int,int)));
192+ QSignalSpy rowsRemoved(model, SIGNAL(rowsRemoved(const QModelIndex&,int,int)));
193+
194 QCOMPARE(model->lastItemText(), QString());
195 QCOMPARE(model->applicationId(), QString());
196 QVERIFY(model->accountModel() == 0);
197+ QCOMPARE(model->rowCount(), 0);
198
199 model->setLastItemText("hello");
200 QCOMPARE(model->lastItemText(), QString("hello"));
201+ QCOMPARE(countChanged.count(), 1);
202+ countChanged.clear();
203+ QCOMPARE(rowsInserted.count(), 1);
204+ /* Compare the line numbers */
205+ QCOMPARE(rowsInserted.at(0).at(1).toInt(), 0);
206+ QCOMPARE(rowsInserted.at(0).at(2).toInt(), 0);
207+ rowsInserted.clear();
208+ QCOMPARE(rowsRemoved.count(), 0);
209
210 QCOMPARE(model->rowCount(), 1);
211 QCOMPARE(model->get(0, "displayName").toString(), QString("hello"));
212@@ -108,6 +121,15 @@
213 model->setAccountModel(accountModel);
214 QCOMPARE(model->rowCount(), 1);
215
216+ model->setLastItemText("");
217+ QCOMPARE(model->lastItemText(), QString(""));
218+ QCOMPARE(countChanged.count(), 1);
219+ QCOMPARE(rowsInserted.count(), 0);
220+ QCOMPARE(rowsRemoved.count(), 1);
221+ /* Compare the line numbers */
222+ QCOMPARE(rowsRemoved.at(0).at(1).toInt(), 0);
223+ QCOMPARE(rowsRemoved.at(0).at(2).toInt(), 0);
224+
225 delete model;
226 delete accountModel;
227 }
228
229=== modified file 'tests/online-accounts-ui/tst_application_manager.cpp'
230--- tests/online-accounts-ui/tst_application_manager.cpp 2014-09-16 11:35:48 +0000
231+++ tests/online-accounts-ui/tst_application_manager.cpp 2014-09-25 13:16:18 +0000
232@@ -450,6 +450,7 @@
233 QTest::addColumn<QString>("contents");
234 QTest::addColumn<QString>("profile");
235 QTest::addColumn<QString>("packageDir");
236+ QTest::addColumn<bool>("isSingleAccount");
237
238 QTest::newRow("no profile") <<
239 "com.ubuntu.test_MyPlugin" <<
240@@ -458,7 +459,19 @@
241 " <name>My Plugin</name>\n"
242 "</provider>" <<
243 QString() <<
244- QString();
245+ QString() <<
246+ false;
247+
248+ QTest::newRow("single account") <<
249+ "com.ubuntu.test_MyPlugin" <<
250+ "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
251+ "<provider id=\"com.ubuntu.test_MyPlugin\">\n"
252+ " <name>My Plugin</name>\n"
253+ " <single-account>true</single-account>\n"
254+ "</provider>" <<
255+ QString() <<
256+ QString() <<
257+ true;
258
259 QTest::newRow("no package-dir") <<
260 "com.ubuntu.test_MyPlugin2" <<
261@@ -466,9 +479,11 @@
262 "<provider id=\"com.ubuntu.test_MyPlugin2\">\n"
263 " <name>My Plugin</name>\n"
264 " <profile>com.ubuntu.test_MyPlugin2_0.2</profile>\n"
265+ " <single-account>false</single-account>\n"
266 "</provider>" <<
267 "com.ubuntu.test_MyPlugin2_0.2" <<
268- QString();
269+ QString() <<
270+ false;
271
272 QTest::newRow("with package-dir") <<
273 "com.ubuntu.test_MyPlugin3" <<
274@@ -479,7 +494,8 @@
275 " <package-dir>/opt/click.ubuntu.com/something</package-dir>\n"
276 "</provider>" <<
277 "com.ubuntu.test_MyPlugin3_0.2" <<
278- "/opt/click.ubuntu.com/something";
279+ "/opt/click.ubuntu.com/something" <<
280+ false;
281 }
282
283 void ApplicationManagerTest::testProviderInfo()
284@@ -490,6 +506,7 @@
285 QFETCH(QString, contents);
286 QFETCH(QString, profile);
287 QFETCH(QString, packageDir);
288+ QFETCH(bool, isSingleAccount);
289
290 writeAccountsFile(providerId + ".provider", contents);
291
292@@ -499,6 +516,7 @@
293 QCOMPARE(info.value("id").toString(), providerId);
294 QCOMPARE(info.value("profile").toString(), profile);
295 QCOMPARE(info.value("package-dir").toString(), packageDir);
296+ QCOMPARE(info.value("isSingleAccount").toBool(), isSingleAccount);
297 }
298
299 QTEST_MAIN(ApplicationManagerTest);

Subscribers

People subscribed via source and target branches

to all changes: