Merge lp:~verterok/account-polld/handle-account-created into lp:~ubuntu-push-hackers/account-polld/trunk

[Guillermo Gonzalez]

Send a notification when an account is created, which actions is to open the accounts settings page.

Send a notification when an account is created, which actions is to open the accounts settings page (this will be changed when we can open a specific account page)

To test this on the device you will need an updated push-helper, grab this: http://paste.ubuntu.com/8038589/ and save it as: /usr/lib/ubuntu-push-client/legacy-helpers/ubuntu-system-settings

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:56
http://jenkins.qa.ubuntu.com/job/account-polld-ci/63/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-amd64-ci/63
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/63
        deb: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/63/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-i386-ci/63

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/account-polld-ci/63/rebuild

review: Approve (continuous-integration)

[Sergio Schvezov (sergiusens)]

couple of inline comments added; I will test in a bit as well.

review: Needs Fixing

[lp:~verterok/account-polld/handle-account-created]

57. By Guillermo Gonzalez on 2014-08-13

add missing TRANSLATORS comments

58. By Guillermo Gonzalez on 2014-08-13

updated translations

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:58
http://jenkins.qa.ubuntu.com/job/account-polld-ci/64/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-amd64-ci/64
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/64
        deb: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/64/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-i386-ci/64

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/account-polld-ci/64/rebuild

review: Approve (continuous-integration)

[Sergio Schvezov (sergiusens)]

if I install this, I don't get updates anymore; only an account creation signal

[lp:~verterok/account-polld/handle-account-created]

59. By Guillermo Gonzalez on 2014-08-14

move mainloop setup into main

60. By Guillermo Gonzalez on 2014-08-14

merge with trunk

[Guillermo Gonzalez (verterok)]

inline comments regarding the mainloop change.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:60
http://jenkins.qa.ubuntu.com/job/account-polld-ci/66/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-amd64-ci/66
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/66
        deb: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/66/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-i386-ci/66

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/account-polld-ci/66/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:60
http://jenkins.qa.ubuntu.com/job/account-polld-ci/67/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-amd64-ci/67
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/67
        deb: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/67/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-i386-ci/67

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/account-polld-ci/67/rebuild

review: Approve (continuous-integration)

[Sergio Schvezov (sergiusens)]

[00:28:32] <sergiusens> I do get the message now, what I don't see working is the actual notifications
[00:28:45] <sergiusens> enabling or disabling the toggle does not trigger the poll loop

and two minor inline comments

review: Needs Fixing

[Sergio Schvezov (sergiusens)]

once more comment

review: Needs Fixing

[lp:~verterok/account-polld/handle-account-created]

61. By Guillermo Gonzalez on 2014-08-15

start the mainlopp in init()

62. By Guillermo Gonzalez on 2014-08-15

update notification body

[Guillermo Gonzalez (verterok)]

Thanks for the review.

Fixed & pushed the 3 issues, I'll test it once jenkins finishes with the build.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:62
http://jenkins.qa.ubuntu.com/job/account-polld-ci/68/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-amd64-ci/68
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/68
        deb: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/68/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-i386-ci/68

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/account-polld-ci/68/rebuild

review: Approve (continuous-integration)

[lp:~verterok/account-polld/handle-account-created]

63. By Guillermo Gonzalez on 2014-08-15

remove break from the accountCh case

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:63
http://jenkins.qa.ubuntu.com/job/account-polld-ci/69/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-amd64-ci/69
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/69
        deb: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/69/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-i386-ci/69

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/account-polld-ci/69/rebuild

review: Approve (continuous-integration)

[Sergio Schvezov (sergiusens)]

tested, works

review: Approve

Unmerged revisions

63. By Guillermo Gonzalez on 2014-08-15

remove break from the accountCh case

62. By Guillermo Gonzalez on 2014-08-15

update notification body

61. By Guillermo Gonzalez on 2014-08-15

start the mainlopp in init()

60. By Guillermo Gonzalez on 2014-08-14

merge with trunk

59. By Guillermo Gonzalez on 2014-08-14

