Merge lp:~ted/gsettings-ubuntu-touch-schemas/notifications into lp:gsettings-ubuntu-touch-schemas

[Ted Gould]

Add schema for popup notification blacklist

Adding schema so that we can have a shared location between system settings and the post office of which applications are in the popup notification user blacklist. This should be used to block notifications if they're bothering the user.

This MR also adds an Upstart job and Click hook to clean up the list as legacy and click applications are removed from the system. In both cases a small Python script is run to ensure that all the entires in the setting are valid and removes those that are not.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:46
http://jenkins.qa.ubuntu.com/job/gsettings-ubuntu-touch-schemas-ci/23/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/gsettings-ubuntu-touch-schemas-utopic-amd64-ci/3
    SUCCESS: http://jenkins.qa.ubuntu.com/job/gsettings-ubuntu-touch-schemas-utopic-armhf-ci/3
    SUCCESS: http://jenkins.qa.ubuntu.com/job/gsettings-ubuntu-touch-schemas-utopic-i386-ci/3

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/gsettings-ubuntu-touch-schemas-ci/23/rebuild

review: Approve (continuous-integration)

[Iain Lane (laney)]

Why is the cleanup necessary? If I uninstall and then reinstall the next day my setting should be remembered, shouldn't

Regardless, I don't like doing this from the schemas package. It's not the place for housekeeping tasks, and the current attempt to run it once a day doesn't feel very nice to me. I would either have system-settings prune the list when it displays it (AIUI it's going to have the toggles) or the service consuming the key run the cleanup.

[Ted Gould (ted)]

On Thu, 2014-06-26 at 08:59 +0000, Iain Lane wrote:

> Why is the cleanup necessary? If I uninstall and then reinstall the
> next day my setting should be remembered, shouldn't

One, we don't know if you're ever going to reinstall the package. Two,
it provides a way to clear a setting that you might have forgotten out
to set. "This application is broken, it stopped sending notifications,
let me reinstall it and see if that fixes it."

> Regardless, I don't like doing this from the schemas package. It's not
> the place for housekeeping tasks, and the current attempt to run it
> once a day doesn't feel very nice to me. I would either have
> system-settings prune the list when it displays it (AIUI it's going to
> have the toggles) or the service consuming the key run the cleanup.

To me it felt like a "library function" of the service and the settings
app. And the "library" that they share is the schemas package. If for
some reason one or the other gets refactored or changed we don't have to
ensure cleanup gets migrated with that, it's part of the shared code
around the settings.

[John Lenton (chipaca)]

