On Mon, May 26, 2008 at 5:09 PM, Tomeu Vizoso <[EMAIL PROTECTED]> wrote: > On Mon, May 26, 2008 at 4:01 PM, Morgan Collett > <[EMAIL PROTECTED]> wrote: >> On Mon, May 26, 2008 at 12:10 PM, Tomeu Vizoso <[EMAIL PROTECTED]> 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: http://dev.laptop.org/git?p=users/morgan/sugar-toolkit;a=shortlog;h=6298 - Guillaume's change, r+ from me http://dev.laptop.org/git?p=users/morgan/presence-service;a=shortlog;h=6298 - Guillaume's change, r+ from me http://dev.laptop.org/git?p=users/morgan/chat-activity;a=shortlog;h=6298 - r+ from me for all Guillaume's changes Regards Morgan _______________________________________________ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar