Lots of XEP-0004 bug fixes.

Forms have default type of 'form'
setFields now uses a list of tuples instead of a dictionary because ordering is important.
getFields defaults to returning a list of tuples, but the use_dict parameter can change that

2 files changed, 92 insertions, 59 deletions

diff --git a/sleekxmpp/plugins/xep_0004.py b/sleekxmpp/plugins/xep_0004.py
index 50f2b5ee..037fc090 100644
--- a/sleekxmpp/plugins/xep_0004.py
+++ b/sleekxmpp/plugins/xep_0004.py
@@ -23,10 +23,18 @@ class Form(ElementBase):
 	sub_interfaces = set(('title',))
 	form_types = set(('cancel', 'form', 'result', 'submit'))
 
-	def addField(self, var, ftype='text-single', label='', desc='', required=False, value=None, options=None):
+	def setup(self, xml=None):
+		if ElementBase.setup(self, xml): #if we had to generate xml
+			self['type'] = 'form'
+
+	def addField(self, var='', ftype=None, label='', desc='', required=False, value=None, options=None, **kwargs):
+		kwtype = kwargs.get('type', None)
+		if kwtype is None:
+			kwtype = ftype
+
 		field = FormField(parent=self)
 		field['var'] = var
-		field['type'] = ftype
+		field['type'] = kwtype
 		field['label'] = label
 		field['desc'] = desc
 		field['required'] = required
@@ -55,7 +63,10 @@ class Form(ElementBase):
 			field['var'] = var
 			field['value'] = values.get(var, None)
 
-	def addReported(self, var, ftype='text-single', label='', desc=''):
+	def addReported(self, var, ftype=None, label='', desc='', **kwargs):
+		kwtype = kwargs.get('type', None)
+		if kwtype is None:
+			kwtype = ftype
 		reported = self.xml.find('{%s}reported' % self.namespace)
 		if reported is None:
 			reported = ET.Element('{%s}reported' % self.namespace)
@@ -64,7 +75,7 @@ class Form(ElementBase):
 		reported.append(fieldXML)
 		field = FormField(xml=fieldXML)
 		field['var'] = var
-		field['type'] = ftype
+		field['type'] = kwtype
 		field['label'] = label
 		field['desc'] = desc
 		return field
@@ -92,19 +103,21 @@ class Form(ElementBase):
 		if reportedXML is not None:
 			self.xml.remove(reportedXML)
 
-	def getFields(self):
-		fields = {}
-		fieldsXML = self.xml.findall('{%s}field' % FormField.namespace)
+	def getFields(self, use_dict=False):
+		fields = {} if use_dict else []
+		fieldsXML = self.xml.findall('{%s}field' % FormField.namespace)    
 		for fieldXML in fieldsXML:
 			field = FormField(xml=fieldXML)
-			fields[field['var']] = field
+			if use_dict:
+				fields[field['var']] = field
+			else:
+				fields.append((field['var'], field))
 		return fields
 
 	def getInstructions(self):
 		instructions = ''
-		instsXML = self.xml.findall('{%s}instructions')
-		for instXML in instsXML:
-			instructions += instXML.text
+		instsXML = self.xml.findall('{%s}instructions' % self.namespace)
+		return "\n".join([instXML.text for instXML in instsXML])
 
 	def getItems(self):
 		items = []
@@ -129,7 +142,7 @@ class Form(ElementBase):
 
 	def getValues(self):
 		values = {}
-		fields = self.getFields()
+		fields = self.getFields(use_dict=True)
 		for var in fields:
 			values[var] = fields[var]['value']
 		return values
@@ -140,20 +153,19 @@ class Form(ElementBase):
 		elif self['type'] == 'submit':
 			self['type'] = 'result'
 
-	def setFields(self, fields):
+	def setFields(self, fields, default=None):
 		del self['fields']
-		for var in fields:
-			field = fields[var]
-
-			# Remap 'type' to 'ftype' to match the addField method
-			ftype = field.get('type', 'text-single')
-			field['type'] = ftype
-			del field['type']
-			field['ftype'] = ftype
+		for field_data in fields:
+			var = field_data[0]
+			field = field_data[1]
+			field['var'] = var
 
-			self.addField(var, **field)
+			self.addField(**field)
 
 	def setInstructions(self, instructions):
+		del self['instructions']
+		if instructions in [None, '']:
+			return
 		instructions = instructions.split('\n')
 		for instruction in instructions:
 			inst = ET.Element('{%s}instructions' % self.namespace)
@@ -164,20 +176,14 @@ class Form(ElementBase):
 		for item in items:
 			self.addItem(item)
 
-	def setReported(self, reported):
+	def setReported(self, reported, default=None):
 		for var in reported:
 			field = reported[var]
-
-			# Remap 'type' to 'ftype' to match the addReported method
-			ftype = field.get('type', 'text-single')
-			field['type'] = ftype
-			del field['type']
-			field['ftype'] = ftype
-
+			field['var'] = var
 			self.addReported(var, **field)
 
 	def setValues(self, values):
-		fields = self.getFields()
+		fields = self.getFields(use_dict=True)
 		for field in values:
 			fields[field]['value'] = values[field]
 
@@ -226,7 +232,7 @@ class FormField(ElementBase):
 		optsXML = self.xml.findall('{%s}option' % self.namespace)
 		for optXML in optsXML:
 			opt = FieldOption(xml=optXML)
