fixes to some unit tests, and a cleanup

git-svn-id: http://svn.roundup-tracker.org/svnroot/roundup/trunk@2101 57a73879-2fb5-44c3-a270-3262357dd7e2

diff --git a/roundup/cgi/actions.py b/roundup/cgi/actions.py
index 43346c237e98d2a033e716b5b8c3f2c226145735..36a4d9dc1b1127b3c3d217eaa3bfde3b319a298f 100755 (executable)
--- a/roundup/cgi/actions.py
+++ b/roundup/cgi/actions.py
@@ -14,7 +14,7 @@ __all__ = ['Action', 'ShowAction', 'RetireAction', 'SearchAction',
 # used by a couple of routines
 chars = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'
 
-class Action:    
+class Action:
     def __init__(self, client):
         self.client = client
         self.form = client.form
@@ -25,7 +25,7 @@ class Action:
         self.userid = client.userid
         self.base = client.base
         self.user = client.user
-        
+
     def execute(self):
         """Execute the action specified by this object."""
         self.permission()
@@ -40,20 +40,20 @@ class Action:
         a simple permission, check whether the user has that permission.
         Subclasses must also define the name attribute if they define
         permissionType.
-        
+
         Despite having this permission, users may still be unauthorised to
-        perform parts of actions. It is up to the subclasses to detect this.        
+        perform parts of actions. It is up to the subclasses to detect this.
         """
         if (self.permissionType and
-            not self.hasPermission(self.permissionType)):
-
-            raise Unauthorised, _('You do not have permission to %s the %s class.' %
-                                  (self.name, self.classname))
+                not self.hasPermission(self.permissionType)):
+            info = {'action': self.name, 'classname': self.classname}
+            raise Unauthorised, _('You do not have permission to '
+                '%(action)s the %(classname)s class.')%info
 
     def hasPermission(self, permission):
         """Check whether the user has 'permission' on the current class."""
         return self.db.security.hasPermission(permission, self.client.userid,
-                                              self.client.classname)
+            self.client.classname)
 
 class ShowAction(Action):
     def handle(self, typere=re.compile('[@:]type'),
@@ -75,7 +75,7 @@ class RetireAction(Action):
     permissionType = 'Edit'
 
     def handle(self):
-        """Retire the context item."""        
+        """Retire the context item."""
         # if we want to view the index template now, then unset the nodeid
         # context info (a special-case for retire actions on the index page)
         nodeid = self.nodeid
@@ -98,7 +98,7 @@ class RetireAction(Action):
 class SearchAction(Action):
     name = 'search'
     permissionType = 'View'
-    
+
     def handle(self, wcre=re.compile(r'[\s,]+')):
         """Mangle some of the form variables.
 
@@ -113,7 +113,7 @@ class SearchAction(Action):
 
         """
         self.fakeFilterVars()
-        queryname = self.getQueryName()        
+        queryname = self.getQueryName()
 
         # handle saving the query params
         if queryname:
@@ -165,7 +165,7 @@ class SearchAction(Action):
                         # replace the single value with the split list
                         for v in l:
                             self.form.value.append(cgi.MiniFieldStorage(key, v))
-        
+
             self.form.value.append(cgi.MiniFieldStorage('@filter', key))
 
     FV_QUERYNAME = re.compile(r'[@:]queryname')
@@ -178,7 +178,7 @@ class SearchAction(Action):
 class EditCSVAction(Action):
     name = 'edit'
     permissionType = 'Edit'
-    
+
     def handle(self):
         """Performs an edit of all of a class' items in one go.
 
@@ -270,13 +270,13 @@ class EditCSVAction(Action):
         self.db.commit()
 
         self.client.ok_message.append(_('Items edited OK'))
-    
+
 class _EditAction(Action):
     def isEditingSelf(self):
         """Check whether a user is editing his/her own details."""
         return (self.nodeid == self.userid
                 and self.db.user.get(self.nodeid, 'username') != 'anonymous')
-    
+
     def editItemPermission(self, props):
         """Determine whether the user has permission to edit this item.
 
@@ -444,12 +444,12 @@ class EditItemAction(_EditAction):
 
     def handleCollision(self):
         self.client.template = 'collision'
-    
+
     def handle(self):
         """Perform an edit of an item in the database.
 
         See parsePropsFromForm and _editnodes for special variables.
-        
+
         """
         if self.detectCollision(self.lastUserActivity(), self.lastNodeActivity()):
             self.handleCollision()
@@ -480,7 +480,7 @@ class EditItemAction(_EditAction):
             req = templating.HTMLRequest(self)
             url += '&' + req.indexargs_href('', {})[1:]
         raise Redirect, url
-    
+
 class NewItemAction(_EditAction):
     def handle(self):
         ''' Add a new item to the database.
@@ -512,14 +512,14 @@ class NewItemAction(_EditAction):
         raise Redirect, '%s%s%s?@ok_message=%s&@template=%s'%(self.base,
             self.classname, self.nodeid, urllib.quote(messages),
             urllib.quote(self.template))
-        
+
 class PassResetAction(Action):
     def handle(self):
         """Handle password reset requests.
-    
+
         Presence of either "name" or "address" generates email. Presence of
         "otk" performs the reset.
-    
+
         """
         if self.form.has_key('otk'):
             # pull the rego information out of the otk database
@@ -621,7 +621,7 @@ class ConfRegoAction(Action):
         except (ValueError, KeyError), message:
             self.client.error_message.append(str(message))
             return
-        
+
         # log the new user in
         self.client.user = self.db.user.get(self.userid, 'username')
         # re-open the database for real, using the user
@@ -645,19 +645,19 @@ class ConfRegoAction(Action):
 class RegisterAction(Action):
     name = 'register'
     permissionType = 'Web Registration'
-    
+
     def handle(self):
         """Attempt to create a new user based on the contents of the form
         and then set the cookie.
 
         Return 1 on successful login.
-        """        
+        """
         props = self.client.parsePropsFromForm(create=1)[0][('user', None)]
 
         # registration isn't allowed to supply roles
         if props.has_key('roles'):
             raise Unauthorised, _("It is not permitted to supply roles "
-                "at registration.")            
+                "at registration.")
 
         username = props['username']
         try:

