Code

can now unset values in CSV editing (sf bug 704788)
[roundup.git] / roundup / cgi / client.py
index 10015b317ef3b87eb84ffd69f08cbb9896bfe255..a96babfd7e6ccb7af54ad7c4fa2a69d5b1ae18f5 100644 (file)
@@ -1,34 +1,53 @@
-# $Id: client.py,v 1.85 2003-02-14 00:31:46 richard Exp $
+# $Id: client.py,v 1.108 2003-03-19 02:50:40 richard Exp $
 
 __doc__ = """
 WWW request handler (also used in the stand-alone server).
 """
 
 import os, os.path, cgi, StringIO, urlparse, re, traceback, mimetypes, urllib
-import binascii, Cookie, time, random
+import binascii, Cookie, time, random, MimeWriter, smtplib, socket, quopri
+import stat, rfc822, string
 
 from roundup import roundupdb, date, hyperdb, password
 from roundup.i18n import _
-
 from roundup.cgi.templating import Templates, HTMLRequest, NoTemplate
 from roundup.cgi import cgitb
-
 from roundup.cgi.PageTemplates import PageTemplate
-
-class Unauthorised(ValueError):
-    pass
-
-class NotFound(ValueError):
-    pass
-
-class Redirect(Exception):
+from roundup.rfc2822 import encode_header
+from roundup.mailgw import uidFromAddress
+
+class HTTPException(Exception):
+      pass
+class  Unauthorised(HTTPException):
+       pass
+class  NotFound(HTTPException):
+       pass
+class  Redirect(HTTPException):
+       pass
+class  NotModified(HTTPException):
+       pass
+
+# set to indicate to roundup not to actually _send_ email
+# this var must contain a file to write the mail to
+SENDMAILDEBUG = os.environ.get('SENDMAILDEBUG', '')
+
+
+# XXX actually _use_ FormError
+class FormError(ValueError):
+    ''' An "expected" exception occurred during form parsing.
+        - ie. something we know can go wrong, and don't want to alarm the
+          user with
+
+        We trap this at the user interface level and feed back a nice error
+        to the user.
+    '''
     pass
 
 class SendFile(Exception):
-    ' Sent a file from the database '
+    ''' Send a file from the database '''
 
 class SendStaticFile(Exception):
-    ' Send a static file from the instance html directory '
+    ''' Send a static file from the instance html directory '''
 
 def initialiseSecurity(security):
     ''' Create some Permissions and Roles on the security object
@@ -83,29 +102,39 @@ class Client:
     Special form variables:
      Note that in various places throughout this code, special form
      variables of the form :<name> are used. The colon (":") part may
-     actually be one of several characters from the set:
-
-       : @ + 
-
+     actually be one of either ":" or "@".
     '''
 
     #
     # special form variables
     #
-    FV_TEMPLATE = re.compile(r'[@+:]template')
-    FV_OK_MESSAGE = re.compile(r'[@+:]ok_message')
-    FV_ERROR_MESSAGE = re.compile(r'[@+:]error_message')
-
-    # specials for parsePropsFromForm
-    FV_REQUIRED = re.compile(r'[@+:]required')
-    FV_ADD = re.compile(r'([@+:])add\1')
-    FV_REMOVE = re.compile(r'([@+:])remove\1')
-    FV_CONFIRM = re.compile(r'.+[@+:]confirm')
-    FV_LINK = re.compile(r'([@+:])link\1(.+)')
-
-    # deprecated
-    FV_NOTE = re.compile(r'[@+:]note')
-    FV_FILE = re.compile(r'[@+:]file')
+    FV_TEMPLATE = re.compile(r'[@:]template')
+    FV_OK_MESSAGE = re.compile(r'[@:]ok_message')
+    FV_ERROR_MESSAGE = re.compile(r'[@:]error_message')
+
+    FV_QUERYNAME = re.compile(r'[@:]queryname')
+
+    # edit form variable handling (see unit tests)
+    FV_LABELS = r'''
+       ^(
+         (?P<note>[@:]note)|
+         (?P<file>[@:]file)|
+         (
+          ((?P<classname>%s)(?P<id>[-\d]+))?  # optional leading designator
+          ((?P<required>[@:]required$)|       # :required
+           (
+            (
+             (?P<add>[@:]add[@:])|            # :add:<prop>
+             (?P<remove>[@:]remove[@:])|      # :remove:<prop>
+             (?P<confirm>[@:]confirm[@:])|    # :confirm:<prop>
+             (?P<link>[@:]link[@:])|          # :link:<prop>
+             ([@:])                           # just a separator
+            )?
+            (?P<propname>[^@:]+)             # <prop>
+           )
+          )
+         )
+        )$'''
 
     # Note: index page stuff doesn't appear here:
     # columns, sort, sortdir, filter, group, groupdir, search_text,
@@ -215,10 +244,15 @@ class Client:
         except SendFile, designator:
             self.serve_file(designator)
         except SendStaticFile, file:
-            self.serve_static_file(str(file))
+            try:
+                self.serve_static_file(str(file))
+            except NotModified:
+                # send the 304 response
+                self.request.send_response(304)
+                self.request.end_headers()
         except Unauthorised, message:
-            self.classname=None
-            self.template=''
+            self.classname = None
+            self.template = ''
             self.error_message.append(message)
             self.write(self.renderContext())
         except NotFound:
@@ -229,8 +263,13 @@ class Client:
             self.write(cgitb.html())
 
     def clean_sessions(self):
-        '''age sessions, remove when they haven't been used for a week.
-        Do it only once an hour'''
+        ''' 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.
+        '''
         sessions = self.db.sessions
         last_clean = sessions.get('last_clean', 'last_use') or 0
 
@@ -238,11 +277,17 @@ class Client:
         hour = 60*60
         now = time.time()
         if now - last_clean > hour:
-            # remove age sessions
+            # remove aged sessions
             for sessid in sessions.list():
                 interval = now - sessions.get(sessid, 'last_use')
                 if interval > week:
                     sessions.destroy(sessid)
+            # remove aged otks
+            otks = self.db.otks
+            for sessid in otks.list():
+                interval = now - otks.get(sessid, '__time')
+                if interval > week:
+                    otks.destroy(sessid)
             sessions.set('last_clean', last_use=time.time())
 
     def determine_user(self):
@@ -256,7 +301,7 @@ class Client:
         sessions = self.db.sessions
 
         # look up the user session cookie
-        cookie = Cookie.Cookie(self.env.get('HTTP_COOKIE', ''))
+        cookie = Cookie.SimpleCookie(self.env.get('HTTP_COOKIE', ''))
         user = 'anonymous'
 
         # bump the "revision" of the cookie since the format changed
@@ -402,11 +447,28 @@ class Client:
         self.write(file.get(nodeid, 'content'))
 
     def serve_static_file(self, file):
+        ims = None
+        # see if there's an if-modified-since...
+        if hasattr(self.request, 'headers'):
+            ims = self.request.headers.getheader('if-modified-since')
+        elif self.env.has_key('HTTP_IF_MODIFIED_SINCE'):
+            # cgi will put the header in the env var
+            ims = self.env['HTTP_IF_MODIFIED_SINCE']
+        filename = os.path.join(self.instance.config.TEMPLATES, file)
+        lmt = os.stat(filename)[stat.ST_MTIME]
+        if ims:
+            ims = rfc822.parsedate(ims)[:6]
+            lmtt = time.gmtime(lmt)[:6]
+            if lmtt <= ims:
+                raise NotModified
+
         # we just want to serve up the file named
         mt = mimetypes.guess_type(str(file))[0]
+        if not mt:
+            mt = 'text/plain'
         self.additional_headers['Content-Type'] = mt
-        self.write(open(os.path.join(self.instance.config.TEMPLATES,
-            file)).read())
+        self.additional_headers['Last-Modifed'] = rfc822.formatdate(lmt)
+        self.write(open(filename, 'rb').read())
 
     def renderContext(self):
         ''' Return a PageTemplate for the named page
@@ -432,9 +494,11 @@ class Client:
     # these are the actions that are available
     actions = (
         ('edit',     'editItemAction'),
-        ('editCSV',  'editCSVAction'),
+        ('editcsv',  'editCSVAction'),
         ('new',      'newItemAction'),
         ('register', 'registerAction'),
+        ('confrego', 'confRegoAction'),
+        ('passrst',  'passResetAction'),
         ('login',    'loginAction'),
         ('logout',   'logout_action'),
         ('search',   'searchAction'),
@@ -442,41 +506,31 @@ class Client:
         ('show',     'showAction'),
     )
     def handle_action(self):
-        ''' Determine whether there should be an _action called.
+        ''' Determine whether there should be an Action called.
 
             The action is defined by the form variable :action which
-            identifies the method on this object to call. The four basic
-            actions are defined in the "actions" sequence on this class:
-             "edit"      -> self.editItemAction
-             "new"       -> self.newItemAction
-             "register"  -> self.registerAction
-             "login"     -> self.loginAction
-             "logout"    -> self.logout_action
-             "search"    -> self.searchAction
-             "retire"    -> self.retireAction
+            identifies the method on this object to call. The actions
+            are defined in the "actions" sequence on this class.
         '''
-        if not self.form.has_key(':action'):
+        if self.form.has_key(':action'):
+            action = self.form[':action'].value.lower()
+        elif self.form.has_key('@action'):
+            action = self.form['@action'].value.lower()
+        else:
             return None
         try:
             # get the action, validate it
-            action = self.form[':action'].value
             for name, method in self.actions:
                 if name == action:
                     break
             else:
                 raise ValueError, 'No such action "%s"'%action
-
             # call the mapped action
             getattr(self, method)()
         except Redirect:
             raise
         except Unauthorised:
             raise
-        except:
-            self.db.rollback()
-            s = StringIO.StringIO()
-            traceback.print_exc(None, s)
-            self.error_message.append('<pre>%s</pre>'%cgi.escape(s.getvalue()))
 
     def write(self, content):
         if not self.headers_done:
@@ -632,18 +686,16 @@ class Client:
         # Let the user know what's going on
         self.ok_message.append(_('You are logged out'))
 
+    chars = string.letters+string.digits
     def registerAction(self):
         '''Attempt to create a new user based on the contents of the form
         and then set the cookie.
 
         return 1 on successful login
         '''
-        # create the new user
-        cl = self.db.user
-
         # parse the props from the form
         try:
-            props = self.parsePropsFromForm()
+            props = self.parsePropsFromForm()[0][('user', None)]
         except (ValueError, KeyError), message:
             self.error_message.append(_('Error: ') + str(message))
             return
@@ -652,18 +704,138 @@ class Client:
         if not self.registerPermission(props):
             raise Unauthorised, _("You do not have permission to register")
 
+        try:
+            self.db.user.lookup(props['username'])
+            self.error_message.append('Error: A user with the username "%s" '
+                'already exists'%props['username'])
+            return
+        except KeyError:
+            pass
+
+        # generate the one-time-key and store the props for later
+        otk = ''.join([random.choice(self.chars) for x in range(32)])
+        for propname, proptype in self.db.user.getprops().items():
+            value = props.get(propname, None)
+            if value is None:
+                pass
+            elif isinstance(proptype, hyperdb.Date):
+                props[propname] = str(value)
+            elif isinstance(proptype, hyperdb.Interval):
+                props[propname] = str(value)
+            elif isinstance(proptype, hyperdb.Password):
+                props[propname] = str(value)
+        props['__time'] = time.time()
+        self.db.otks.set(otk, **props)
+
+        # send the email
+        tracker_name = self.db.config.TRACKER_NAME
+        subject = 'Complete your registration to %s'%tracker_name
+        body = '''
+To complete your registration of the user "%(name)s" with %(tracker)s,
+please visit the following URL:
+
+   %(url)s?@action=confrego&otk=%(otk)s
+'''%{'name': props['username'], 'tracker': tracker_name, 'url': self.base,
+                'otk': otk}
+        if not self.sendEmail(props['address'], subject, body):
+            return
+
+        # commit changes to the database
+        self.db.commit()
+
+        # redirect to the "you're almost there" page
+        raise Redirect, '%suser?@template=rego_progress'%self.base
+
+    def sendEmail(self, to, subject, content):
+        # send email to the user's email address
+        message = StringIO.StringIO()
+        writer = MimeWriter.MimeWriter(message)
+        tracker_name = self.db.config.TRACKER_NAME
+        writer.addheader('Subject', encode_header(subject))
+        writer.addheader('To', to)
+        writer.addheader('From', roundupdb.straddr((tracker_name,
+            self.db.config.ADMIN_EMAIL)))
+        writer.addheader('Date', time.strftime("%a, %d %b %Y %H:%M:%S +0000",
+            time.gmtime()))
+        # add a uniquely Roundup header to help filtering
+        writer.addheader('X-Roundup-Name', tracker_name)
+        # avoid email loops
+        writer.addheader('X-Roundup-Loop', 'hello')
+        writer.addheader('Content-Transfer-Encoding', 'quoted-printable')
+        body = writer.startbody('text/plain; charset=utf-8')
+
+        # message body, encoded quoted-printable
+        content = StringIO.StringIO(content)
+        quopri.encode(content, body, 0)
+
+        if SENDMAILDEBUG:
+            # don't send - just write to a file
+            open(SENDMAILDEBUG, 'a').write('FROM: %s\nTO: %s\n%s\n'%(
+                self.db.config.ADMIN_EMAIL,
+                ', '.join(to),message.getvalue()))
+        else:
+            # now try to send the message
+            try:
+                # send the message as admin so bounces are sent there
+                # instead of to roundup
+                smtp = smtplib.SMTP(self.db.config.MAILHOST)
+                smtp.sendmail(self.db.config.ADMIN_EMAIL, [to],
+                    message.getvalue())
+            except socket.error, value:
+                self.error_message.append("Error: couldn't send email: "
+                    "mailhost %s"%value)
+                return 0
+            except smtplib.SMTPException, msg:
+                self.error_message.append("Error: couldn't send email: %s"%msg)
+            return 0
+        return 1
+
+    def registerPermission(self, props):
+        ''' Determine whether the user has permission to register
+
+            Base behaviour is to check the user has "Web Registration".
+        '''
+        # registration isn't allowed to supply roles
+        if props.has_key('roles'):
+            return 0
+        if self.db.security.hasPermission('Web Registration', self.userid):
+            return 1
+        return 0
+
+    def confRegoAction(self):
+        ''' Grab the OTK, use it to load up the new user details
+        '''
+        # pull the rego information out of the otk database
+        otk = self.form['otk'].value
+        props = self.db.otks.getall(otk)
+        for propname, proptype in self.db.user.getprops().items():
+            value = props.get(propname, None)
+            if value is None:
+                pass
+            elif isinstance(proptype, hyperdb.Date):
+                props[propname] = date.Date(value)
+            elif isinstance(proptype, hyperdb.Interval):
+                props[propname] = date.Interval(value)
+            elif isinstance(proptype, hyperdb.Password):
+                props[propname] = password.Password()
+                props[propname].unpack(value)
+
         # re-open the database as "admin"
         if self.user != 'admin':
             self.opendb('admin')
-            
+
         # create the new user
         cl = self.db.user
+# XXX we need to make the "default" page be able to display errors!
         try:
             props['roles'] = self.instance.config.NEW_WEB_USER_ROLES
-            self.userid = cl.create(**props['user'])
+            del props['__time']
+            self.userid = cl.create(**props)
+            # clear the props from the otk database
+            self.db.otks.destroy(otk)
             self.db.commit()
         except (ValueError, KeyError), message:
