Merge lp:~ralsina/ubuntu-system-settings/per-package-notifications-rtm into lp:ubuntu-system-settings/rtm-14.09

Proposed by Roberto Alsina
Status: Merged
Approved by: Ken VanDine
Approved revision: 1007
Merged at revision: 1010
Proposed branch: lp:~ralsina/ubuntu-system-settings/per-package-notifications-rtm
Merge into: lp:ubuntu-system-settings/rtm-14.09
Diff against target: 248 lines (+146/-37)
3 files modified
plugins/notifications/notification_manager.cpp (+88/-37)
plugins/notifications/notification_manager.h (+1/-0)
tests/autopilot/ubuntu_system_settings/tests/test_notifications.py (+57/-0)
To merge this branch: bzr merge lp:~ralsina/ubuntu-system-settings/per-package-notifications-rtm
Reviewer Review Type Date Requested Status
Ken VanDine Approve
PS Jenkins bot continuous-integration Needs Fixing
Review via email: mp+254625@code.launchpad.net

Commit message

Use per-package notification settings instead of per-app and per-scope.

Description of the change

------------

To test:

* Using the current ubuntu-system-settings, with telegram installed, you have 2 telegram entries on the notifications panel

* Using this version you should have one.

* If you untick the telegram item, and run this command, you should see something like this:

$ gsettings get com.ubuntu.notifications.hub blacklist

[('com.ubuntu.telegram', 'sctelegram'), ('com.ubuntu.telegram', 'telegram')]

* Ticking the telegram item should make those two items disappear

-------------

Additionally, if either one of the telegram items were NOT ticked, then the single telegram item in the new version should be NOT ticked.

If any other app was ticked/not ticked with the current version, it should stay the same in the new version.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ken VanDine (ken-vandine) wrote :

Looks good

review: Approve
1008. By Roberto Alsina

