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

[Roberto Alsina]

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

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

It's compatible with previous version, and now has the following semantics:

* Uncheck a package: all apps/scopes in that package are now blacklisted
* Check a package: all apps/scopes in that package are whitelisted
* If it reads a config from an older version and ONE of the apps/scopes in a package is blacklisted, the whole package and all its apps/scopes will now be blacklisted.

For legacy apps/helpers, like system settings there is no 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.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1361
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/2039/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2037
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-vivid-i386-ci/309
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1793
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2035
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2035/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19281

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

review: Approve (continuous-integration)

[Ken VanDine (ken-vandine)]

Looks fine and works as described

review: Approve

[lp:~ralsina/ubuntu-system-settings/per-package-notifications]

1362. By Roberto Alsina on 2015-03-31

merged trunk

1363. By Roberto Alsina on 2015-03-31

added autopilot test to check the right number of items is shown

1364. By Roberto Alsina on 2015-03-31

style fixes

=== 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 19:38:32 +0000
@@ -82,6 +82,7 @@
     gchar *pkg;
     gchar *app;
     m_blacklist.clear();
+    appnames_per_package.clear();
     while (g_variant_iter_loop (iter, "(ss)", &pkg, &app)) {
         m_blacklist[QString(pkg)+"::::"+app] = true;
     }
@@ -99,6 +100,7 @@
             char *display_name;
             char *icon_fname;
             app_data_from_desktop_id(appid.toUtf8().constData(), &display_name, &icon_fname);
+            qDebug() << m_blacklist;
             bool blacklisted = m_blacklist.contains(key);
             if (!display_name || !icon_fname) {
                 continue; // Broken .desktop file
@@ -119,9 +121,9 @@
     QJsonArray array = document.array();
 
 
-	// Iterate over all the installed click packages,
-	// and, for those packages that have a push-helper hook,
-	// list all apps (hooks entries with a desktop field)
+    // Iterate over all the installed click packages,
+    // and, list those packages that have a push-helper hook,
+    // and either an app or a scope hook
 
     for (int i = 0; i < array.size(); i++) { // This iterates over packages
         QJsonObject object = array.at(i).toObject();
@@ -145,50 +147,101 @@
             continue;
         }
 
-        // It has a helper, so add items for all entries that have
-        // a "desktop" key.
+        // Check if package contains either a scope or an app
+        // and get information from it
+        bool has_app_or_scope = false;
+        bool is_blacklisted = false;
+        char *display_name = 0;
+        char *icon_fname = 0;
+        appnames_per_package[pkgname] = QStringList();
         for (int j = 0; j < keys.size(); ++j) {
             QString appname = keys.at(j);
             QVariantMap hook = hooks.value(appname).toMap();
             if (hook.contains("desktop") || hook.contains("scope")) {
+                has_app_or_scope = true;
+                // Check if it should be enabled or disabled.
+                // Because of bug #1434181 if any of the apps or scope is marked as blacklisted,
+                // blacklist the package
                 QString key = pkgname+"::::"+appname;
+                if (m_blacklist.contains(key)) {
+                    is_blacklisted = true;
+                }
                 QString appid = pkgname+"_"+appname+"_"+version+".desktop"; // Full versioned APP_ID + ".desktop"
-                char *display_name;
-                char *icon_fname;
-                app_data_from_desktop_id(appid.toUtf8().constData(), &display_name, &icon_fname);
-                // fall back to the manifest's title & icon if missing from .desktop
-                if (!display_name) {
-                    display_name = g_strdup(object.value("title").toString().toUtf8().data());
-                }
-                if (!icon_fname) {
-                    icon_fname =  g_strdup(object.value("icon").toString().toUtf8().data());
-                }
-                NotificationItem *item = new NotificationItem();
-                bool blacklisted = m_blacklist.contains(key);
-                item->setItemData(QString(display_name), QString(icon_fname), !blacklisted, key);
-                g_free(display_name);
-                g_free(icon_fname);
-                m_model.append(QVariant::fromValue(item));
-                connect(item, &NotificationItem::updateNotificationStatus,
-                        this, &NotificationsManager::checkUpdates);
+                appnames_per_package[pkgname].append(appname);
+                // Get the icon and display name from the appid if possible
+                char *d_name;
+                char *i_fname;
+                app_data_from_desktop_id(appid.toUtf8().constData(), &d_name, &i_fname);
+                if (!display_name && d_name) {
+                    display_name = g_strdup(d_name);
+                }
+                if (!icon_fname && i_fname) {
+                    icon_fname = g_strdup(i_fname);
+                }
+                g_free(d_name);
+                g_free(i_fname);
             }
         }
+
+        // Has app or scope and helper, show
+        if (has_app_or_scope) {
+            // If we still don't have icon or title, because no app or scope has them, 
+            // fallback to getting them from the package
+                    
+            if (!display_name) {
+                display_name = g_strdup(object.value("title").toString().toUtf8().data());
+            }
+            if (!icon_fname) {
+                icon_fname =  g_strdup(object.value("icon").toString().toUtf8().data());
+            }
+            NotificationItem *item = new NotificationItem();
+            item->setItemData(QString(display_name), QString(icon_fname), !is_blacklisted, pkgname);
+            g_free(display_name);
+            g_free(icon_fname);
+            m_model.append(QVariant::fromValue(item));
+            connect(item, &NotificationItem::updateNotificationStatus,
+                    this, &NotificationsManager::checkUpdates);
+        }        
     }
     Q_EMIT modelChanged();
 }
 
