Merge lp:~azzar1/update-notifier/livepatch-notification-button into lp:update-notifier

Proposed by Andrea Azzarone on 2019-01-30
Status: Merged
Approved by: Sebastien Bacher on 2019-02-18
Approved revision: 951
Merged at revision: 953
Proposed branch: lp:~azzar1/update-notifier/livepatch-notification-button
Merge into: lp:update-notifier
Diff against target: 146 lines (+64/-8)
2 files modified
debian/changelog (+6/-0)
src/livepatch.c (+58/-8)
To merge this branch: bzr merge lp:~azzar1/update-notifier/livepatch-notification-button
Reviewer Review Type Date Requested Status
Sebastien Bacher 2019-01-30 Approve on 2019-02-18
Review via email: mp+362478@code.launchpad.net

Commit message

src/livepatch.c: Add a "Settings..." button to the notification.

To post a comment you must log in.
951. By Andrea Azzarone on 2019-01-30

src/livepatch.c: Add a "Settings..." button to the notification.

Sebastien Bacher (seb128) wrote :

Thank you for your work, that looks mostly fine, I'm unsure why you add the action twice though?

+ if (info) {
+ notify_notification_add_action (n, "settings", _("Show Settings…"),
+ notify_action_cb, NULL, NULL);
+ notify_notification_add_action (n, "default", _("Show Settings…"),
+ notify_action_cb, NULL, NULL);

Wouldn't one be enough?

review: Needs Information
Andrea Azzarone (azzar1) wrote :

> Thank you for your work, that looks mostly fine, I'm unsure why you add the
> action twice though?
>
> + if (info) {
> + notify_notification_add_action (n, "settings", _("Show Settings…"),
> + notify_action_cb, NULL, NULL);
> + notify_notification_add_action (n, "default", _("Show Settings…"),
> + notify_action_cb, NULL, NULL);
>
> Wouldn't one be enough?

One is to add a button, one is to open software properties if you click on the notification.

Sebastien Bacher (seb128) wrote :

Thanks

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2019-01-07 16:32:46 +0000
3+++ debian/changelog 2019-01-30 17:24:03 +0000
4@@ -1,3 +1,9 @@
5+update-notifier (3.192.12) UNRELEASED; urgency=medium
6+
7+ * src/livepatch.c: Add a "Settings..." button to the notification.
8+
9+ -- Andrea Azzarone <andrea.azzarone@canonical.com> Wed, 30 Jan 2019 17:20:42 +0000
10+
11 update-notifier (3.192.11) disco; urgency=medium
12
13 * src/update-notifier.c: Don't use G_SPAWN_DO_NOT_REAP_CHILD in order
14
15=== modified file 'src/livepatch.c'
16--- src/livepatch.c 2018-11-12 09:52:00 +0000
17+++ src/livepatch.c 2019-01-30 17:24:03 +0000
18@@ -3,6 +3,7 @@
19 #endif
20
21 #include <errno.h>
22+#include <gio/gdesktopappinfo.h>
23 #include <glib.h>
24 #include <glib/gstdio.h>
25 #include <libnotify/notify.h>
26@@ -11,6 +12,7 @@
27
28 #include "update-notifier.h"
29
30+#define LIVEPATCH_DESKTOP_FILE "software-properties-livepatch.desktop"
31 #define STATUS_PATH "/var/snap/canonical-livepatch/current/status"
32
33 static void
34@@ -29,12 +31,55 @@
35 }
36
37 static void
38+notify_action_cb (NotifyNotification *notification,
39+ char *action,
40+ gpointer user_data)
41+{
42+ g_autoptr(GDesktopAppInfo) info = NULL;
43+ g_autoptr(GdkAppLaunchContext) context = NULL;
44+ g_autoptr(GError) error = NULL;
45+
46+ info = g_desktop_app_info_new (LIVEPATCH_DESKTOP_FILE);
47+ if (!info) {
48+ g_warning ("Could not find application '%s'", LIVEPATCH_DESKTOP_FILE);
49+ return;
50+ }
51+
52+ context = gdk_display_get_app_launch_context (gdk_display_get_default ());
53+ if (!g_app_info_launch (G_APP_INFO (info), NULL, G_APP_LAUNCH_CONTEXT (context), &error)) {
54+ g_warning ("Could not launch application '%s'", LIVEPATCH_DESKTOP_FILE);
55+ }
56+}
57+
58+static gboolean
59 show_notification (const char *summary, const char *body, const char *icon)
60 {
61- NotifyNotification *n = notify_notification_new (summary, body, icon);
62+ NotifyNotification *n;
63+ g_autoptr(GDesktopAppInfo) info = NULL;
64+ g_autoptr(GError) error = NULL;
65+
66+ n = notify_notification_new (summary, body, icon);
67 notify_notification_set_timeout (n, 60000);
68- notify_notification_show (n, NULL);
69- g_object_unref (n);
70+
71+ info = g_desktop_app_info_new (LIVEPATCH_DESKTOP_FILE);
72+ if (info) {
73+ notify_notification_add_action (n, "settings", _("Show Settings…"),
74+ notify_action_cb, NULL, NULL);
75+ notify_notification_add_action (n, "default", _("Show Settings…"),
76+ notify_action_cb, NULL, NULL);
77+ } else {
78+ g_warning ("Could not find application '%s'. The notification will not "
79+ "have a 'Show Settings…' button.", LIVEPATCH_DESKTOP_FILE);
80+ }
81+
82+ g_signal_connect (n, "closed", G_CALLBACK (gtk_main_quit), NULL);
83+
84+ if (!notify_notification_show (n, &error)) {
85+ g_warning ("Could not show notification: '%s", error->message);
86+ return FALSE;
87+ }
88+
89+ return TRUE;
90 }
91
92 static void
93@@ -94,17 +139,17 @@
94 return difftime (status_stat.st_mtim.tv_sec, boot_timestamp) >= 0;
95 }
96
97-static void
98+static gboolean
99 show_status_notification ()
100 {
101 g_autofree gchar *event = NULL;
102 g_autofree gchar *description = NULL;
103
104 if (!g_file_test (STATUS_PATH, G_FILE_TEST_EXISTS))
105- return;
106+ return FALSE;
107
108 if (!file_modified_after_boot (STATUS_PATH))
109- return;
110+ return FALSE;
111
112 get_event_from_file (STATUS_PATH, &event, &description);
113
114@@ -120,6 +165,7 @@
115
116 if (is_overflow || conversion_failed) {
117 g_warning ("Failed to parse the status file");
118+ return FALSE;
119 } else if (num_updates != 0) {
120 body = g_strdup_printf (
121 ngettext ("%" G_GUINT64_FORMAT " Livepatch update has been successfully applied.",
122@@ -127,18 +173,22 @@
123 num_updates),
124 num_updates);
125
126- show_notification (_("Canonical Livepatch"), body, NULL);
127+ return show_notification (_("Canonical Livepatch"), body, NULL);
128 }
129 }
130+
131+ return FALSE;
132 }
133
134 int
135 main (int argc, char **argv)
136 {
137+ gtk_init (&argc, &argv);
138 init_notification ();
139 init_gettext ();
140
141- show_status_notification ();
142+ if (show_status_notification ())
143+ gtk_main ();
144
145 return EXIT_SUCCESS;
146 }

Subscribers

People subscribed via source and target branches

to all changes: