more cgi form parsing tests, and a fix for an outstanding couple of bugs

Author: Richard Jones

doc/upgrading.txt

@@ -13,7 +13,6 @@ Migrating from 0.5 to 0.6
 - Introduced EMAIL_FROM_TAG config variable.
 
 
-
 Migrating from 0.4.x to 0.5.0
 =============================
 

roundup/cgi/client.py

@@ -1,4 +1,4 @@
-# $Id: client.py,v 1.68 2003-01-14 22:21:35 richard Exp $
+# $Id: client.py,v 1.69 2003-01-15 11:07:45 richard Exp $
 
 __doc__ = """
 WWW request handler (also used in the stand-alone server).
@@ -1242,6 +1242,9 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
                 # it's a MiniFieldStorage, but may be a comma-separated list
                 # of values
                 value = [i.strip() for i in value.value.split(',')]
+
+            # filter out the empty bits
+            value = filter(None, value)
         else:
             # multiple values are not OK
             if isinstance(value, type([])):
@@ -1302,12 +1305,13 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
         elif isinstance(proptype, hyperdb.Multilink):
             # perform link class key value lookup if necessary
             link = proptype.classname
+            link_cl = db.classes[link]
             l = []
             for entry in value:
                 if not entry: continue
                 if not num_re.match(entry):
                     try:
-                        entry = db.classes[link].lookup(entry)
+                        entry = link_cl.lookup(entry)
                     except KeyError:
                         raise ValueError, _('property "%(propname)s": '
                             '"%(value)s" not an entry of %(classname)s')%{
@@ -1371,6 +1375,10 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
             if not existing and not value:
                 continue
 
+            # existing will come out unsorted in some cases
+            if isinstance(proptype, hyperdb.Multilink):
+                existing.sort()
+
             # if changed, set it
             if value != existing:
                 props[propname] = value

test/test_cgi.py

@@ -8,7 +8,7 @@
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 #
-# $Id: test_cgi.py,v 1.2 2003-01-14 22:21:35 richard Exp $
+# $Id: test_cgi.py,v 1.3 2003-01-15 11:07:45 richard Exp $
 
 import unittest, os, shutil, errno, sys, difflib, cgi
 
@@ -70,6 +70,8 @@ def testEmptyString(self):
             makeForm({'title': ''})), {})
         self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
             makeForm({'title': ' '})), {})
+        self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
+            self.db.issue, makeForm({'title': ['', '']}))
 
     def testSetString(self):
         self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
@@ -97,10 +99,14 @@ def testEmptyMultilink(self):
     def testSetMultilink(self):
         self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
             makeForm({'nosy': '1'})), {'nosy': ['1']})
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({'nosy': 'admin'})), {'nosy': ['1']})
         self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
             makeForm({'nosy': ['1','2']})), {'nosy': ['1','2']})
         self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
             makeForm({'nosy': '1,2'})), {'nosy': ['1','2']})
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({'nosy': 'admin,2'})), {'nosy': ['1','2']})
 
     def testEmptyMultilinkSet(self):
         nodeid = self.db.issue.create(nosy=['1','2'])
@@ -110,6 +116,48 @@ def testEmptyMultilinkSet(self):
         self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
             makeForm({'nosy': ' '}), nodeid), {'nosy': []})
 
+    def testInvalidMultilinkValue(self):
+# XXX This is not the current behaviour - should we enforce this?
+#        self.assertRaises(IndexError, client.parsePropsFromForm, self.db,
+#            self.db.issue, makeForm({'nosy': '4'}))
+        self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
+            self.db.issue, makeForm({'nosy': 'frozzle'}))
+        self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
+            self.db.issue, makeForm({'nosy': '1,frozzle'}))
+# XXX need a test for the TypeError (where the ML class doesn't define a key
+
+    def testMultilinkAdd(self):
+        nodeid = self.db.issue.create(nosy=['1'])
+        # do nothing
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({':add:nosy': ''}), nodeid), {})
+
+        # do something ;)
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({':add:nosy': '2'}), nodeid), {'nosy': ['1','2']})
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({':add:nosy': '2,mary'}), nodeid), {'nosy': ['1','2','4']})
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({':add:nosy': ['2','3']}), nodeid), {'nosy': ['1','2','3']})
+
+    def testMultilinkRemove(self):
+        nodeid = self.db.issue.create(nosy=['1','2'])
+        # do nothing
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({':remove:nosy': ''}), nodeid), {})
+
+        # do something ;)
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({':remove:nosy': '1'}), nodeid), {'nosy': ['2']})
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({':remove:nosy': 'admin,2'}), nodeid), {'nosy': []})
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({':remove:nosy': ['1','2']}), nodeid), {'nosy': []})
+
+        # remove one that doesn't exist?
+        self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
+            self.db.issue, makeForm({':remove:nosy': '4'}), nodeid)
+
     #
     # Password
     #
@@ -118,6 +166,11 @@ def testEmptyPassword(self):
             makeForm({'password': ''})), {})
         self.assertEqual(client.parsePropsFromForm(self.db, self.db.user,
             makeForm({'password': ''})), {})
+        self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
+            self.db.user, makeForm({'password': ['', '']}))
+        self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
+            self.db.user, makeForm({'password': 'foo',
+            'password:confirm': ['', '']}))
 
     def testSetPassword(self):
         self.assertEqual(client.parsePropsFromForm(self.db, self.db.user,