move mainloop setup into main

58. By Guillermo Gonzalez on 2014-08-13

updated translations

57. By Guillermo Gonzalez on 2014-08-13

add missing TRANSLATORS comments

56. By Guillermo Gonzalez on 2014-08-13

fix the dbus path

55. By Guillermo Gonzalez on 2014-08-13

- improve the notification message
- update translations

54. By Guillermo Gonzalez on 2014-08-12

use the correct url for the accounts settings page

=== modified file 'accounts/account-watcher.c'
--- accounts/account-watcher.c	2014-07-24 05:46:03 +0000
+++ accounts/account-watcher.c	2014-08-15 19:02:24 +0000
@@ -22,7 +22,7 @@
 
 #include "account-watcher.h"
 
-/* #define DEBUG */
+//#define DEBUG
 #ifdef DEBUG
 #  define trace(...) fprintf(stderr, __VA_ARGS__)
 #else
@@ -37,9 +37,11 @@
     GHashTable *services;
 
     gulong enabled_event_signal_id;
+    gulong created_event_signal_id;
     gulong account_deleted_signal_id;
 
     AccountEnabledCallback callback;
+    AccountCreatedCallback created_callback;
     void *user_data;
 };
 
@@ -231,6 +233,28 @@
     g_object_unref(account);
 }
 
+static void account_watcher_created_event_cb(
+    AgManager *manager, AgAccountId account_id, AccountWatcher *watcher) {
+    trace("created-event for %u\n", account_id);
+    AgAccount *account = ag_manager_get_account(manager, account_id);
+    if (account == NULL) {
+        /* There was a problem looking up the account */
+        return;
+    }
+    /* Since our AgManager is restricted to a particular service type,
+     * pick the first service for the account. */
+    GList *services = ag_account_list_services(account);
+    if (services != NULL) {
+        AgService *service = services->data;
+        const char *service_name = ag_service_get_name(service);
+        watcher->created_callback(watcher,
+                                  account_id,
+                                  service_name);
+    } 
+    ag_service_list_free(services);
+    g_object_unref(account);
+}
+
 static void account_watcher_account_deleted_cb(
     AgManager *manager, AgAccountId account_id, AccountWatcher *watcher) {
     trace("account-deleted for %u\n", account_id);
@@ -246,6 +270,9 @@
     watcher->enabled_event_signal_id = g_signal_connect(
         watcher->manager, "enabled-event",
         G_CALLBACK(account_watcher_enabled_event_cb), watcher);
+    watcher->created_event_signal_id = g_signal_connect(
+        watcher->manager, "account-created",
+        G_CALLBACK(account_watcher_created_event_cb), watcher);
     watcher->account_deleted_signal_id = g_signal_connect(
         watcher->manager, "account-deleted",
         G_CALLBACK(account_watcher_account_deleted_cb), watcher);
@@ -264,6 +291,7 @@
 
 AccountWatcher *account_watcher_new(const char *service_type,
                                     AccountEnabledCallback callback,
+                                    AccountCreatedCallback created_callback,
                                     void *user_data) {
     AccountWatcher *watcher = g_new0(AccountWatcher, 1);
 
@@ -271,6 +299,7 @@
     watcher->services = g_hash_table_new_full(
         g_direct_hash, g_direct_equal, NULL, (GDestroyNotify)account_info_free);
     watcher->callback = callback;
+    watcher->created_callback = created_callback;
     watcher->user_data = user_data;
 
     /* Make sure main setup occurs within the mainloop thread */

=== modified file 'accounts/account-watcher.h'
--- accounts/account-watcher.h	2014-07-24 05:06:56 +0000
+++ accounts/account-watcher.h	2014-08-15 19:02:24 +0000
@@ -32,10 +32,16 @@
                                        const char *token_secret,
                                        void *user_data);
 
+typedef void (*AccountCreatedCallback)(AccountWatcher *watcher,
+                                       unsigned int account_id,
+                                       const char *service_name);
+
 AccountWatcher *account_watcher_new(const char *service_type,
                                     AccountEnabledCallback callback,
+                                    AccountCreatedCallback created_callback,
                                     void *user_data);
 
 void account_watcher_refresh(AccountWatcher *watcher, unsigned int account_id);
 
+
 #endif

=== modified file 'accounts/accounts.c'
--- accounts/accounts.c	2014-07-24 05:06:56 +0000
+++ accounts/accounts.c	2014-08-15 19:02:24 +0000
@@ -19,6 +19,7 @@
     }
 
     AccountWatcher *watcher = account_watcher_new(
-        service_type, (AccountEnabledCallback)authCallback, NULL);
+        service_type, (AccountEnabledCallback)authCallback, 
+        (AccountCreatedCallback)accountCallback, NULL);
     return watcher;
 }

