Re: [sugar] review: Guillaume's 'activity' branch of Gadget
Le mardi 10 juin 2008 à 14:42 +0100, Dafydd Harries a écrit :
diff --git a/gadget/component.py b/gadget/component.py
index b5fc702..dd85a03 100644
--- a/gadget/component.py
+++ b/gadget/component.py
@@ -92,8 +92,9 @@ class Room(object):
def __init__(self, own_nick):
self.own_nick = own_nick
self.membership = None
-self.members = {}
+self.members = set()
self.properties = {}
+self.id = None
Would it be possible to just have a Room.activity property and use that to
store members/id?
I'll remove the Room object and move membership management to Activity
as we discussed yesterday.
class GadgetService(component.Service):
debug = False
@@ -333,7 +334,7 @@ class GadgetService(component.Service):
def message(self, stanza):
type = stanza.getAttribute('type', 'normal')
-if type in ('chat', 'groupchat'):
+if type in ('chat'):
return
This doesn't do quite what you want. () doesn't create a tuple, , does, so
this is the same as type in 'chat', which will be true if type is a
substring of chat. Better to just say ==.
Right. Forgot about that, good catch. Fixed.
if type == 'normal':
@@ -343,12 +344,17 @@ class GadgetService(component.Service):
return
elif (elem.uri == ns.ACTIVITY_PROP and
elem.name == 'properties'):
-self.message_activity_properties(stanza, elem)
+self.message_pre_invite_activity_properties(stanza,
elem)
return
elif elem.uri == ns.PUBSUB_EVENT and elem.name == 'event':
self.message_pubsub_notification(stanza, elem)
return
+elif type == 'groupchat':
+for elem in stanza.elements():
+if elem.uri == ns.ACTIVITY_PROP and elem.name ==
'properties':
+self.message_activity_properties(stanza, elem)
You should return here. Perhaps we should dispatch messages more like we
dispatch IQs, i.e. using a dict.
fixed. I added a FIXME suggestion a better dispatch system.
+
# FIXME: handle close query stanza
log.msg('got unhandled message')
@@ -357,7 +363,7 @@ class GadgetService(component.Service):
if elem.uri == ns.MUC_USER and elem.name == 'invite':
self.message_muc_invite(stanza, elem)
-def create_room(self, jid, properties):
+def create_room(self, jid, properties, id):
Hmm, can we put the id before the properties? I think it's more aesthetically
pleasing that way.
done.
if jid in self.mucs:
# We already have a room by this name.
return
@@ -366,9 +372,10 @@ class GadgetService(component.Service):
# join it.
room = Room('inspector')
room.properties = properties
+room.id = id
self.mucs[jid] = room
-def message_activity_properties(self, stanza, properties_elem):
+def message_pre_invite_activity_properties(self, stanza,
properties_elem):
# XXX Restrictions on from address?
try:
@@ -384,7 +391,7 @@ class GadgetService(component.Service):
if not (properties and activity and room_jid):
return
-self.create_room(room_jid, properties)
+self.create_room(room_jid, properties, activity)
Perhaps we should rename 'activity' to 'activity_id' for clarity.
done.
def message_muc_invite(self, stanza, invite):
to = stanza.getAttribute('to')
@@ -502,17 +509,54 @@ class GadgetService(component.Service):
# Presence for our in-room self; we've finished joining the
# room.
room.membership = 'joined'
+
+# activities are created and added to the model when the
+# inspector actually joined the muc.
English nitpicks: Your tenses don't agree here. are created ... joined.
s/joined/join/. If you use a full stop, you should also capitalise the first
letter.
fixed.
+# If we'll do it before, we won't be able to properly track
+# activity's properties and members.
I don't understand this comment.
Humm me neither actually :) Removed.
+activity = self.model.activity_add(room.id, room_jid,
+room.properties)
+
+# Activity and Room now share the same set() object
+activity.members = room.members
return
+item = xpath_query('/presence/[EMAIL PROTECTED]%s]/item' %
ns.MUC_USER,
+stanza)
+if not item or item[0].getAttribute('jid') is None:
+log.msg(full jid of %s missing. Can't update activity
[sugar] review: Guillaume's 'activity' branch of Gadget
diff --git a/gadget/component.py b/gadget/component.py
index b5fc702..dd85a03 100644
--- a/gadget/component.py
+++ b/gadget/component.py
@@ -92,8 +92,9 @@ class Room(object):
def __init__(self, own_nick):
self.own_nick = own_nick
self.membership = None
-self.members = {}
+self.members = set()
self.properties = {}
+self.id = None
Would it be possible to just have a Room.activity property and use that to
store members/id?
class GadgetService(component.Service):
debug = False
@@ -333,7 +334,7 @@ class GadgetService(component.Service):
def message(self, stanza):
type = stanza.getAttribute('type', 'normal')
-if type in ('chat', 'groupchat'):
+if type in ('chat'):
return
This doesn't do quite what you want. () doesn't create a tuple, , does, so
this is the same as type in 'chat', which will be true if type is a
substring of chat. Better to just say ==.
if type == 'normal':
@@ -343,12 +344,17 @@ class GadgetService(component.Service):
return
elif (elem.uri == ns.ACTIVITY_PROP and
elem.name == 'properties'):
-self.message_activity_properties(stanza, elem)
+self.message_pre_invite_activity_properties(stanza, elem)
return
elif elem.uri == ns.PUBSUB_EVENT and elem.name == 'event':
self.message_pubsub_notification(stanza, elem)
return
+elif type == 'groupchat':
+for elem in stanza.elements():
+if elem.uri == ns.ACTIVITY_PROP and elem.name ==
'properties':
+self.message_activity_properties(stanza, elem)
You should return here. Perhaps we should dispatch messages more like we
dispatch IQs, i.e. using a dict.
+
# FIXME: handle close query stanza
log.msg('got unhandled message')
@@ -357,7 +363,7 @@ class GadgetService(component.Service):
if elem.uri == ns.MUC_USER and elem.name == 'invite':
self.message_muc_invite(stanza, elem)
-def create_room(self, jid, properties):
+def create_room(self, jid, properties, id):
Hmm, can we put the id before the properties? I think it's more aesthetically
pleasing that way.
if jid in self.mucs:
# We already have a room by this name.
return
@@ -366,9 +372,10 @@ class GadgetService(component.Service):
# join it.
room = Room('inspector')
room.properties = properties
+room.id = id
self.mucs[jid] = room
-def message_activity_properties(self, stanza, properties_elem):
+def message_pre_invite_activity_properties(self, stanza,
properties_elem):
# XXX Restrictions on from address?
try:
@@ -384,7 +391,7 @@ class GadgetService(component.Service):
if not (properties and activity and room_jid):
return
-self.create_room(room_jid, properties)
+self.create_room(room_jid, properties, activity)
Perhaps we should rename 'activity' to 'activity_id' for clarity.
def message_muc_invite(self, stanza, invite):
to = stanza.getAttribute('to')
@@ -502,17 +509,54 @@ class GadgetService(component.Service):
# Presence for our in-room self; we've finished joining the
# room.
room.membership = 'joined'
+
+# activities are created and added to the model when the
+# inspector actually joined the muc.
English nitpicks: Your tenses don't agree here. are created ... joined.
s/joined/join/. If you use a full stop, you should also capitalise the first
letter.
+# If we'll do it before, we won't be able to properly track
+# activity's properties and members.
I don't understand this comment.
+activity = self.model.activity_add(room.id, room_jid,
+room.properties)
+
+# Activity and Room now share the same set() object
+activity.members = room.members
return
+item = xpath_query('/presence/[EMAIL PROTECTED]%s]/item' %
ns.MUC_USER,
+stanza)
+if not item or item[0].getAttribute('jid') is None:
+log.msg(full jid of %s missing. Can't update activity members
+% jid.resource)
English nitpicks: you should capitalise full and have a full stop after
the second sentence.
+return
+
+full_jid = JID(item[0]['jid']).userhost()
Hmm, full_jid seems like a bad name, since it implies a JID with a resource.
Perhaps it should be real_jid or just jid.
+
+activities = self.model.activity_by_room(room_jid)
+
if type is None:
log.msg('room %s has