So, with this, popups can be blacklisted through this setting, sounds (and vibrations?) can be muted elsewhere. Messaging menu/notification centre entries, emblem counters, and led light whatsits (haven't looked into those yet so don't know what they're called) can't be disabled.

Is that the intent?

[John Lenton (chipaca)]

Actually, I'll talk with mpt about changing the settings to be disable all "interrupting" notifications per-app (that is, disabling bubbles, sounds and vibrations). It seems more sensible than disabling just popups. If he agrees, can we rename this too?

[Ted Gould (ted)]

On Tue, 2014-07-08 at 12:15 +0000, John Lenton wrote:

> Actually, I'll talk with mpt about changing the settings to be disable
> all "interrupting" notifications per-app (that is, disabling bubbles,
> sounds and vibrations). It seems more sensible than disabling just
> popups. If he agrees, can we rename this too?

Sure, I was just mapping to the system settings design. If that changes,
I think this should adapt as well.

[John Lenton (chipaca)]

WFM. Let's land this, and change it if the design changes.

review: Approve

[Roberto Alsina (ralsina)]

Just to be extra sure: for click apps then this expects an array of (s,s) where the strings are like:

"com.ubuntu.developer.ralsina.hello" "com.ubuntu.developer.ralsina.hello_hello" ?

Or

"com.ubuntu.developer.ralsina.hello" "com.ubuntu.developer.ralsina.hello_hello_0.2" ?

Or

"com.ubuntu.developer.ralsina.hello" "hello" ?

[Thomas Voß (thomas-voss)]

> On Thu, 2014-06-26 at 08:59 +0000, Iain Lane wrote:
>
> > Why is the cleanup necessary? If I uninstall and then reinstall the
> > next day my setting should be remembered, shouldn't
>
>
> One, we don't know if you're ever going to reinstall the package. Two,
> it provides a way to clear a setting that you might have forgotten out
> to set. "This application is broken, it stopped sending notifications,
> let me reinstall it and see if that fixes it."
>
>
> > Regardless, I don't like doing this from the schemas package. It's not
> > the place for housekeeping tasks, and the current attempt to run it
> > once a day doesn't feel very nice to me. I would either have
> > system-settings prune the list when it displays it (AIUI it's going to
> > have the toggles) or the service consuming the key run the cleanup.
>
>
> To me it felt like a "library function" of the service and the settings
> app. And the "library" that they share is the schemas package. If for
> some reason one or the other gets refactored or changed we don't have to
> ensure cleanup gets migrated with that, it's part of the shared code
> around the settings.

Well, that's the difficult part of just sharing settings instead of putting a "real" library in place. I see your point about sharing those keys, but I agree with Laney that we shouldn't rely on a daily task to cleanup.

[Thomas Voß (thomas-voss)]

As pointed out before: We should encode the cleanup logic in a different place. I'm fine with the schema and sharing the keys. Briefly checked with security, and gsettings/dconf is fine for system services.

review: Needs Fixing

[Ted Gould (ted)]

So clearly I'm outvoted on this one. Do you folks think that we should
remove the Upstart job and the Click hook or just the Upstart job. I
think the Upstart job is more hacky considering we don't get a notice in
the session of the package being removed. But I think the click hook
makes more sense to be centrally located.

[Thomas Voß (thomas-voss)]

> So clearly I'm outvoted on this one. Do you folks think that we should
> remove the Upstart job and the Click hook or just the Upstart job. I
> think the Upstart job is more hacky considering we don't get a notice in
> the session of the package being removed. But I think the click hook
> makes more sense to be centrally located.

So I think we should just keep the Click hook in place for the time being. Admittedly, legacy applications are not captured by removing the upstart hook, but we can tackle that issue going forward, with a little more time to think it through.

[Roberto Alsina (ralsina)]

As a paliative, system-settings can do some cleanup in that clicks that are not installed are going to disappear from the list, and on save will disappear from the settings as well. Of course if the user forgot the settings page existed it doesn't help at all.

[Iain Lane (laney)]

It's been a philosophical position of mine that this schemas project is only about the schemas themselves and it does not care about their contents. A few months ago we had a skirmish around the sounds - the schemas grew a dependency on the default set of ringtones. We came to the realisation that this was an error and reverted the change.

I'd like to maintain that position here. I don't want gsettings-ubuntu-touch-schemas to be a repository for cleanup scripts. In this case, as I mentioned earlier, the hook can move to whatever is displaying the notification, or system settings can get this logic if it's not so important to do this housekeeping regularly. I don't think we should be worrying about possible future refactorings losing random bits of still-useful code, because we would have some problems if that were taking place and need to make sure it doesn't.

[John Lenton (chipaca)]

I've proposed just the settings at https://code.launchpad.net/~chipaca/gsettings-ubuntu-touch-schemas/just-the-touch-settings/+merge/228317

The cleanup scripts will be picked up by the touch client itself.

[Sebastien Bacher (seb128)]

Should we mark that merge request superseeded then?

[John Lenton (chipaca)]

That one? no. But this one, probably. Doing so now.

[John Lenton (chipaca)]

Oh, I can't do it myself.

Unmerged revisions

=== modified file 'Makefile.am'
--- Makefile.am	2014-01-13 16:30:17 +0000
+++ Makefile.am	2014-06-26 03:15:21 +0000
@@ -1,4 +1,4 @@
-SUBDIRS = accountsservice schemas po
+SUBDIRS = accountsservice schemas po cleanup
 
 EXTRA_DIST = autogen.sh
 

=== added directory 'cleanup'
=== added file 'cleanup/Makefile.am'
--- cleanup/Makefile.am	1970-01-01 00:00:00 +0000
+++ cleanup/Makefile.am	2014-06-26 03:15:21 +0000
@@ -0,0 +1,29 @@
+
+CLICK_HOOK_OUTPUT=$(top_srcdir)/debian/gsettings-ubuntu-schemas.touch-schemas-clean.click-hook 
+
+all: $(CLICK_HOOK_OUTPUT)
+
+$(CLICK_HOOK_OUTPUT): touch-schemas-clean.click-hook.in Makefile
+	$(AM_V_GEN) $(SED) -e 's^\@pkgdatadir\@^$(pkgdatadir)^g' < $< > $@
+
+%.conf: %.conf.in Makefile
+	$(AM_V_GEN) $(SED) -e 's^\@pkgdatadir\@^$(pkgdatadir)^g' < $< > $@
+
+upstart_jobs_in = \
+	touch-schemas-clean.conf.in
+
+upstart_jobs_DATA = $(upstart_jobs_in:.conf.in=.conf)
+upstart_jobsdir = $(datadir)/upstart/sessions
+
+upstart_scripts_SCRIPTS = \
+	clean-schemas.py
+upstart_scriptsdir = $(pkgdatadir)
+
+EXTRA_DIST = \
+	touch-schemas-clean.click-hook.in \
+	$(upstart_jobs_in) \
+	$(upstart_scripts_SCRIPTS)
+
+CLEANFILES = \
+	$(CLICK_HOOK_OUTPUT) \
+	$(upstart_jobs_DATA)

=== added file 'cleanup/clean-schemas.py'
--- cleanup/clean-schemas.py	1970-01-01 00:00:00 +0000
+++ cleanup/clean-schemas.py	2014-06-26 03:15:21 +0000
@@ -0,0 +1,31 @@
+#!/usr/bin/python3
+
+from gi.repository import GLib
+from gi.repository import Gio
+from gi.repository import Click
+
+def tup2variant (tup) :
+	builder = GLib.VariantBuilder.new(GLib.VariantType.new("(ss)"))
+	builder.add_value(GLib.Variant.new_string(tup[0]))
+	builder.add_value(GLib.Variant.new_string(tup[1]))
+	return builder.end()
+
+clickdb = Click.DB.new()
+clickdb.read()
+
+pkgnames = [] 
+for package in clickdb.get_packages(False) :
+	pkgnames.append(package.get_property('package'))
+
+settings = Gio.Settings.new('com.ubuntu.touch.notifications')
+goodapps = GLib.VariantBuilder.new(GLib.VariantType.new("a(ss)"))
+
+for appname in settings.get_value('popup-blacklist').unpack() :
+	if appname[0] in pkgnames :
+		goodapps.add_value(tup2variant(appname))
+	elif (appname[0] == appname[1]) :
+		appinfo = Gio.DesktopAppInfo.new(appname[0] + ".desktop")
+		if not appinfo is None :
+			goodapps.add_value(tup2variant(appname))
+
+settings.set_value('popup-blacklist', goodapps.end())

=== added file 'cleanup/touch-schemas-clean.click-hook.in'
--- cleanup/touch-schemas-clean.click-hook.in	1970-01-01 00:00:00 +0000
+++ cleanup/touch-schemas-clean.click-hook.in	2014-06-26 03:15:21 +0000
@@ -0,0 +1,4 @@
+Pattern: ${home}/.cache/gsettings-ubuntu-schemas/packages/${id}.json
+Exec: @pkgdatadir@/clean-schemas.py
+User-Level: yes
+Hook-Name: apparmor

=== added file 'cleanup/touch-schemas-clean.conf.in'
--- cleanup/touch-schemas-clean.conf.in	1970-01-01 00:00:00 +0000
+++ cleanup/touch-schemas-clean.conf.in	2014-06-26 03:15:21 +0000
@@ -0,0 +1,12 @@
+# When we rotate the logs (approximately daily) we also check to see
+# if we need to update any of the schemas to remove legacy applications
+# that have been removed.
+
+description "Remove any entries that are related to removed applications"
+
+start on :sys:rotate-logs
+
+task
+
+exec @pkgdatadir@/clean-schemas.py
+

=== modified file 'configure.ac'
--- configure.ac	2014-01-13 16:30:17 +0000
+++ configure.ac	2014-06-26 03:15:21 +0000
@@ -27,6 +27,7 @@
 accountsservice/Makefile
 schemas/Makefile
 po/Makefile.in
+cleanup/Makefile
 ])
 
 dnl ---------------------------------------------------------------------------