-            self.error_message.append(message)
+            self.error_message.append(str(message))
             return
 
         # log the new user in
@@ -682,21 +854,99 @@ class Client:
         # nice message
         message = _('You are now registered, welcome!')
 
-        # redirect to the item's edit page
-        raise Redirect, '%s%s%s?+ok_message=%s'%(
-            self.base, self.classname, self.userid,  urllib.quote(message))
+        # redirect to the user's page
+        raise Redirect, '%suser%s?@ok_message=%s&@template=%s'%(self.base,
+            self.userid, urllib.quote(message), urllib.quote(self.template))
 
-    def registerPermission(self, props):
-        ''' Determine whether the user has permission to register
+    def passResetAction(self):
+        ''' Handle password reset requests.
 
-            Base behaviour is to check the user has "Web Registration".
+            Presence of either "name" or "address" generate email.
+            Presense of "otk" performs the reset.
         '''
-        # registration isn't allowed to supply roles
-        if props.has_key('roles'):
-            return 0
-        if self.db.security.hasPermission('Web Registration', self.userid):
-            return 1
-        return 0
+        if self.form.has_key('otk'):
+            # pull the rego information out of the otk database
+            otk = self.form['otk'].value
+            uid = self.db.otks.get(otk, 'uid')
+
+            # re-open the database as "admin"
+            if self.user != 'admin':
+                self.opendb('admin')
+
+            # change the password
+            newpw = ''.join([random.choice(self.chars) for x in range(8)])
+
+            cl = self.db.user
+# XXX we need to make the "default" page be able to display errors!
+            try:
+                # set the password
+                cl.set(uid, password=password.Password(newpw))
+                # clear the props from the otk database
+                self.db.otks.destroy(otk)
+                self.db.commit()
+            except (ValueError, KeyError), message:
+                self.error_message.append(str(message))
+                return
+
+            # user info
+            address = self.db.user.get(uid, 'address')
+            name = self.db.user.get(uid, 'username')
+
+            # send the email
+            tracker_name = self.db.config.TRACKER_NAME
+            subject = 'Password reset for %s'%tracker_name
+            body = '''
+The password has been reset for username "%(name)s".
+
+Your password is now: %(password)s
+'''%{'name': name, 'password': newpw}
+            if not self.sendEmail(address, subject, body):
+                return
+
+            self.ok_message.append('Password reset and email sent to %s'%address)
+            return
+
+        # no OTK, so now figure the user
+        if self.form.has_key('username'):
+            name = self.form['username'].value
+            try:
+                uid = self.db.user.lookup(name)
+            except KeyError:
+                self.error_message.append('Unknown username')
+                return
+            address = self.db.user.get(uid, 'address')
+        elif self.form.has_key('address'):
+            address = self.form['address'].value
+            uid = uidFromAddress(self.db, ('', address), create=0)
+            if not uid:
+                self.error_message.append('Unknown email address')
+                return
+            name = self.db.user.get(uid, 'username')
+        else:
+            self.error_message.append('You need to specify a username '
+                'or address')
+            return
+
+        # generate the one-time-key and store the props for later
+        otk = ''.join([random.choice(self.chars) for x in range(32)])
+        self.db.otks.set(otk, uid=uid, __time=time.time())
+
+        # send the email
+        tracker_name = self.db.config.TRACKER_NAME
+        subject = 'Confirm reset of password for %s'%tracker_name
+        body = '''
+Someone, perhaps you, has requested that the password be changed for your
+username, "%(name)s". If you wish to proceed with the change, please follow
+the link below:
+
+  %(url)suser?@template=forgotten&@action=passrst&otk=%(otk)s
+
+You should then receive another email with the new password.
+'''%{'name': name, 'tracker': tracker_name, 'url': self.base, 'otk': otk}
+        if not self.sendEmail(address, subject, body):
+            return
+
+        self.ok_message.append('Email sent to %s'%address)
 
     def editItemAction(self):
         ''' Perform an edit of an item in the database.
@@ -721,8 +971,9 @@ class Client:
         self.db.commit()
 
         # redirect to the item's edit page
-        raise Redirect, '%s%s%s?+ok_message=%s'%(self.base, self.classname,
-            self.nodeid,  urllib.quote(message))
+        raise Redirect, '%s%s%s?@ok_message=%s&@template=%s'%(self.base,
+            self.classname, self.nodeid, urllib.quote(message),
+            urllib.quote(self.template))
 
     def editItemPermission(self, props):
         ''' Determine whether the user has permission to edit this item.
@@ -755,38 +1006,29 @@ class Client:
             special form values.
         '''
         # parse the props from the form
-#        try:
-        if 1:
+        try:
             props, links = self.parsePropsFromForm()
-#        except (ValueError, KeyError), message:
-#            self.error_message.append(_('Error: ') + str(message))
-#            return
+        except (ValueError, KeyError), message:
+            self.error_message.append(_('Error: ') + str(message))
+            return
 
         # handle the props - edit or create
