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

[Tomeu Vizoso]

Hi,

         self._dict = {}
+        self._private_invites = {}

_private_invites doesn't seem to be used in the patch.

         owner.connect('joined-activity', self._owner_joined_cb)
+        # FIXME need equivalent for ^ for private invite

Having a ticket number may be a good way of tracking this?

-    def add_invite(self, issuer, bundle_id, activity_id):
+    def add_invite(self, bundle_id, activity_id):

We know that one of these days (perhaps for the August release), we'll
need a version number to identify unequivocally a bundle. Would make
sense to add it now to the API even if it's not used yet?

+        invite = Invite(bundle_id,
+                        private_connection=private_connection)

This could fit in one line.

+        self._dict.pop(invite.get_private_connection())

if you are not going to use the result of pop(), then I think using
'del' would be clearer.

+        invite = self._dict.get(private_connection)

Why not self._dict[private_connection]?

         tp_channel = json.write([str(bus_name), str(connection),
                                  str(channel)])

Instead of converting to str every value, why not using simplejson? It
will accept subclasses of the base types. python-json doesn't seem to
be very actively maintained right now.

+        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?

+    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?

+from sugar import activity, profile

I think this should be two lines.

+        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?

+            if not activity_model:
+                return

Should this raise an exception instead? Or at least log an error/warning?

Sorry about the nipicks!

Tomeu