-		options.append({'label': opt['label'], 'value':opt['value']})
+			options.append({'label': opt['label'], 'value':opt['value']})
 		return options
 
 	def getRequired(self):
@@ -277,22 +283,24 @@ class FormField(ElementBase):
 	def setValue(self, value):
 		self.delValue()
 		valXMLName = '{%s}value' % self.namespace
-	
+
 		if self['type'] == 'boolean':
 			if value in self.true_values:
 				valXML = ET.Element(valXMLName)
-				valXML.text = 'true'
+				valXML.text = '1'
 				self.xml.append(valXML)
 			else:
 				valXML = ET.Element(valXMLName)
-				valXML.text = 'true'
+				valXML.text = '0'
 				self.xml.append(valXML)
-		if self['type'] in self.multi_value_types:
+		elif self['type'] in self.multi_value_types or self['type'] in ['', None]:
 			if self['type'] in self.multi_line_types and isinstance(value, str):
 				value = value.split('\n')
 			if not isinstance(value, list):
 				value = [value]
 			for val in value:
+				if self['type'] in ['', None] and val in self.true_values:
+					val = '1'
 				valXML = ET.Element(valXMLName)
 				valXML.text = val
 				self.xml.append(valXML)
diff --git a/tests/test_forms.py b/tests/test_forms.py
index 16160249..7d37506d 100644
--- a/tests/test_forms.py
+++ b/tests/test_forms.py
@@ -1,5 +1,5 @@
 from sleektest import *
-import sleekxmpp.plugins.alt_0004 as xep_0004
+import sleekxmpp.plugins.xep_0004 as xep_0004
 
 
 class TestDataForms(SleekTest):
@@ -16,19 +16,19 @@ class TestDataForms(SleekTest):
 
         self.checkMessage(msg, """
           <message>
-            <x xmlns="jabber:x:data">
+            <x xmlns="jabber:x:data" type="form">
               <instructions>Instructions</instructions>
               <instructions>Second batch</instructions>
             </x>
           </message>
-        """, use_values=False)
+        """)
 
     def testAddField(self):
         """Testing adding fields to a data form."""
 
         msg = self.Message()
         form = msg['form']
-        form.addField('f1', 
+        form.addField(var='f1', 
                       ftype='text-single', 
                       label='Text',
                       desc='A text field', 
@@ -37,7 +37,7 @@ class TestDataForms(SleekTest):
 
         self.checkMessage(msg, """
           <message>
-            <x xmlns="jabber:x:data">
+            <x xmlns="jabber:x:data" type="form">
               <field var="f1" type="text-single" label="Text">
                 <desc>A text field</desc>
                 <required />
@@ -45,26 +45,26 @@ class TestDataForms(SleekTest):
               </field>
             </x>
           </message>
-        """, use_values=False)
+        """)
 
-        form['fields'] = {'f1': {'type': 'text-single', 
-                                 'label': 'Username', 
-                                 'required': True},
-                          'f2': {'type': 'text-private',
-                                 'label': 'Password',
-                                 'required': True},
-                          'f3': {'type': 'text-multi',
-                                 'label': 'Message',
-                                 'value': 'Enter message.\nA long one even.'},
-                          'f4': {'type': 'list-single',
-                                 'label': 'Message Type',
-                                 'options': [{'label': 'Cool!', 
-                                              'value': 'cool'},
-                                             {'label': 'Urgh!',
-                                              'value': 'urgh'}]}}
+        form['fields'] = [('f1', {'type': 'text-single', 
+                                  'label': 'Username', 
+                                  'required': True}),
+                          ('f2', {'type': 'text-private',
+                                  'label': 'Password',
+                                  'required': True}),
+                          ('f3', {'type': 'text-multi',
+                                  'label': 'Message',
+                                  'value': 'Enter message.\nA long one even.'}),
+                          ('f4', {'type': 'list-single',
+                                  'label': 'Message Type',
+                                  'options': [{'label': 'Cool!', 
+                                               'value': 'cool'},
+                                              {'label': 'Urgh!',
+                                               'value': 'urgh'}]})]
         self.checkMessage(msg, """
           <message>
-            <x xmlns="jabber:x:data">
+            <x xmlns="jabber:x:data" type="form">
               <field var="f1" type="text-single" label="Username">
                 <required />
               </field>
@@ -85,6 +85,31 @@ class TestDataForms(SleekTest):
               </field>
             </x>
           </message>
-        """, use_values=False)
+        """)
+
+    def testSetValues(self):
+        """Testing setting form values"""
+        
+        msg = self.Message()
+        form = msg['form']
+        form.setFields([
+                ('foo', {'type': 'text-single'}),
+                ('bar', {'type': 'list-multi'})])
+        
+        form.setValues({'foo': 'Foo!',
+                        'bar': ['a', 'b']})
+
+        self.checkMessage(msg, """
+          <message>
+            <x xmlns="jabber:x:data" type="form">
+              <field var="foo" type="text-single">
+                <value>Foo!</value>
+              </field>
+              <field var="bar" type="list-multi">
+                <value>a</value>
+                <value>b</value>
+              </field>
+            </x>
+          </message>""")
 
 suite = unittest.TestLoader().loadTestsFromTestCase(TestDataForms)