trying out the hint from github issue

[plcapi.git] / PLC / Persons.py

diff --git a/PLC/Persons.py b/PLC/Persons.py
index e0e4c84..d1b3c79 100644 (file)
--- a/PLC/Persons.py
+++ b/PLC/Persons.py
@@ -4,9 +4,6 @@
 # Mark Huang <mlhuang@cs.princeton.edu>
 # Copyright (C) 2006 The Trustees of Princeton University
 #
-# $Id$
-# $URL$
-#
 
 from types import StringTypes
 try:
@@ -81,7 +78,7 @@ class Person(Row):
         Validate email address. Stolen from Mailman.
         """
         email = email.lower()
-        invalid_email = PLCInvalidArgument("Invalid e-mail address")
+        invalid_email = PLCInvalidArgument("Invalid e-mail address %s"%email)
 
         if not email:
             raise invalid_email
@@ -146,8 +143,8 @@ class Person(Row):
 
         if 'pi' in self['roles']:
             if set(self['site_ids']).intersection(person['site_ids']):
-                # Can update person is neither a PI or ADMIN
-                return (not (('pi' in person['roles']) or ('admin' in person['roles'])))
+                # non-admin users cannot update a person who is neither a PI or ADMIN
+                return (not set(['pi','admin']).intersection(person['roles']))
 
         return False
 
@@ -158,7 +155,7 @@ class Person(Row):
 
         1. We are the person.
         2. We are an admin.
-        3. We are a PI and the person is at one of our sites.
+        3. We are a PI or Tech and the person is at one of our sites.
         """
 
         assert isinstance(person, Person)
@@ -166,10 +163,10 @@ class Person(Row):
         if self.can_update(person):
             return True
 
-        if 'pi' in self['roles']:
+        # pis and techs can see all people on their site
+        if set(['pi','tech']).intersection(self['roles']):
             if set(self['site_ids']).intersection(person['site_ids']):
-                # Can view people with equal or higher role IDs
-                return 'admin' not in person['roles']
+                return True
 
         return False
 
@@ -362,6 +359,12 @@ class Person(Row):
 
         # Mark as deleted
         self['deleted'] = True
+
+        # delete will fail if timestamp fields aren't validated, so lets remove them
+        for field in ['verification_expires', 'date_created', 'last_updated']:
+            if field in self:
+                self.pop(field)
+
         # don't validate, so duplicates can be consistently removed
         self.sync(commit, validate=False)
 
@@ -390,10 +393,11 @@ class Persons(Table):
                 person_filter = Filter(Person.fields, {'person_id': ints, 'email': strs})
                 sql += " AND (%s) %s" % person_filter.sql(api, "OR")
             elif isinstance(person_filter, dict):
-                person_filter = Filter(Person.fields, person_filter)
+                allowed_fields = dict(Person.fields.items() + Person.tags.items())
+                person_filter = Filter(allowed_fields, person_filter)
                 sql += " AND (%s) %s" % person_filter.sql(api, "AND")
             elif isinstance (person_filter, StringTypes):
-                person_filter = Filter(Person.fields, {'email':person_filter})
+                person_filter = Filter(Person.fields, {'email' : person_filter})
                 sql += " AND (%s) %s" % person_filter.sql(api, "AND")
             elif isinstance (person_filter, (int, long)):
                 person_filter = Filter(Person.fields, {'person_id':person_filter})