[sugar] [PATCH] Refactor invites for 1-1 Chat (#6298)

Morgan Collett morgan.collett
Wed Jun 4 11:08:50 EDT 2008

On Mon, May 26, 2008 at 5:09 PM, Tomeu Vizoso <tomeu at tomeuvizoso.net> wrote:
> On Mon, May 26, 2008 at 4:01 PM, Morgan Collett
> <morgan.collett at gmail.com> wrote:
>> On Mon, May 26, 2008 at 12:10 PM, Tomeu Vizoso <tomeu at tomeuvizoso.net> wrote:
>>> +        if channel_type == CHANNEL_TYPE_TEXT:
>>> +            bundle_id = 'org.laptop.Chat'
>>> +        else:
>>> +            bundle_id = 'org.laptop.VideoChat'
>>> What are the plans for the future? Could we drop these hardcoded values somehow?
>> I'm open to suggestions... at the moment, Chat is the only sugarized
>> activity supporting XMPP but it's possible the overlay chat can handle
>> this type of connection in the future. We would still need a way to
>> launch a VideoChat (or equivalent) activity.
>> Perhaps at the time when we have multiple activities that could be
>> launched, we could add a Control Panel option?
> Rather than that, what about activities stating which kind of
> invitations can accept? Then the user could choose one of several
> activities from the invitation palette.
> But this looks like too much complexity for the August release.
>>> +    def start_activity_with_uri(self, activity_type, uri):
>>> What's the format of private_connection and how is it an URI? I was
>>> expecting activityfactory.create_with_uri() to be dropped at some
>>> point. Why cannot be used an activity_id like in normal invites?
>> This is something I would especially like input on. With an XMPP
>> connection from a non Sugar XMPP client, PS sends the
>> private-invitation signal but there is no shared activity. The whole
>> point of supporting this is that shared activities have a room (MUC)
>> on the server (in the case of gabble, but salut has an equivalent)
>> whereas now this will support 1-1 XMPP chat with no room. So there's
>> no activity_id.
> Hmm, I don't quite see why the presence of a MUC should affect having
> an activity_id or a private connection. Seems to me like an
> implementation detail surfacing too high on the API. There's no way a
> private connection could be exposed as a privately-shared activity?
>> Therefore, I needed a way to pass the Telepathy channel to the
>> instance of Chat. It's not a URI at all, just an arbitrary string
>> (actually multiple values encoded with json). The values we need to
>> get to Chat are, for example:
>> bus_name: "org.freedesktop.Telepathy.Connection.gabble.jabber.a088b096fcea6d858fc0cfbfe8879f67817b6aca_40your_2ehost_2ename_2fTelepathy"
>> connection: "/org/freedesktop/Telepathy/Connection/gabble/jabber/a088b096fcea6d858fc0cfbfe8879f67817b6aca_40your_2ehost_2ename_2fTelepathy"
>> Channel: "/org/freedesktop/Telepathy/Connection/gabble/jabber/a088b096fcea6d858fc0cfbfe8879f67817b6aca_40your_2ehost_2ename_2fTelepathy/ImChannel4"
>> So, should I find some other piece of metadata to use, and create an
>> equivalent of start_activity_with_uri that passes the parameter(s)
>> there? Any other ways of passing these values to a newly-instantiated
>> activity?
> No other way that I know.
>>> +        if invite.get_activity_id():
>>> +            # shared activity
>>> +            mesh = shellmodel.get_instance().get_mesh()
>>> +            activity_model = mesh.get_activity(invite.get_activity_id())
>>> +            self._activity_model = activity_model
>>> +            self._bundle_id = activity_model.get_bundle_id()
>>> +        else:
>>> +            # private invite to 1-1 connection
>>> +            self._private_connection = invite.get_private_connection()
>>> +            self._bundle_id = invite.get_bundle_id()
>>> Suggestion: what about having ActivityInvite and PrivateInvite both
>>> inheriting from BaseInvite?
>> I had it like that at one point, but decided to combine them for
>> simplicity since ActivityInvite had almost nothing in common with
>> PrivateInvite. The parameters we need to pass are completely
>> different... Perhaps the code is less readable now though, I'll take
>> another look.
> Yes, if they have nothing in common, being two classes look cleaner ;)

I have split it out into ActivityInvite and PrivateInvite as you recommended.

I've updated the patches and pushed to the repos below. I think I have
dealt with all the review comments so far, so please take another
look. I have tested it and fixed the problem where the invites weren't
removed. In testing on my branch it is working well for me.

The main code to review is at:
http://dev.laptop.org/git?p=users/morgan/sugar;a=shortlog;h=6298 (3
most recent patches).

Related changes are at:
- Guillaume's change, r+ from me
- Guillaume's change, r+ from me
- r+ from me for all Guillaume's changes


More information about the Sugar-devel mailing list