-#        try:
-        if 1:
-            # create the context here
-            cn = self.classname
-            nid = self._createnode(cn, props[(cn, None)])
-            del props[(cn, None)]
-
-            extra = self._editnodes(props, links, {(cn, None): nid})
-            if extra:
-                extra = '<br>' + extra
-
-            # now do the rest
-            messages = '%s %s created'%(cn, nid) + extra
-#        except (ValueError, KeyError, IndexError), message:
-#            # these errors might just be indicative of user dumbness
-#            self.error_message.append(_('Error: ') + str(message))
-#            return
+        try:
+            # when it hits the None element, it'll set self.nodeid
+            messages = self._editnodes(props, links)
+
+        except (ValueError, KeyError, IndexError), message:
+            # these errors might just be indicative of user dumbness
+            self.error_message.append(_('Error: ') + str(message))
+            return
 
         # commit now that all the tricky stuff is done
         self.db.commit()
 
         # redirect to the new item's page
-        raise Redirect, '%s%s%s?:ok_message=%s'%(self.base, self.classname,
-            nid, urllib.quote(messages))
+        raise Redirect, '%s%s%s?@ok_message=%s&@template=%s'%(self.base,
+            self.classname, self.nodeid, urllib.quote(messages),
+            urllib.quote(self.template))
 
     def newItemPermission(self, props):
         ''' Determine whether the user has permission to create (edit) this
@@ -804,6 +1046,128 @@ class Client:
             return 1
         return 0
 
+
+    #
+    #  Utility methods for editing
+    #
+    def _editnodes(self, all_props, all_links, newids=None):
+        ''' Use the props in all_props to perform edit and creation, then
+            use the link specs in all_links to do linking.
+        '''
+        # figure dependencies and re-work links
+        deps = {}
+        links = {}
+        for cn, nodeid, propname, vlist in all_links:
+            if not all_props.has_key((cn, nodeid)):
+                # link item to link to doesn't (and won't) exist
+                continue
+            for value in vlist:
+                if not all_props.has_key(value):
+                    # link item to link to doesn't (and won't) exist
+                    continue
+                deps.setdefault((cn, nodeid), []).append(value)
+                links.setdefault(value, []).append((cn, nodeid, propname))
+
+        # figure chained dependencies ordering
+        order = []
+        done = {}
+        # loop detection
+        change = 0
+        while len(all_props) != len(done):
+            for needed in all_props.keys():
+                if done.has_key(needed):
+                    continue
+                tlist = deps.get(needed, [])
+                for target in tlist:
+                    if not done.has_key(target):
+                        break
+                else:
+                    done[needed] = 1
+                    order.append(needed)
+                    change = 1
+            if not change:
+                raise ValueError, 'linking must not loop!'
+
+        # now, edit / create
+        m = []
+        for needed in order:
+            props = all_props[needed]
+            if not props:
+                # nothing to do
+                continue
+            cn, nodeid = needed
+
+            if nodeid is not None and int(nodeid) > 0:
+                # make changes to the node
+                props = self._changenode(cn, nodeid, props)
+
+                # and some nice feedback for the user
+                if props:
+                    info = ', '.join(props.keys())
+                    m.append('%s %s %s edited ok'%(cn, nodeid, info))
+                else:
+                    m.append('%s %s - nothing changed'%(cn, nodeid))
+            else:
+                assert props
+
+                # make a new node
+                newid = self._createnode(cn, props)
+                if nodeid is None:
+                    self.nodeid = newid
+                nodeid = newid
+
+                # and some nice feedback for the user
+                m.append('%s %s created'%(cn, newid))
+
+            # fill in new ids in links
+            if links.has_key(needed):
+                for linkcn, linkid, linkprop in links[needed]:
+                    props = all_props[(linkcn, linkid)]
+                    cl = self.db.classes[linkcn]
+                    propdef = cl.getprops()[linkprop]
+                    if not props.has_key(linkprop):
+                        if linkid is None or linkid.startswith('-'):
+                            # linking to a new item
+                            if isinstance(propdef, hyperdb.Multilink):
+                                props[linkprop] = [newid]
+                            else:
+                                props[linkprop] = newid
+                        else:
+                            # linking to an existing item
+                            if isinstance(propdef, hyperdb.Multilink):
+                                existing = cl.get(linkid, linkprop)[:]
+                                existing.append(nodeid)
+                                props[linkprop] = existing
+                            else:
+                                props[linkprop] = newid
+
+        return '<br>'.join(m)
+
+    def _changenode(self, cn, nodeid, props):
+        ''' change the node based on the contents of the form
+        '''
+        # check for permission
+        if not self.editItemPermission(props):
+            raise Unauthorised, 'You do not have permission to edit %s'%cn
+
+        # make the changes
+        cl = self.db.classes[cn]
+        return cl.set(nodeid, **props)
+
+    def _createnode(self, cn, props):
+        ''' create a node based on the contents of the form
+        '''
+        # check for permission
+        if not self.newItemPermission(props):
+            raise Unauthorised, 'You do not have permission to create %s'%cn
+
+        # create the node and return its id
+        cl = self.db.classes[cn]
+        return cl.create(**props)
+
+    # 
+    # More actions
+    #
     def editCSVAction(self):
         ''' Performs an edit of all of a class' items in one go.
 
@@ -849,6 +1213,12 @@ class Client:
             nodeid, values = values[0], values[1:]
             found[nodeid] = 1
 