=== modified file 'debian/control'
--- debian/control	2014-02-20 15:41:04 +0000
+++ debian/control	2014-06-26 03:15:21 +0000
@@ -2,7 +2,8 @@
 Section: gnome
 Priority: optional
 Maintainer: Ubuntu Desktop Team <[e-mail]>
-Build-Depends: debhelper (>= 9),
+Build-Depends: click-dev,
+               debhelper (>= 9),
                dh-autoreconf,
                gnome-common,
                intltool (>= 0.40.0),

=== modified file 'debian/gsettings-ubuntu-schemas.install'
--- debian/gsettings-ubuntu-schemas.install	2014-02-19 12:53:52 +0000
+++ debian/gsettings-ubuntu-schemas.install	2014-06-26 03:15:21 +0000
@@ -1,1 +1,3 @@
 usr/share/glib-2.0
+usr/share/upstart
+usr/share/gsettings-ubuntu-touch-schemas

=== modified file 'debian/rules'
--- debian/rules	2013-07-24 06:24:34 +0000
+++ debian/rules	2014-06-26 03:15:21 +0000
@@ -5,7 +5,10 @@
 #export DH_VERBOSE=1
 
 %:
-	dh $@  --with autoreconf --fail-missing
+	dh $@  --with autoreconf,click --fail-missing
 
 override_dh_autoreconf:
 	dh_autoreconf -- ./autogen.sh