=== modified file 'accounts/accounts.go'
--- accounts/accounts.go	2014-07-25 20:09:20 +0000
+++ accounts/accounts.go	2014-08-15 19:02:24 +0000
@@ -33,8 +33,14 @@
 )
 
 type Watcher struct {
-	C       <-chan AuthData
-	watcher *C.AccountWatcher
+	C         <-chan AuthData
+	AccountCh <-chan AccountData
+	watcher   *C.AccountWatcher
+}
+
+type AccountData struct {
+	AccountId   uint
+	ServiceName string
 }
 
 type AuthData struct {
@@ -50,18 +56,12 @@
 }
 
 var (
-	mainLoopOnce     sync.Once
-	authChannels     = make(map[*C.AccountWatcher]chan<- AuthData)
-	authChannelsLock sync.Mutex
+	authChannels        = make(map[*C.AccountWatcher]chan<- AuthData)
+	authChannelsLock    sync.Mutex
+	accountChannels     = make(map[*C.AccountWatcher]chan<- AccountData)
+	accountChannelsLock sync.Mutex
 )
 
-func startMainLoop() {
-	mainLoopOnce.Do(func() {
-		mainLoop := C.g_main_loop_new(nil, C.gboolean(1))
-		go C.g_main_loop_run(mainLoop)
-	})
-}
-
 // NewWatcher creates a new account watcher for the given service names
 func NewWatcher(serviceType string) *Watcher {
 	w := new(Watcher)
@@ -74,8 +74,12 @@
 	authChannelsLock.Lock()
 	authChannels[w.watcher] = ch
 	authChannelsLock.Unlock()
-
-	startMainLoop()
+	// account stuff
+	accountCh := make(chan AccountData)
+	w.AccountCh = accountCh
+	accountChannelsLock.Lock()
+	accountChannels[w.watcher] = accountCh
+	accountChannelsLock.Unlock()
 
 	return w
 }
@@ -121,3 +125,21 @@
 	}
 	ch <- data
 }
+
+//export accountCallback
+func accountCallback(watcher unsafe.Pointer, accountId C.uint, serviceName *C.char) {
+	// Ideally the first argument would be of type
+	// *C.AccountWatcher, but that fails with Go 1.2.
+	accountChannelsLock.Lock()
+	ch := accountChannels[(*C.AccountWatcher)(watcher)]
+	accountChannelsLock.Unlock()
+	if ch == nil {
+		// Log the error
+		return
+	}
+
+	var data AccountData
+	data.AccountId = uint(accountId)
+	data.ServiceName = C.GoString(serviceName)
+	ch <- data
+}

=== modified file 'cmd/account-polld/main.go'
--- cmd/account-polld/main.go	2014-08-08 20:11:14 +0000
+++ cmd/account-polld/main.go	2014-08-15 19:02:24 +0000
@@ -17,10 +17,17 @@
 
 package main
 
+/*
+#cgo pkg-config: glib-2.0
+#include <glib.h>
+*/
+import "C"
 import (
 	"encoding/json"
 	"fmt"
 	"strings"
+	"sync"
+	"time"
 
 	"log"
 
@@ -52,7 +59,11 @@
 	POSTAL_OBJECT_PATH_PART = "/com/ubuntu/Postal/"
 )
 