+            # see if the node exists
+            if cl.hasnode(nodeid):
+                exists = 1
+            else:
+                exists = 0
+
             # confirm correct weight
             if len(idlessprops) != len(values):
                 self.error_message.append(
@@ -858,16 +1228,23 @@ class Client:
             # extract the new values
             d = {}
             for name, value in zip(idlessprops, values):
+                prop = cl.properties[name]
                 value = value.strip()
                 # only add the property if it has a value
                 if value:
                     # if it's a multilink, split it
-                    if isinstance(cl.properties[name], hyperdb.Multilink):
+                    if isinstance(prop, hyperdb.Multilink):
                         value = value.split(':')
                     d[name] = value
+                elif exists:
+                    # nuke the existing value
+                    if isinstance(prop, hyperdb.Multilink):
+                        d[name] = []
+                    else:
+                        d[name] = None
 
             # perform the edit
-            if cl.hasnode(nodeid):
+            if exists:
                 # edit existing
                 cl.set(nodeid, **d)
             else:
@@ -911,8 +1288,15 @@ class Client:
 
         # add a faked :filter form variable for each filtering prop
         props = self.db.classes[self.classname].getprops()
+        queryname = ''
         for key in self.form.keys():
-            if not props.has_key(key): continue
+            # special vars
+            if self.FV_QUERYNAME.match(key):
+                queryname = self.form[key].value.strip()
+                continue
+
+            if not props.has_key(key):
+                continue
             if isinstance(self.form[key], type([])):
                 # search for at least one entry which is not empty
                 for minifield in self.form[key]:
@@ -922,32 +1306,30 @@ class Client:
                     continue
             else:
                 if not self.form[key].value: continue
-            self.form.value.append(cgi.MiniFieldStorage(':filter', key))
+            self.form.value.append(cgi.MiniFieldStorage('@filter', key))
 
         # handle saving the query params
-        if self.form.has_key(':queryname'):
-            queryname = self.form[':queryname'].value.strip()
-            if queryname:
-                # parse the environment and figure what the query _is_
-                req = HTMLRequest(self)
-                url = req.indexargs_href('', {})
-
-                # handle editing an existing query
-                try:
-                    qid = self.db.query.lookup(queryname)
-                    self.db.query.set(qid, klass=self.classname, url=url)
-                except KeyError:
-                    # create a query
-                    qid = self.db.query.create(name=queryname,
-                        klass=self.classname, url=url)
+        if queryname:
+            # parse the environment and figure what the query _is_
+            req = HTMLRequest(self)
+            url = req.indexargs_href('', {})
+
+            # handle editing an existing query
+            try:
+                qid = self.db.query.lookup(queryname)
+                self.db.query.set(qid, klass=self.classname, url=url)
+            except KeyError:
+                # create a query
+                qid = self.db.query.create(name=queryname,
+                    klass=self.classname, url=url)
 
-                    # and add it to the user's query multilink
-                    queries = self.db.user.get(self.userid, 'queries')
-                    queries.append(qid)
-                    self.db.user.set(self.userid, queries=queries)
+                # and add it to the user's query multilink
+                queries = self.db.user.get(self.userid, 'queries')
+                queries.append(qid)
+                self.db.user.set(self.userid, queries=queries)
 
-                # commit the query change to the database
-                self.db.commit()
+            # commit the query change to the database
+            self.db.commit()
 
     def searchPermission(self):
         ''' Determine whether the user has permission to search this class.
@@ -1001,132 +1383,60 @@ class Client:
         return 1
 
 
-    def showAction(self):
-        ''' Show a node
+    def showAction(self, typere=re.compile('[@:]type'),
+            numre=re.compile('[@:]number')):
+        ''' Show a node of a particular class/id
         '''
-        t = self.form[':type'].value
-        n = self.form[':number'].value
+        t = n = ''
+        for key in self.form.keys():
+            if typere.match(key):
+                t = self.form[key].value.strip()
+            elif numre.match(key):
+                n = self.form[key].value.strip()
+        if not t:
+            raise ValueError, 'Invalid %s number'%t
         url = '%s%s%s'%(self.db.config.TRACKER_WEB, t, n)
         raise Redirect, url
 
-
-    #
-    #  Utility methods for editing
-    #
-    def _editnodes(self, all_props, all_links, newids=None):
-        ''' Use the props in all_props to perform edit and creation, then
-            use the link specs in all_links to do linking.
-        '''
-        m = []
-        if newids is None:
-            newids = {}
-        for (cn, nodeid), props in all_props.items():
-            if int(nodeid) > 0:
-                # make changes to the node
-                props = self._changenode(cn, nodeid, props)
-
-                # and some nice feedback for the user
-                if props:
-                    info = ', '.join(props.keys())
-                    m.append('%s %s %s edited ok'%(cn, nodeid, info))
-                else:
-                    m.append('%s %s - nothing changed'%(cn, nodeid))
-            elif props:
-                # make a new node
-                newid = self._createnode(cn, props)
-                newids[(cn, nodeid)] = newid
-                nodeid = newid
-
-                # and some nice feedback for the user
-                m.append('%s %s created'%(cn, newid))
-
-        # handle linked nodes
-        keys = self.form.keys()
-        for cn, nodeid, propname, value in all_links:
-            cl = self.db.classes[cn]
-            property = cl.getprops()[propname]
-            if nodeid is None or nodeid.startswith('-'):
-                if not newids.has_key((cn, nodeid)):
-                    continue
-                nodeid = newids[(cn, nodeid)]
-
-            # map the desired classnames to their actual created ids
-            for link in value:
-                if not newids.has_key(link):
-                    continue
-                linkid = newids[link]
-                if isinstance(property, hyperdb.Multilink):
-                    # take a dupe of the list so we're not changing the cache
-                    existing = cl.get(nodeid, propname)[:]
-                    existing.append(linkid)
-                    cl.set(nodeid, **{propname: existing})
-                elif isinstance(property, hyperdb.Link):
-                    # make the Link set
-                    cl.set(nodeid, **{propname: linkid})
-                else:
-                    raise ValueError, '%s %s is not a link or multilink '\
-                        'property'%(cn, propname)
-                m.append('%s %s linked to <a href="%s%s">%s %s</a>'%(
-                    link[0], linkid, cn, nodeid, cn, nodeid))
-
-        return '<br>'.join(m)
-
-    def _changenode(self, cn, nodeid, props):
-        ''' change the node based on the contents of the form
-        '''
-        # check for permission
-        if not self.editItemPermission(props):
-            raise PermissionError, 'You do not have permission to edit %s'%cn
-
-        # make the changes
-        cl = self.db.classes[cn]
-        return cl.set(nodeid, **props)
-
-    def _createnode(self, cn, props):
-        ''' create a node based on the contents of the form
-        '''
-        # check for permission
-        if not self.newItemPermission(props):
-            raise PermissionError, 'You do not have permission to create %s'%cn
-
-        # create the node and return its id
-        cl = self.db.classes[cn]
-        return cl.create(**props)
-
     def parsePropsFromForm(self, num_re=re.compile('^\d+$')):
-        ''' Pull properties for the given class out of the form.
+        ''' Pull properties out of the form.
 
             In the following, <bracketed> values are variable, ":" may be
-            any of : @ + and other text "required" is fixed.
+            one of ":" or "@", and other text "required" is fixed.
+
+            Properties are specified as form variables:
+
+             <propname>
+              - property on the current context item
 
-            Properties are specified as form variables
              <designator>:<propname>
+              - property on the indicated item
+
+             <classname>-<N>:<propname>
+              - property on the Nth new item of classname
 
-            where the propery belongs to the context class or item if the
-            designator is not specified. The designator may specify a
-            negative item id value (ie. "issue-1") and a new item of the
-            specified class will be created for each negative id found.
+            Once we have determined the "propname", we check to see if it
+            is one of the special form values:
 
-            If a "<designator>:required" parameter is supplied,
-            then the named property values must be supplied or a
-            ValueError will be raised.
+             :required
+              The named property values must be supplied or a ValueError
+              will be raised.
 
-            Other special form values:
-             [classname|designator]:remove:<propname>=id(s)
+             :remove:<propname>=id(s)
               The ids will be removed from the multilink property.
-             [classname|designator]:add:<propname>=id(s)
+
+             :add:<propname>=id(s)
               The ids will be added to the multilink property.
 
-             [classname|designator]:link:<propname>=<classname>
+             :link:<propname>=<designator>
               Used to add a link to new items created during edit.
               These are collected up and returned in all_links. This will
               result in an additional linking operation (either Link set or
               Multilink append) after the edit/create is done using
-              all_props in _editnodes. The <propname> on
-              [classname|designator] will be set/appended the id of the
-              newly created item of class <classname>.
-
-            Note: the colon may be one of:  : @ +
+              all_props in _editnodes. The <propname> on the current item
+              will be set/appended the id of the newly created item of
+              class <designator> (where <designator> must be
+              <classname>-<N>).
 
             Any of the form variables may be prefixed with a classname or
             designator.
@@ -1142,16 +1452,29 @@ class Client:
             If a String property's form value is a file upload, then we
             try to set additional properties "filename" and "type" (if
             they are valid for the class).
+
+            Two special form values are supported for backwards
+            compatibility:
+             :note - create a message (with content, author and date), link
+                     to the context item. This is ALWAYS desginated "msg-1".
+             :file - create a file, attach to the current item and any
+                     message created by :note. This is ALWAYS designated
+                     "file-1".
+
+            We also check that FileClass items have a "content" property with
+            actual content, otherwise we remove them from all_props before
+            returning.
         '''
         # some very useful variables
         db = self.db
         form = self.form
 
-        if not hasattr(self, 'FV_ITEMSPEC'):
-            # generate the regexp for detecting
-            # <classname|designator>[@:+]property
+        if not hasattr(self, 'FV_SPECIAL'):
+            # generate the regexp for handling special form values
             classes = '|'.join(db.classes.keys())
-            self.FV_ITEMSPEC = re.compile(r'(%s)([-\d]+)[@+:](.+)$'%classes)
+            # specials for parsePropsFromForm
+            # handle the various forms (see unit tests)
+            self.FV_SPECIAL = re.compile(self.FV_LABELS%classes, re.VERBOSE)
             self.FV_DESIGNATOR = re.compile(r'(%s)([-\d]+)'%classes)
 
         # these indicate the default class / item
@@ -1171,54 +1494,52 @@ class Client:
         keys = form.keys()
         timezone = db.getUserTimezone()
 
+        # sentinels for the :note and :file props
+        have_note = have_file = 0
+
+        # extract the usable form labels from the form
+        matches = []
         for key in keys:
-            # see if this value modifies a different item to the default
-            m = self.FV_ITEMSPEC.match(key)
+            m = self.FV_SPECIAL.match(key)
             if m:
+                matches.append((key, m.groupdict()))
+
+        # now handle the matches
+        for key, d in matches:
+            if d['classname']:
                 # we got a designator
-                cn = m.group(1)
+                cn = d['classname']
                 cl = self.db.classes[cn]
-                nodeid = m.group(2)
-                propname = m.group(3)
-            elif key == ':note':
-                # backwards compatibility: the special note field
+                nodeid = d['id']
+                propname = d['propname']
+            elif d['note']:
+                # the special note field
                 cn = 'msg'
                 cl = self.db.classes[cn]
                 nodeid = '-1'
                 propname = 'content'
                 all_links.append((default_cn, default_nodeid, 'messages',
                     [('msg', '-1')]))
-            elif key == ':file':
-                # backwards compatibility: the special file field
+                have_note = 1
+            elif d['file']:
+                # the special file field
                 cn = 'file'
                 cl = self.db.classes[cn]
                 nodeid = '-1'
                 propname = 'content'
                 all_links.append((default_cn, default_nodeid, 'files',
                     [('file', '-1')]))
-                if self.form.has_key(':note'):
-                    all_links.append(('msg', '-1', 'files', [('file', '-1')]))
+                have_file = 1
             else:
                 # default
                 cn = default_cn
                 cl = default_cl
                 nodeid = default_nodeid
-                propname = key
+                propname = d['propname']
 
             # the thing this value relates to is...
             this = (cn, nodeid)
 
-            # is this a link command?
-            if self.FV_LINK.match(propname):
-                value = []
-                for entry in extractFormList(form[key]):
-                    m = self.FV_DESIGNATOR.match(entry)
-                    if not m:
-                        raise ValueError, \
-                            'link "%s" value "%s" not a designator'%(key, entry)
-                    value.append((m.groups(1), m.groups(2)))
-                all_links.append((cn, nodeid, propname[6:], value))
-
             # get more info about the class, and the current set of
             # form props for it
             if not all_propdef.has_key(cn):
@@ -1228,8 +1549,27 @@ class Client:
                 all_props[this] = {}
             props = all_props[this]
 
+            # is this a link command?
+            if d['link']:
+                value = []
+                for entry in extractFormList(form[key]):
+                    m = self.FV_DESIGNATOR.match(entry)
+                    if not m:
+                        raise ValueError, \
+                            'link "%s" value "%s" not a designator'%(key, entry)
+                    value.append((m.group(1), m.group(2)))
+
+                # make sure the link property is valid
+                if (not isinstance(propdef[propname], hyperdb.Multilink) and
+                        not isinstance(propdef[propname], hyperdb.Link)):
+                    raise ValueError, '%s %s is not a link or '\
+                        'multilink property'%(cn, propname)
+
+                all_links.append((cn, nodeid, propname, value))
+                continue
+
             # detect the special ":required" variable
-            if self.FV_REQUIRED.match(key):
+            if d['required']:
                 all_required[this] = extractFormList(form[key])
                 continue
 
@@ -1240,11 +1580,9 @@ class Client:
 
             # see if we're performing a special multilink action
             mlaction = 'set'
-            if self.FV_REMOVE.match(propname):
-                propname = propname[8:]
+            if d['remove']:
                 mlaction = 'remove'
-            elif self.FV_ADD.match(propname):
-                propname = propname[5:]
+            elif d['add']:
                 mlaction = 'add'
 
             # does the property exist?
@@ -1253,6 +1591,8 @@ class Client:
                     raise ValueError, 'You have submitted a %s action for'\
                         ' the property "%s" which doesn\'t exist'%(mlaction,
                         propname)
+                # the form element is probably just something we don't care
+                # about - ignore it
                 continue
             proptype = propdef[propname]
 
@@ -1275,7 +1615,7 @@ class Client:
 
             # now that we have the props field, we need a teensy little
             # extra bit of help for the old :note field...
-            if key == ':note' and value:
+            if d['note'] and value:
                 props['author'] = self.db.getuid()
                 props['date'] = date.Date()
 
@@ -1284,8 +1624,8 @@ class Client:
                 if not value:
                     # ignore empty password values
                     continue
-                for key in keys:
-                    if self.FV_CONFIRM.match(key):
+                for key, d in matches:
+                    if d['confirm'] and d['propname'] == propname:
                         confirm = form[key]
                         break
                 else:
@@ -1456,6 +1796,10 @@ class Client:
             if propname in required and value is not None:
                 required.remove(propname)
 
+        # check to see if we need to specially link a file to the note
+        if have_note and have_file:
+            all_links.append(('msg', '-1', 'files', [('file', '-1')]))
+
         # see if all the required properties have been supplied
         s = []
         for thing, required in all_required.items():
@@ -1470,6 +1814,15 @@ class Client:
         if s:
             raise ValueError, '\n'.join(s)
 
+        # check that FileClass entries have a "content" property with
+        # content, otherwise remove them
+        for (cn, id), props in all_props.items():
+            cl = self.db.classes[cn]
+            if not isinstance(cl, hyperdb.FileClass):
+                continue
+            # we also don't want to create FileClass items with no content
+            if not props.get('content', ''):
+                del all_props[(cn, id)]
         return all_props, all_links
 
 def fixNewlines(text):