-void NotificationsManager::checkUpdates(QString key, bool value)
+void NotificationsManager::checkUpdates(QString pkgname, bool value)
 {
-    // Update the internal blacklist
-    if (!value) {
-        if (!m_blacklist.contains(key)) {
-            m_blacklist[key] = true;
-        }
-    } else {
-        if (m_blacklist.contains(key)) {
-            m_blacklist.remove(key);
-        }
-    }
+    // Update in internal blacklist all pkgname::appname for this
+    // package.
+    std::cout << "==>" << pkgname.toStdString() << value << "\n";
+    
+    // If pkgname starts with "::::" it's a legacy app
+    if (pkgname.startsWith("::::")) {
+        if (!value) {
+            if (!m_blacklist.contains(pkgname)) {
+                m_blacklist[pkgname] = true;
+            }
+        } else {
+            if (m_blacklist.contains(pkgname)) {
+                m_blacklist.remove(pkgname);
+            }
+        }
+    }
+    else {
+        // It's a click, need to set for all apps/scopes
+        for (int i = 0; i < appnames_per_package[pkgname].size(); ++i) {
+            QString appname = appnames_per_package[pkgname][i];
+            QString key = pkgname+"::::"+appname;
+            // Update the internal blacklist
+            if (!value) {
+                if (!m_blacklist.contains(key)) {
+                    m_blacklist[key] = true;
+                }
+            } else {
+                if (m_blacklist.contains(key)) {
+                    m_blacklist.remove(key);
+                }
+            }
+        }
+    }
+    
     // Save the config settings
     GVariantBuilder builder;
     g_variant_builder_init(&builder, G_VARIANT_TYPE("a(ss)"));
@@ -206,6 +259,4 @@
     }
     g_settings_set_value(m_pushSettings, BLACKLIST_KEY, g_variant_builder_end (&builder));
 }
-
-}
-
+}
\ No newline at end of file

=== 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 19:38:32 +0000
@@ -52,6 +52,7 @@
     QProcess *m_process;
     GSettings *m_pushSettings;
     QMap <QString, bool> m_blacklist;
+    QMap <QString, QStringList> appnames_per_package;
 };
 
 }

=== 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 19:38:32 +0000
@@ -0,0 +1,57 @@
+# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
+# Copyright 2013 Canonical
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+
+import json
+import os
+import subprocess
+import time
+
+from testtools.matchers import Equals, NotEquals
+
+from ubuntu_system_settings.tests import UbuntuSystemSettingsTestCase
+
+from ubuntuuitoolkit import emulators as toolkit_emulators
+
+""" Tests for Ubuntu System Settings """
+
+
+def has_helper(package):
+    """Return True if  package['hooks']['foo']['push-helper'] exists"""
+    return any(package['hooks'][hook].get('push-helper', None)
+               for hook in package['hooks'])
+
+
+class NotificationsTestCases(UbuntuSystemSettingsTestCase):
+    """ Tests for Search """
+
+    def setUp(self):
+        # Check legacy items: one for each file in legacy-helpers
+        super(NotificationsTestCases, self).setUp()
+
+    def test_item_counts(self):
+        """ Checks whether the Notificatins category is available """
+        try:
+            legacy_count = len(
+                os.listdir("/usr/lib/ubuntu-push-client/legacy-helpers/"))
+        except os.FileNotFoundError:
+            legacy_count = 0
+        # Get output of click list --manifest, and parse for the rest
+        packages = json.loads(
+            subprocess.check_output(
+                ['click', 'list', '--manifest']).decode('utf8'))
+        click_count = len([x for x in packages if has_helper(x)])
+
+        notif = self.main_view.select_single(
+            objectName='entryComponent-notifications')
+        self.assertThat(notif, NotEquals(None))
+        self.main_view.pointing_device.click_object(notif)
+        # Have to wait until the model loads
+        time.sleep(1)
+        notif_page = self.main_view.select_single(
+            objectName='systemNotificationsPage')
+        items = notif_page.select_many(toolkit_emulators.Standard)
+        self.assertThat(len(items), Equals(click_count + legacy_count))