+var mainLoopOnce sync.Once
+var mainLoop *C.GMainLoop
+
 func init() {
+	startGlibMainLoop()
 }
 
 func main() {
@@ -76,40 +87,61 @@
 	<-done
 }
 
+func startGlibMainLoop() {
+	mainLoopOnce.Do(func() {
+		mainLoop = C.g_main_loop_new(nil, C.gboolean(1))
+		go C.g_main_loop_run(mainLoop)
+	})
+}
+
 func monitorAccounts(postWatch chan *PostWatch) {
 	watcher := accounts.NewWatcher(SERVICETYPE_POLL)
 	mgr := make(map[uint]*AccountManager)
 L:
-	for data := range watcher.C {
-		if account, ok := mgr[data.AccountId]; ok {
-			if data.Enabled {
-				log.Println("New account data for existing account with id", data.AccountId)
-				account.updateAuthData(data)
-			} else {
-				account.Delete()
-				delete(mgr, data.AccountId)
-			}
-		} else if data.Enabled {
-			var plugin plugins.Plugin
-			switch data.ServiceName {
-			case SERVICENAME_GMAIL:
-				log.Println("Creating account with id", data.AccountId, "for", data.ServiceName)
-				plugin = gmail.New(data.AccountId)
-			case SERVICENAME_FACEBOOK:
-				// This is just stubbed until the plugin exists.
-				log.Println("Creating account with id", data.AccountId, "for", data.ServiceName)
-				plugin = facebook.New(data.AccountId)
-			case SERVICENAME_TWITTER:
-				// This is just stubbed until the plugin exists.
-				log.Println("Creating account with id", data.AccountId, "for", data.ServiceName)
-				plugin = twitter.New()
-			default:
-				log.Println("Unhandled account with id", data.AccountId, "for", data.ServiceName)
-				continue L
-			}
-			mgr[data.AccountId] = NewAccountManager(watcher, postWatch, plugin)
-			mgr[data.AccountId].updateAuthData(data)
-			go mgr[data.AccountId].Loop()
+	for {
+		select {
+		case data := <-watcher.C:
+			if account, ok := mgr[data.AccountId]; ok {
+				if data.Enabled {
+					log.Println("New account data for existing account with id", data.AccountId)
+					account.updateAuthData(data)
+				} else {
+					account.Delete()
+					delete(mgr, data.AccountId)
+				}
+			} else if data.Enabled {
+				var plugin plugins.Plugin
+				switch data.ServiceName {
+				case SERVICENAME_GMAIL:
+					log.Println("Creating account with id", data.AccountId, "for", data.ServiceName)
+					plugin = gmail.New(data.AccountId)
+				case SERVICENAME_FACEBOOK:
+					// This is just stubbed until the plugin exists.
+					log.Println("Creating account with id", data.AccountId, "for", data.ServiceName)
+					plugin = facebook.New(data.AccountId)
+				case SERVICENAME_TWITTER:
+					// This is just stubbed until the plugin exists.
+					log.Println("Creating account with id", data.AccountId, "for", data.ServiceName)
+					plugin = twitter.New()
+				default:
+					log.Println("Unhandled account with id", data.AccountId, "for", data.ServiceName)
+					break L
+				}
+				mgr[data.AccountId] = NewAccountManager(watcher, postWatch, plugin)
+				mgr[data.AccountId].updateAuthData(data)
+				go mgr[data.AccountId].Loop()
+			}
+		case data := <-watcher.AccountCh:
+			log.Println("New Account:", data.AccountId, "for", data.ServiceName)
+			// TRANSLATORS: This is the summary of the notification triggered after creating a new account
+			summary := gettext.Gettext("New account created")
+			// TRANSLATORS: This is the body of the notification triggered after creating a new account
+			body := gettext.Gettext("To enable notifications for this account, toggle Notifications from the account setting")
+			epoch := time.Now().Unix()
+			action := "settings://personal/online-accounts"
+			pushMsg := *plugins.NewStandardPushMessage(summary, body, action, "", epoch)
+			msgs := []plugins.PushMessage{pushMsg}
+			postWatch <- &PostWatch{messages: msgs, appId: "_ubuntu-system-settings"}
 		}
 	}
 }
@@ -140,18 +172,21 @@
 // e.g.; if the APP_ID is com.ubuntu.music", the returned object path
 // would be "/com/ubuntu/PushNotifications/com_2eubuntu_2eubuntu_2emusic
 func pushObjectPath(id plugins.ApplicationId) dbus.ObjectPath {
+	pkg := POSTAL_OBJECT_PATH_PART
 	idParts := strings.Split(string(id), "_")
 	if len(idParts) < 2 {
 		panic(fmt.Sprintf("APP_ID '%s' is not valid", id))
-	}
-
-	pkg := POSTAL_OBJECT_PATH_PART
-	for _, c := range idParts[0] {
-		switch c {
-		case '+', '.', '-', ':', '~', '_':
-			pkg += fmt.Sprintf("_%x", string(c))
-		default:
-			pkg += string(c)
+	} else if strings.HasPrefix(string(id), "_") {
+		// it's a legacy app!
+		pkg += "_"
+	} else {
+		for _, c := range idParts[0] {
+			switch c {
+			case '+', '.', '-', ':', '~', '_':
+				pkg += fmt.Sprintf("_%x", string(c))
+			default:
+				pkg += string(c)
+			}
 		}
 	}
 	return dbus.ObjectPath(pkg)

=== modified file 'po/account-polld.pot'
--- po/account-polld.pot	2014-08-11 18:35:28 +0000
+++ po/account-polld.pot	2014-08-15 19:02:24 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: account-polld\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2014-08-11 15:35-0300\n"
+"POT-Creation-Date: 2014-08-15 11:06-0300\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[e-mail]>\n"
@@ -17,6 +17,31 @@
 "Content-Type: text/plain; charset=CHARSET\n"
 "Content-Transfer-Encoding: 8bit\n"
 
+#. TRANSLATORS: This is the summary of the notification triggered after creating a new account
+#: cmd/account-polld/main.go:137
+msgid "New account created"
+msgstr ""
+
+#. TRANSLATORS: This is the body of the notification triggered after creating a new account
+#: cmd/account-polld/main.go:139
+msgid ""
+"To enable notifications for this account, toggle Notifications from the "
+"account setting"
+msgstr ""
+
+#. TRANSLATORS: This represents a notification summary about more facebook notifications
+#: plugins/facebook/facebook.go:170
+msgid "Multiple more notifications"
+msgstr ""
+
+#. TRANSLATORS: This represents a notification body with the comma separated facebook usernames
+#. TRANSLATORS: This represents a notification body with the comma separated twitter usernames
+#: plugins/facebook/facebook.go:172 plugins/twitter/twitter.go:130
+#: plugins/twitter/twitter.go:187
+#, c-format
+msgid "From %s"
+msgstr ""
+
 #. TRANSLATORS: The first %s refers to the twitter user's Name, the second %s to the username.
 #: plugins/twitter/twitter.go:113 plugins/twitter/twitter.go:170
 #, c-format
@@ -28,14 +53,6 @@
 msgid "Multiple more mentions"
 msgstr ""
 
-#. TRANSLATORS: This represents a notification body with the comma separated twitter usernames
-#. TRANSLATORS: This represents a notification body with the comma separated facebook usernames
-#: plugins/twitter/twitter.go:130 plugins/twitter/twitter.go:187
-#: plugins/facebook/facebook.go:172
-#, c-format
-msgid "From %s"
-msgstr ""
-
 #. TRANSLATORS: This represents a notification summary about more twitter direct messages available
 #: plugins/twitter/twitter.go:185
 msgid "Multiple direct messages available"
@@ -72,11 +89,6 @@
 msgid "You have an approximate of %d additional unread messages"
 msgstr ""
 
-#. TRANSLATORS: This represents a notification summary about more facebook notifications
-#: plugins/facebook/facebook.go:170
-msgid "Multiple more notifications"
-msgstr ""
-
 #: data/account-polld.desktop.tr.h:1
 msgid "Notifications"
 msgstr ""