Bug 28037

Summary: TpContact should have helper API to set alias
Product: Telepathy Reporter: Xavier Claessens <xclaesse>
Component: tp-glibAssignee: Telepathy bugs list <telepathy-bugs>
Status: RESOLVED MOVED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: enhancement    
Priority: medium CC: guillaume.desmottes
Version: unspecified   
Hardware: Other   
OS: All   
URL: http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?id=refs/heads/set-alias
Whiteboard:
i915 platform: i915 features:

Description Xavier Claessens 2010-05-09 03:30:52 UTC
In most cases, setting the alias is done only for 1 contact. Lowlevel API is a pain to use because it needs creating hashtable, etc...

I suggest adding simple API on TpContact:

void tp_contact_set_alias_async (TpContact *self, const gchar *new_alias,
    GAsyncReadyCallback callback, gpointer user_data);

gboolean tp_contact_set_alias_finish (TpContact *self,
    GAsyncResult *result, GError **error);
Comment 1 Xavier Claessens 2010-05-09 04:18:11 UTC
That's a one patch branch: 

http://git.collabora.co.uk/?p=user/xclaesse/telepathy-glib.git;a=shortlog;h=refs/heads/set-alias

Review needed :)
Comment 2 Simon McVittie 2010-05-10 02:19:35 UTC
The concept seems reasonable, but I'm a little worried that this is going to either need deprecating, or have a name that's not consistent with the rest of the API, when we implement the ContactList and Names D-Bus API shortly.

The current plan is to solve the "is it a nickname or user-assigned?" problem, Bug #14540, by introducing a Connection.Interface.Names with separate contact attributes for remote-contact-assigned nickname, local-user-assigned "local alias"/"pet name", and abbreviated identifier (e.g. just the part of a JID before the @).

See Bug #21787, although I might split the implementation of Names out of that bug into Bug #14540. Your comments and/or help on either or both interfaces would be appreciated.

I think the semantics of this method should be "store a name for the contact", defined in a way that currently means SetAliases, but can become SetLocalAlias (or whatever) when we introduce it. That makes the naming tricky, though, because we're still not sure what to call the alias assigned by the local user ("local alias" is the current proposal), so can we defer this a little until Names has settled down enough that we know what to call the method?

(I plan to work on ContactList and Names while on trains later today.)

Looking at the implementation:

> +    TpConnectionAliasFlags alias_flags;

You don't seem to use this? Remove it; we can add retrieval of the Names information as part of the development of that interface.

> + * @self's #TpConnection must be ready (#TpConnection:connection-ready
> + *  must be %TRUE) and not invalidated.

You don't need to return_if_fail that the connection is ready: if the connection was able to create a contact, then it must have been CONNECTED at some stage (only CONNECTED connections can have valid handles, and TpContact always has a valid handle).

If you did need to make this assertion, new code should be documented in terms of the features (in this case it'd be TP_CONNECTION_FEATURE_CONNECTED, which is in fact equivalent to connection-ready).

An invalidated connection shouldn't be a return_if_fail: the whole point of the invalidation mechanism is to fail gracefully on dead objects, without criticalling and hence requiring a boilerplate check in advance like dbus-glib does. Just add a regression test that tries it on a dead connection, and makes sure the async op fails (making a tp_cli D-Bus call on an invalidated connection fails with the invalidation error, so the minimal implementation is to do nothing special).

Talking of which, where's the regression test? :-)
Comment 3 Xavier Claessens 2010-05-10 05:16:10 UTC
Note that TpAccount has tp_account_set_nickname_async(), I actually picked the API from it.

I'll come back to this when we have a concensus of what's the best API naming.
Comment 4 Simon McVittie 2010-07-13 05:36:32 UTC
(In reply to comment #3)
> Note that TpAccount has tp_account_set_nickname_async(), I actually picked the
> API from it.

That's to set our own nickname. There are two orthogonal concepts:

* nickname: X's nickname is chosen by X, so by definition, we can only set the SelfHandle's nickname

* local-alias: X's local alias is chosen by the SelfHandle: if supported by the protocol, we can set anyone's local alias

Aliasing mixes the two; Names won't (see Bug #14540).

An API for local alias on TpContact would have undefined results if used on the self-handle's corresponding contact. Under Aliasing, it sets the self-handle's local alias *and* nickname (implementation detail: in Gabble, it only sets the local alias if the self-handle is already on the roster).

Under Names, the naive version would be to put the self-handle on the roster and set its local alias; one possible refinement would be to ignore this for the self-handle, or ignore this for the self-handle if it isn't already on the roster.

The Account API anticipates Names, by calling the local user's name for themselves "Nickname" rather than "Alias". At the moment MC uses Aliasing to set the self-handle's nickname, perhaps with the side-effect of changing the local alias; when Names happens, MC should be changed to try to set the nickname via that first, falling back to Aliasing.
Comment 5 Guillaume Desmottes 2011-11-25 01:13:13 UTC
*** Bug 42559 has been marked as a duplicate of this bug. ***
Comment 6 Guillaume Desmottes 2011-11-25 01:16:56 UTC
I need API to get the alias flag (see https://bugzilla.gnome.org/show_bug.cgi?id=663328 ) so I'm going to add just that. We shouldn't wait for Names, we'll just add new API later.
Comment 7 Guillaume Desmottes 2011-11-25 02:16:12 UTC
http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=alias-28037

It shouldn't conflict much with Xavier's branch as I just added support to check the alias flag.
Comment 8 Xavier Claessens 2011-11-28 01:25:54 UTC
You can use tp_tests_proxy_run_until_prepared(). The rest looks good.
Comment 9 Guillaume Desmottes 2011-11-28 01:33:56 UTC
My branch has been merged to master. I keep the bug open because we still don't have API to set aliases.
Comment 10 GitLab Migration User 2019-12-03 20:02:53 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/telepathy/telepathy-glib/issues/31.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.