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

Subscribers

People subscribed via source and target branches