[Bug 28199] port Gabble to TpBaseContactList

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=28199

Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|http://git.collabora.co.uk/ |http://git.collabora.co.uk/
                   |?p=user/smcv/telepathy-gabb |?p=user/smcv/telepathy-gabb
                   |le-smcv.git;a=shortlog;h=re |le-smcv.git;a=shortlog;h=re
                   |fs/heads/contact-list-fixes |fs/heads/contact-list-revie
                   |                            |wed
           Keywords|patch                       |

--- Comment #12 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-09-21 03:55:35 PDT ---
(In reply to comment #11)
> REMOVE is a verb. Would something like "NOT_YET_STORED" be more descriptive, or
> am I missing some subtlety here?

It's REMOVE because it corresponds to <item subscription="remove">. It is
indeed a verb (state transition) in XMPP too, but appears in an attribute that
is normally an adjective; the "default subscription" is of course "not on the
roster", so you only see "remove" in XMPP for the state transition from present
to absent.

I could change it to REMOVED, but I think it might be clearer to stick to the
XMPP phrasing.

> Also, I looked and saw that GabbleRosterSubscription is a set of flags, but
> you're testing it as if it was a normal enum. Is there a case where
> subscription == GABBLE_ROSTER_SUBSCRIPTION_REMOVE |
> GABBLE_ROSTER_SUBSCRIPTION_INVALID or something, and do we behave correctly in
> this case?

As far as I can see, the only states that can ever exist in a GabbleRosterItem
are NONE (= 0), FROM, TO, FROM|TO (= BOTH) and REMOVE; additionally, the
new_subscription in a GabbleRosterItemEdit can be INVALID. The only bitfielding
that goes on should be to combine FROM and TO.

_subscription_to_string() and process_roster() assert this,
_parse_item_subscription() can only return these, and I've just checked that no
other states are assigned.

> -  if (item->subscription != GABBLE_ROSTER_SUBSCRIPTION_NONE)
> +  if (item->subscription != GABBLE_ROSTER_SUBSCRIPTION_NONE &&
> +      item->subscription != GABBLE_ROSTER_SUBSCRIPTION_REMOVE)

I'm reasonably sure this is right: "no subscription and present on the contact
list" and "no subscription and absent from the contact list" were previously
conflated as NONE, so we now want to treat REMOVE much like NONE.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.