+
+override_dh_click:
+	dh_click --name touch-schemas-clean

=== modified file 'po/POTFILES.in'
--- po/POTFILES.in	2014-02-24 12:01:25 +0000
+++ po/POTFILES.in	2014-06-26 03:15:21 +0000
@@ -1,6 +1,7 @@
 # List of source files containing translatable strings.
 # Please keep this list in alphabetic order.
 schemas/com.ubuntu.touch.network.gschema.xml.in.in
+schemas/com.ubuntu.touch.notifications.gschema.xml.in.in
 schemas/com.ubuntu.touch.sound.gschema.xml.in.in
 schemas/com.ubuntu.touch.system.gschema.xml.in.in
 schemas/com.ubuntu.sound.gschema.xml.in.in

=== modified file 'schemas/Makefile.am'
--- schemas/Makefile.am	2014-02-24 11:36:24 +0000
+++ schemas/Makefile.am	2014-06-26 03:15:21 +0000
@@ -1,5 +1,6 @@
 desktop_gschemas_in_in = \ 
 	com.ubuntu.touch.network.gschema.xml.in.in \
+	com.ubuntu.touch.notifications.gschema.xml.in.in \
 	com.ubuntu.touch.sound.gschema.xml.in.in   \
 	com.ubuntu.touch.system.gschema.xml.in.in  \
 	com.ubuntu.sound.gschema.xml.in.in         \

=== added file 'schemas/com.ubuntu.touch.notifications.gschema.xml.in.in'
--- schemas/com.ubuntu.touch.notifications.gschema.xml.in.in	1970-01-01 00:00:00 +0000
+++ schemas/com.ubuntu.touch.notifications.gschema.xml.in.in	2014-06-26 03:15:21 +0000
@@ -0,0 +1,15 @@
+<schemalist gettext-domain="gsettings-ubuntu-touch-schemas">
+    <schema id="com.ubuntu.touch.notifications" path="/com/ubuntu/touch/notifications/">
+		<key name="popup-blacklist" type="a(ss)">
+			<_summary>User blacklist of application notifications</_summary>
+			<default>[]</default>
+			<_description>
+				A list of applications that the user has blocked notifications from.
+				For applications that are installed via Click packaging the strings are
+				the name of the package and then the name of the application in the
+				package. For dpkg based applications the strings are both the
+				AppID of the application, or the desktop file name.
+			</_description>
+		</key>
+    </schema>
+</schemalist>