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

Proposed by Alberto Mardegan
Status: Merged
Merged at revision: 189
Proposed branch: lp:~mardy/ubuntu-system-settings-online-accounts/lp1373288
Merge into: lp:~online-accounts/ubuntu-system-settings-online-accounts/master
Diff against target: 246 lines (+86/-9)
6 files modified
online-accounts-ui/access-model.cpp (+25/-3)
online-accounts-ui/application-manager.cpp (+1/-0)
online-accounts-ui/qml/AuthorizationPage.qml (+1/-1)
online-accounts-ui/qml/ProviderRequest.qml (+16/-2)
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:~mardy/ubuntu-system-settings-online-accounts/lp1373288
Reviewer Review Type Date Requested Status
David Barth (community) Approve
Review via email: mp+235914@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.
190. By Alberto Mardegan

Merge from master

Revision history for this message
David Barth (dbarth) wrote :

LGTM

review: Approve

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 08:53:16 +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 08:53:16 +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/qml/AuthorizationPage.qml'
63--- online-accounts-ui/qml/AuthorizationPage.qml 2014-02-03 10:27:39 +0000
64+++ online-accounts-ui/qml/AuthorizationPage.qml 2014-09-25 08:53:16 +0000
65@@ -54,7 +54,7 @@
66 }
67 onDelegateClicked: {
68 /* The last item in the model is the "Add another..." label */
69- if (index == model.count - 1) root.createAccount();
70+ if (model.lastItemText && index == model.count - 1) root.createAccount();
71 }
72 }
73
74
75=== modified file 'online-accounts-ui/qml/ProviderRequest.qml'
76--- online-accounts-ui/qml/ProviderRequest.qml 2014-09-17 07:47:13 +0000
77+++ online-accounts-ui/qml/ProviderRequest.qml 2014-09-25 08:53:16 +0000
78@@ -26,6 +26,7 @@
79
80 property variant applicationInfo: application
81 property variant providerInfo: provider
82+ property bool wasDenied: false
83 property int __createdAccountId: 0
84
85 signal denied
86@@ -38,11 +39,17 @@
87
88 Component.onCompleted: {
89 i18n.domain = "ubuntu-system-settings-online-accounts"
90+ if (accessModel.count === 0) {
91+ /* No accounts to authorize */
92+ denied()
93+ return
94+ }
95 loader.active = true
96 pageStack.push(mainPage)
97 }
98
99 on__CreatedAccountIdChanged: grantAccessIfReady()
100+ onDenied: wasDenied = true
101
102 PageStack {
103 id: pageStack
104@@ -55,7 +62,9 @@
105 id: loader
106 anchors.fill: parent
107 active: false
108- sourceComponent: (accessModel.count <= 1 || applicationInfo.id === "system-settings") ? accountCreationPage : authorizationPage
109+ sourceComponent: ((accessModel.count <= 1 && accessModel.canCreateAccounts()) ||
110+ applicationInfo.id === "system-settings") ?
111+ accountCreationPage : authorizationPage
112 onLoaded: {
113 // use this trick to break the sourceComponent binding
114 var tmp = sourceComponent
115@@ -85,7 +94,12 @@
116 id: accessModel
117 accountModel: accountsModel
118 applicationId: applicationInfo.id
119- lastItemText: i18n.tr("Add another")
120+ lastItemText: canCreateAccounts() ? i18n.tr("Add another") : ""
121+
122+ function canCreateAccounts() {
123+ if (!providerInfo.isSingleAccount) return true
124+ return accountsModel.count === 0
125+ }
126 }
127
128 Component {
129
130=== modified file 'tests/online-accounts-ui/tst_access_model.cpp'
131--- tests/online-accounts-ui/tst_access_model.cpp 2014-02-03 10:27:39 +0000
132+++ tests/online-accounts-ui/tst_access_model.cpp 2014-09-25 08:53:16 +0000
133@@ -95,12 +95,25 @@
134 AccessModel *model = new AccessModel(this);
135 QVERIFY(model != 0);
136
137+ QSignalSpy countChanged(model, SIGNAL(countChanged()));
138+ QSignalSpy rowsInserted(model, SIGNAL(rowsInserted(const QModelIndex&,int,int)));
139+ QSignalSpy rowsRemoved(model, SIGNAL(rowsRemoved(const QModelIndex&,int,int)));
140+
141 QCOMPARE(model->lastItemText(), QString());
142 QCOMPARE(model->applicationId(), QString());
143 QVERIFY(model->accountModel() == 0);
144+ QCOMPARE(model->rowCount(), 0);
145
146 model->setLastItemText("hello");
147 QCOMPARE(model->lastItemText(), QString("hello"));
148+ QCOMPARE(countChanged.count(), 1);
149+ countChanged.clear();
150+ QCOMPARE(rowsInserted.count(), 1);
151+ /* Compare the line numbers */
152+ QCOMPARE(rowsInserted.at(0).at(1).toInt(), 0);
153+ QCOMPARE(rowsInserted.at(0).at(2).toInt(), 0);
154+ rowsInserted.clear();
155+ QCOMPARE(rowsRemoved.count(), 0);
156
157 QCOMPARE(model->rowCount(), 1);
158 QCOMPARE(model->get(0, "displayName").toString(), QString("hello"));
159@@ -108,6 +121,15 @@
160 model->setAccountModel(accountModel);
161 QCOMPARE(model->rowCount(), 1);
162
163+ model->setLastItemText("");
164+ QCOMPARE(model->lastItemText(), QString(""));
165+ QCOMPARE(countChanged.count(), 1);
166+ QCOMPARE(rowsInserted.count(), 0);
167+ QCOMPARE(rowsRemoved.count(), 1);
168+ /* Compare the line numbers */
169+ QCOMPARE(rowsRemoved.at(0).at(1).toInt(), 0);
170+ QCOMPARE(rowsRemoved.at(0).at(2).toInt(), 0);
171+
172 delete model;
173 delete accountModel;
174 }
175
176=== modified file 'tests/online-accounts-ui/tst_application_manager.cpp'
177--- tests/online-accounts-ui/tst_application_manager.cpp 2014-09-16 11:35:48 +0000
178+++ tests/online-accounts-ui/tst_application_manager.cpp 2014-09-25 08:53:16 +0000
179@@ -450,6 +450,7 @@
180 QTest::addColumn<QString>("contents");
181 QTest::addColumn<QString>("profile");
182 QTest::addColumn<QString>("packageDir");
183+ QTest::addColumn<bool>("isSingleAccount");
184
185 QTest::newRow("no profile") <<
186 "com.ubuntu.test_MyPlugin" <<
187@@ -458,7 +459,19 @@
188 " <name>My Plugin</name>\n"
189 "</provider>" <<
190 QString() <<
191- QString();
192+ QString() <<
193+ false;
194+
195+ QTest::newRow("single account") <<
196+ "com.ubuntu.test_MyPlugin" <<
197+ "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
198+ "<provider id=\"com.ubuntu.test_MyPlugin\">\n"
199+ " <name>My Plugin</name>\n"
200+ " <single-account>true</single-account>\n"
201+ "</provider>" <<
202+ QString() <<
203+ QString() <<
204+ true;
205
206 QTest::newRow("no package-dir") <<
207 "com.ubuntu.test_MyPlugin2" <<
208@@ -466,9 +479,11 @@
209 "<provider id=\"com.ubuntu.test_MyPlugin2\">\n"
210 " <name>My Plugin</name>\n"
211 " <profile>com.ubuntu.test_MyPlugin2_0.2</profile>\n"
212+ " <single-account>false</single-account>\n"
213 "</provider>" <<
214 "com.ubuntu.test_MyPlugin2_0.2" <<
215- QString();
216+ QString() <<
217+ false;
218
219 QTest::newRow("with package-dir") <<
220 "com.ubuntu.test_MyPlugin3" <<
221@@ -479,7 +494,8 @@
222 " <package-dir>/opt/click.ubuntu.com/something</package-dir>\n"
223 "</provider>" <<
224 "com.ubuntu.test_MyPlugin3_0.2" <<
225- "/opt/click.ubuntu.com/something";
226+ "/opt/click.ubuntu.com/something" <<
227+ false;
228 }
229
230 void ApplicationManagerTest::testProviderInfo()
231@@ -490,6 +506,7 @@
232 QFETCH(QString, contents);
233 QFETCH(QString, profile);
234 QFETCH(QString, packageDir);
235+ QFETCH(bool, isSingleAccount);
236
237 writeAccountsFile(providerId + ".provider", contents);
238
239@@ -499,6 +516,7 @@
240 QCOMPARE(info.value("id").toString(), providerId);
241 QCOMPARE(info.value("profile").toString(), profile);
242 QCOMPARE(info.value("package-dir").toString(), packageDir);
243+ QCOMPARE(info.value("isSingleAccount").toBool(), isSingleAccount);
244 }
245
246 QTEST_MAIN(ApplicationManagerTest);

Subscribers

People subscribed via source and target branches