After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 652543 - Add a MC GOA plugin
Add a MC GOA plugin
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Accounts
2.33.x
Other Linux
: Normal enhancement
: 3.2
Assigned To: empathy-maint
Depends on: 652574
Blocks:
 
 
Reported: 2011-06-14 11:29 UTC by Guillaume Desmottes
Modified: 2011-10-20 07:33 UTC
See Also:
GNOME target: 3.2
GNOME version: ---



Description Guillaume Desmottes 2011-06-14 11:29:37 UTC
We should implement a MC account plugin talking to the GNOME Online Account service to automatically create TP accounts when Facebook/GTalk/... accounts are created.

It could be based on this similar plugin for libsocialweb: http://cgit.collabora.com/git/meego-facebook-plugins.git/tree/mission-control
Comment 1 Danielle Madeley 2011-06-14 16:21:49 UTC
Work in Progress: http://cgit.collabora.com/git/user/danni/empathy.git/log/?h=goa-mc-plugin
Comment 2 Guillaume Desmottes 2011-06-20 14:55:37 UTC
+  GHashTable *accounts; /* string -> GoaAccount */
Please document the semantic of the string.


GTalk account should have fallback-servers and extra-certificate-identities
set, see empathy_protocol_chooser_create_account_settings().
We should probably set require-encryption as well.
Comment 3 Bastien Nocera 2011-06-23 19:20:55 UTC
See also bug 653269 about exporting more information for the Chat accounts. I'm thinking that there shouldn't be any special-casing in the plugin, and the "Chat" account info would be exported.
Comment 4 Guillaume Desmottes 2011-06-28 09:27:11 UTC
(In reply to comment #2)
> GTalk account should have fallback-servers and extra-certificate-identities
> set, see empathy_protocol_chooser_create_account_settings().
> We should probably set require-encryption as well.

We just added more parameters to GTalk accounts:
http://git.gnome.org/browse/empathy/commit/?id=53a110fcf6a8b697f8d27a8c34b0cccf4db04b24
http://git.gnome.org/browse/empathy/commit/?id=60d6f85f560b4c958085c8f759f519363a05e779
Comment 5 Danielle Madeley 2011-06-29 07:13:51 UTC
(In reply to comment #3)
> See also bug 653269 about exporting more information for the Chat accounts. I'm
> thinking that there shouldn't be any special-casing in the plugin, and the
> "Chat" account info would be exported.

There's a whole bunch of IM/Telepathy specific stuff that Empathy already knows about, we can move it into GOA, but for now it's not important and has been hardcoded using Account.ProviderType.

For reference, the parameters I've included are:

account - e.g. danielle@nyan.cat -- comes from GOA, currently using Account.Identity [1]
server - e.g. talk.google.com -- hardcoded for some services
fallback-servers -- hardcoded for some services
extra-certificate-identities -- hardcoded for some services
require-encryption -- hardcoded for some services

[1] I suppose this may not be correct for all services? It's correct for Google though.
Comment 6 Danielle Madeley 2011-06-29 07:25:13 UTC
(In reply to comment #4)
> > GTalk account should have fallback-servers and extra-certificate-identities
> > set, see empathy_protocol_chooser_create_account_settings().
> > We should probably set require-encryption as well.
> 
> We just added more parameters to GTalk accounts:

Updated for all of these points.

I'm trying to work out how to determine if the 'Chat' switch is turned on. The account doesn't seem to expose o.g.OA.Chat.
Comment 7 Guillaume Desmottes 2011-08-31 13:09:37 UTC
I ressurected this branch and hooked the 'Chat' switch: http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=goa-mc-plugin-652543

How should we store things in mcp_account_manager_goa_set()? MC asks us to store HasBeenOnline, Nickname and NormalizedName, should we request properties to GOA guys for this?
We should probably be able to store AvatarMime and avatar_token as well.
Comment 8 Danielle Madeley 2011-09-01 00:45:34 UTC
This looks pretty good, except I was hoping for d71691f82c6c64af2a003743177722de9768a841, that rather than not being able to set Enabled, we should call whatever API lets you toggle the Chat switch (I assume the configuration panel must do something).

> How should we store things in mcp_account_manager_goa_set()? MC asks us to
> store HasBeenOnline, Nickname and NormalizedName, should we request properties
> to GOA guys for this?

Returning FALSE (iirc) then tries to call set() on the next storage plugin (i.e. the config file). I think we may as well store these properties in accounts.cfg, since they're meaningless to anything that isn't Telepathy.
Comment 9 Guillaume Desmottes 2011-09-01 08:51:54 UTC
I opened bug #653269 about the storage of extra paramters. Ideally we should fix this before merging the plugin.
Comment 10 Guillaume Desmottes 2011-09-01 08:58:59 UTC
(In reply to comment #8)
> This looks pretty good, except I was hoping for
> d71691f82c6c64af2a003743177722de9768a841, that rather than not being able to
> set Enabled, we should call whatever API lets you toggle the Chat switch (I
> assume the configuration panel must do something).

I opened bug #657905 for this, but I don't think that's a blocker for getting
this plugin merged.
Comment 11 Guillaume Desmottes 2011-09-01 09:05:47 UTC
(In reply to comment #9)
> I opened bug #653269 about the storage of extra paramters. Ideally we should
> fix this before merging the plugin.

I meant 'commented'; the bug was already there.
Comment 12 Guillaume Desmottes 2011-09-01 12:34:17 UTC
(In reply to comment #7)
> I ressurected this branch and hooked the 'Chat' switch:
> http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=goa-mc-plugin-652543

I pushed an extra commit cleaning the code a bit.
Comment 13 Guillaume Desmottes 2011-09-02 12:33:59 UTC
I just added support for storing the extra parameters to a key file as that seems to way to go (see bug #653269).
Comment 14 Danielle Madeley 2011-09-05 01:59:20 UTC
+      if (keys == NULL)
+        n = 0;

Is this required? Random aside, I prefer nkeys to n. Tells you what the variables contains.

+      keys = g_key_file_get_keys (priv->store, acct, &n, NULL);

I can't see where this is free'd.
Comment 15 Guillaume Desmottes 2011-09-05 14:02:25 UTC
(In reply to comment #14)
> +      if (keys == NULL)
> +        n = 0;
> 
> Is this required? Random aside, I prefer nkeys to n. Tells you what the
> variables contains.

Yes, if there is an error. I renamed the variable.

> +      keys = g_key_file_get_keys (priv->store, acct, &n, NULL);
> 
> I can't see where this is free'd.

Fixed.
Comment 16 Danielle Madeley 2011-09-05 23:27:08 UTC
(In reply to comment #15)

> Yes, if there is an error. I renamed the variable.

I would have thought GLib would 0 this value on error. If not that's a bug we should file in GLib.

Looks good to me. Ship it!
Comment 17 Guillaume Desmottes 2011-09-06 05:38:07 UTC
Merged to master; thanks !

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.
Comment 18 Guillaume Desmottes 2011-09-06 05:44:18 UTC
(In reply to comment #16)
> (In reply to comment #15)
> 
> > Yes, if there is an error. I renamed the variable.
> 
> I would have thought GLib would 0 this value on error. If not that's a bug we
> should file in GLib.

Bug #658315