new autopilot test for notifications plugin

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/notifications/notification_manager.cpp'
2--- plugins/notifications/notification_manager.cpp 2014-10-20 18:41:31 +0000
3+++ plugins/notifications/notification_manager.cpp 2015-03-31 21:20:56 +0000
4@@ -82,6 +82,7 @@
5 gchar *pkg;
6 gchar *app;
7 m_blacklist.clear();
8+ appnames_per_package.clear();
9 while (g_variant_iter_loop (iter, "(ss)", &pkg, &app)) {
10 m_blacklist[QString(pkg)+"::::"+app] = true;
11 }
12@@ -99,6 +100,7 @@
13 char *display_name;
14 char *icon_fname;
15 app_data_from_desktop_id(appid.toUtf8().constData(), &display_name, &icon_fname);
16+ qDebug() << m_blacklist;
17 bool blacklisted = m_blacklist.contains(key);
18 if (!display_name || !icon_fname) {
19 continue; // Broken .desktop file
20@@ -119,9 +121,9 @@
21 QJsonArray array = document.array();
22
23
24- // Iterate over all the installed click packages,
25- // and, for those packages that have a push-helper hook,
26- // list all apps (hooks entries with a desktop field)
27+ // Iterate over all the installed click packages,
28+ // and, list those packages that have a push-helper hook,
29+ // and either an app or a scope hook
30
31 for (int i = 0; i < array.size(); i++) { // This iterates over packages
32 QJsonObject object = array.at(i).toObject();
33@@ -145,50 +147,101 @@
34 continue;
35 }
36
37- // It has a helper, so add items for all entries that have
38- // a "desktop" key.
39+ // Check if package contains either a scope or an app
40+ // and get information from it
41+ bool has_app_or_scope = false;
42+ bool is_blacklisted = false;
43+ char *display_name = 0;
44+ char *icon_fname = 0;
45+ appnames_per_package[pkgname] = QStringList();
46 for (int j = 0; j < keys.size(); ++j) {
47 QString appname = keys.at(j);
48 QVariantMap hook = hooks.value(appname).toMap();
49 if (hook.contains("desktop") || hook.contains("scope")) {
50+ has_app_or_scope = true;
51+ // Check if it should be enabled or disabled.
52+ // Because of bug #1434181 if any of the apps or scope is marked as blacklisted,
53+ // blacklist the package
54 QString key = pkgname+"::::"+appname;
55+ if (m_blacklist.contains(key)) {
56+ is_blacklisted = true;
57+ }
58 QString appid = pkgname+"_"+appname+"_"+version+".desktop"; // Full versioned APP_ID + ".desktop"
59- char *display_name;
60- char *icon_fname;
61- app_data_from_desktop_id(appid.toUtf8().constData(), &display_name, &icon_fname);
62- // fall back to the manifest's title & icon if missing from .desktop
63- if (!display_name) {
64- display_name = g_strdup(object.value("title").toString().toUtf8().data());
65- }
66- if (!icon_fname) {
67- icon_fname = g_strdup(object.value("icon").toString().toUtf8().data());
68- }
69- NotificationItem *item = new NotificationItem();
70- bool blacklisted = m_blacklist.contains(key);
71- item->setItemData(QString(display_name), QString(icon_fname), !blacklisted, key);
72- g_free(display_name);
73- g_free(icon_fname);
74- m_model.append(QVariant::fromValue(item));
75- connect(item, &NotificationItem::updateNotificationStatus,
76- this, &NotificationsManager::checkUpdates);
77+ appnames_per_package[pkgname].append(appname);
78+ // Get the icon and display name from the appid if possible
79+ char *d_name;
80+ char *i_fname;
81+ app_data_from_desktop_id(appid.toUtf8().constData(), &d_name, &i_fname);
82+ if (!display_name && d_name) {
83+ display_name = g_strdup(d_name);
84+ }
85+ if (!icon_fname && i_fname) {
86+ icon_fname = g_strdup(i_fname);
87+ }
88+ g_free(d_name);
89+ g_free(i_fname);
90 }
91 }
92+
93+ // Has app or scope and helper, show
94+ if (has_app_or_scope) {
95+ // If we still don't have icon or title, because no app or scope has them,
96+ // fallback to getting them from the package
97+
98+ if (!display_name) {
99+ display_name = g_strdup(object.value("title").toString().toUtf8().data());
100+ }
101+ if (!icon_fname) {
102+ icon_fname = g_strdup(object.value("icon").toString().toUtf8().data());
103+ }
104+ NotificationItem *item = new NotificationItem();
105+ item->setItemData(QString(display_name), QString(icon_fname), !is_blacklisted, pkgname);
106+ g_free(display_name);
107+ g_free(icon_fname);
108+ m_model.append(QVariant::fromValue(item));
109+ connect(item, &NotificationItem::updateNotificationStatus,
110+ this, &NotificationsManager::checkUpdates);
111+ }
112 }
113 Q_EMIT modelChanged();
114 }
115
116-void NotificationsManager::checkUpdates(QString key, bool value)
117+void NotificationsManager::checkUpdates(QString pkgname, bool value)
118 {
119- // Update the internal blacklist
120- if (!value) {
121- if (!m_blacklist.contains(key)) {
122- m_blacklist[key] = true;
123- }
124- } else {
125- if (m_blacklist.contains(key)) {
126- m_blacklist.remove(key);
127- }
128- }
129+ // Update in internal blacklist all pkgname::appname for this
130+ // package.
131+ std::cout << "==>" << pkgname.toStdString() << value << "\n";
132+
133+ // If pkgname starts with "::::" it's a legacy app
134+ if (pkgname.startsWith("::::")) {
135+ if (!value) {
136+ if (!m_blacklist.contains(pkgname)) {
137+ m_blacklist[pkgname] = true;
138+ }
139+ } else {
140+ if (m_blacklist.contains(pkgname)) {
141+ m_blacklist.remove(pkgname);
142+ }
143+ }
144+ }
145+ else {
146+ // It's a click, need to set for all apps/scopes
147+ for (int i = 0; i < appnames_per_package[pkgname].size(); ++i) {
148+ QString appname = appnames_per_package[pkgname][i];
149+ QString key = pkgname+"::::"+appname;
150+ // Update the internal blacklist
151+ if (!value) {
152+ if (!m_blacklist.contains(key)) {
153+ m_blacklist[key] = true;
154+ }
155+ } else {
156+ if (m_blacklist.contains(key)) {
157+ m_blacklist.remove(key);
158+ }
159+ }
160+ }
161+ }
162+
163 // Save the config settings
164 GVariantBuilder builder;
165 g_variant_builder_init(&builder, G_VARIANT_TYPE("a(ss)"));
166@@ -206,6 +259,4 @@
167 }
168 g_settings_set_value(m_pushSettings, BLACKLIST_KEY, g_variant_builder_end (&builder));
169 }
170-
171-}
172-
173+}
174\ No newline at end of file
175
176=== modified file 'plugins/notifications/notification_manager.h'
177--- plugins/notifications/notification_manager.h 2014-08-05 16:19:38 +0000
178+++ plugins/notifications/notification_manager.h 2015-03-31 21:20:56 +0000
179@@ -52,6 +52,7 @@
180 QProcess *m_process;
181 GSettings *m_pushSettings;
182 QMap <QString, bool> m_blacklist;
183+ QMap <QString, QStringList> appnames_per_package;
184 };
185
186 }
187
188=== added file 'tests/autopilot/ubuntu_system_settings/tests/test_notifications.py'
189--- tests/autopilot/ubuntu_system_settings/tests/test_notifications.py 1970-01-01 00:00:00 +0000
190+++ tests/autopilot/ubuntu_system_settings/tests/test_notifications.py 2015-03-31 21:20:56 +0000
191@@ -0,0 +1,57 @@
192+# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
193+# Copyright 2013 Canonical
194+#
195+# This program is free software: you can redistribute it and/or modify it
196+# under the terms of the GNU General Public License version 3, as published
197+# by the Free Software Foundation.
198+
199+import json
200+import os
201+import subprocess
202+import time
203+
204+from testtools.matchers import Equals, NotEquals
205+
206+from ubuntu_system_settings.tests import UbuntuSystemSettingsTestCase
207+
208+from ubuntuuitoolkit import emulators as toolkit_emulators
209+
210+""" Tests for Ubuntu System Settings """
211+
212+
213+def has_helper(package):
214+ """Return True if package['hooks']['foo']['push-helper'] exists"""
215+ return any(package['hooks'][hook].get('push-helper', None)
216+ for hook in package['hooks'])
217+
218+
219+class NotificationsTestCases(UbuntuSystemSettingsTestCase):
220+ """ Tests for Search """
221+
222+ def setUp(self):
223+ # Check legacy items: one for each file in legacy-helpers
224+ super(NotificationsTestCases, self).setUp()
225+
226+ def test_item_counts(self):
227+ """ Checks whether the Notificatins category is available """
228+ try:
229+ legacy_count = len(
230+ os.listdir("/usr/lib/ubuntu-push-client/legacy-helpers/"))
231+ except os.FileNotFoundError:
232+ legacy_count = 0
233+ # Get output of click list --manifest, and parse for the rest
234+ packages = json.loads(
235+ subprocess.check_output(
236+ ['click', 'list', '--manifest']).decode('utf8'))
237+ click_count = len([x for x in packages if has_helper(x)])
238+
239+ notif = self.main_view.select_single(
240+ objectName='entryComponent-notifications')
241+ self.assertThat(notif, NotEquals(None))
242+ self.main_view.pointing_device.click_object(notif)
243+ # Have to wait until the model loads
244+ time.sleep(1)
245+ notif_page = self.main_view.select_single(
246+ objectName='systemNotificationsPage')
247+ items = notif_page.select_many(toolkit_emulators.Standard)
248+ self.assertThat(len(items), Equals(click_count + legacy_count))

Subscribers

People subscribed via source and target branches