diff --git a/roundup/cgi/client.py b/roundup/cgi/client.py
index e095de1ea213e46ea8890de67480c331ab5e18a0..4a2a78d1fa4f0d01c6e8b7b88a7c945184571d1d 100644 (file)
--- a/roundup/cgi/client.py
+++ b/roundup/cgi/client.py
@@ -1,4 +1,4 @@
-# $Id: client.py,v 1.161 2004-02-15 10:08:20 jlgijsbers Exp $
+# $Id: client.py,v 1.162 2004-02-20 03:48:16 richard Exp $
 
 """WWW request handler (also used in the stand-alone server).
 """
@@ -239,7 +239,7 @@ class Client:
 
     def clean_sessions(self):
         """Age sessions, remove when they haven't been used for a week.
-        
+
         Do it only once an hour.
 
         Note: also cleans One Time Keys, and other "session" based stuff.
@@ -558,8 +558,8 @@ class Client:
                 # old way of specifying actions
                 getattr(self, action_klass)()
             else:
-                action_klass(self).handle()
-                
+                action_klass(self).execute()
+
         except ValueError, err:
             self.error_message.append(str(err))
 

diff --git a/test/test_actions.py b/test/test_actions.py
index 2c807c125a332606d0d5025e6420490d0f991154..ebf39dfc5ed727e92e1d999aeca43e022adfcfa3 100755 (executable)
--- a/test/test_actions.py
+++ b/test/test_actions.py
@@ -20,11 +20,11 @@ class MockNull:
         #\r
         # For example (with just 'client' defined):\r
         #\r
-        # client.db.config.TRACKER_WEB = 'BASE/'        \r
+        # client.db.config.TRACKER_WEB = 'BASE/'\r
         setattr(self, name, MockNull())\r
         return getattr(self, name)\r
 \r
-    def __getitem__(self, key): return self    \r
+    def __getitem__(self, key): return self\r
     def __nonzero__(self): return 0\r
     def __str__(self): return ''\r
 \r
@@ -36,7 +36,7 @@ class ActionTestCase(unittest.TestCase):
         self.form = FieldStorage()\r
         self.client = MockNull()\r
         self.client.form = self.form\r
-    \r
+\r
 class ShowActionTestCase(ActionTestCase):\r
     def assertRaisesMessage(self, exception, callable, message, *args, **kwargs):\r
         try:\r
@@ -47,7 +47,7 @@ class ShowActionTestCase(ActionTestCase):
             if hasattr(excClass,'__name__'): excName = excClass.__name__\r
             else: excName = str(excClass)\r
             raise self.failureException, excName\r
-    \r
+\r
     def testShowAction(self):\r
         self.client.db.config.TRACKER_WEB = 'BASE/'\r
 \r
@@ -56,7 +56,7 @@ class ShowActionTestCase(ActionTestCase):
 \r
         self.form.value.append(MiniFieldStorage('@type', 'issue'))\r
         self.assertRaisesMessage(Redirect, action.handle, 'BASE/issue')\r
-        \r
+\r
         self.form.value.append(MiniFieldStorage('@number', '1'))\r
         self.assertRaisesMessage(Redirect, action.handle, 'BASE/issue1')\r
 \r
@@ -68,13 +68,18 @@ class RetireActionTestCase(ActionTestCase):
         self.assert_(len(self.client.ok_message) == 1)\r
 \r
     def testNoPermission(self):\r
-        self.assertRaises(Unauthorised, RetireAction(self.client).handle)\r
+        self.assertRaises(Unauthorised, RetireAction(self.client).execute)\r
 \r
     def testDontRetireAdminOrAnonymous(self):\r
         self.client.db.security.hasPermission=true\r
-        self.client.classname = 'user'        \r
+        # look up the user class\r
+        self.client.classname = 'user'\r
+        # but always look up admin, regardless of nodeid\r
         self.client.db.user.get = lambda a,b: 'admin'\r
         self.assertRaises(ValueError, RetireAction(self.client).handle)\r
+        # .. or anonymous\r
+        self.client.db.user.get = lambda a,b: 'anonymous'\r
+        self.assertRaises(ValueError, RetireAction(self.client).handle)\r
 \r
 class SearchActionTestCase(ActionTestCase):\r
     def setUp(self):\r
@@ -83,8 +88,8 @@ class SearchActionTestCase(ActionTestCase):
 \r
 class StandardSearchActionTestCase(SearchActionTestCase):\r
     def testNoPermission(self):\r
-        self.assertRaises(Unauthorised, self.action.handle)\r
-    \r
+        self.assertRaises(Unauthorised, self.action.execute)\r
+\r
     def testQueryName(self):\r
         self.assertEqual(self.action.getQueryName(), '')\r
 \r
@@ -99,7 +104,7 @@ class FakeFilterVarsTestCase(SearchActionTestCase):
     def assertFilterEquals(self, expected):\r
         self.action.fakeFilterVars()\r
         self.assertEqual(self.form.getvalue('@filter'), expected)\r
-        \r
+\r
     def testEmptyMultilink(self):\r
         self.form.value.append(MiniFieldStorage('foo', ''))\r
         self.form.value.append(MiniFieldStorage('foo', ''))\r
@@ -128,7 +133,7 @@ class FakeFilterVarsTestCase(SearchActionTestCase):
     def testTokenizedStringKey(self):\r
         self.client.db.classes.getprops = lambda: {'foo': hyperdb.String()}\r
         self.form.value.append(MiniFieldStorage('foo', 'hello world'))\r
-        \r
+\r
         self.assertFilterEquals('foo')\r
 \r
         # The single value gets replaced with the tokenized list.\r
@@ -139,11 +144,11 @@ class CollisionDetectionTestCase(ActionTestCase):
         ActionTestCase.setUp(self)\r
         self.action = EditItemAction(self.client)\r
         self.now = Date('.')\r
-        \r
+\r
     def testLastUserActivity(self):\r
         self.assertEqual(self.action.lastUserActivity(), None)\r
 \r
-        self.client.form.value.append(MiniFieldStorage('@lastactivity', str(self.now)))        \r
+        self.client.form.value.append(MiniFieldStorage('@lastactivity', str(self.now)))\r
         self.assertEqual(self.action.lastUserActivity(), self.now)\r
 \r
     def testLastNodeActivity(self):\r
@@ -161,8 +166,8 @@ class CollisionDetectionTestCase(ActionTestCase):
     def testCollision(self):\r
         self.failUnless(self.action.detectCollision(self.now, self.now + Interval("1d")))\r
         self.failIf(self.action.detectCollision(self.now, self.now - Interval("1d")))\r
-        self.failIf(self.action.detectCollision(None, self.now))        \r
-        \r
+        self.failIf(self.action.detectCollision(None, self.now))\r
+\r
 def test_suite():\r
     suite = unittest.TestSuite()\r
     suite.addTest(unittest.makeSuite(RetireActionTestCase))\r