Merge lp:~ptressel/sahana-eden/devel into lp:sahana-eden

[Pat Tressel]

There are two example fields added to the hrm_human_resource list_fields, which makes those lines a bit long. So we might want to find someplace else to add fake virtual fields as an example.

Seems like having an official report_fields would be good -- people will likely want different (i.e. more) fields in reports than in list views.

I saw someone ask a question about naming conventions for globals -- is there an Official Change? Because there might be some violations of whatever the new rule is. There are several staticmethod functions in S3VirtualField that might be violations. And there's a method _get_field that could be shared -- I see the same sort of thing being done elsewhere. I gave get_list_fields a _ on front because it's really not appropriate for outside callers -- too non-general.

There are some ToDos. Main one is adding support for fake virtual fields in exporter.xls. If we add report_fields, and switch that over the exporter call to using report_fields, and don't put virtual fields in report_fields yet, we can kick the can down the road a bit.

[Pat Tressel (ptressel)]

Got questions? This doesn't really impact things that don't use the fake virtual fields, with one exception that's easy to fix, which is exporter.xls, to which support has not been added. If that is filtering fields that aren't in the table, then there's no effect. Otherwise, the quick way to avoid any interaction is to use report_fields for exporter.xls, and don't put any virtual fields in report_fields.

Here's one example use of a fake virtual field -- given population centers data, compute the population within a specified radius of some location. E.g. let the user pick the radius and set it in their session, then alter the gis_location list_fields to include that virtual field for a population centers query. This is (part of) a problem on the RHoK #3 list, proposed by Cat Graham, to go along with the previous population centers work.

[nursix (nursix.org)]

All in all a brave, but good solution.

Could you please remove/shorten the comments by what does not directly relate to the actual code, but is actually discussing theoretical backgrounds or design options (e.g. the introduction of the S3VirtualField class)?

These comments are very hard too parse, are focussed on working *on* (rather than *with*) the code, and all discussion should better happen on either ML or wiki (preferrably the latter if the design, like in this case, needs some deeper understanding of the background).

There is no issue whatsoever with the naming of the functions inside the S3VirtualField class, don't worry.

review: Needs Fixing

[nursix (nursix.org)]

1) I do not see a need to declare the virtual fields by name, they can actually be added directly (i.e. as S3VirtualField instances) to the list_fields array. This would also remove the need for the field lookup loop in _get_list_fields.

2) I totally agree that report types like XLS or PDF need a separate array, which can also include S3VirtualFields. Consequently, _get_list_fields should be made generic over any type of field lists, and probably be moved into S3Method (nothing you need to fix for merge, just to answer the question you brought up).

3) Authorization of access to referenced tables (in both fk$fn fields as well as virtual fields) is still missing. This is not quite a new issue, but with this implementation it becomes resolvable.

4) We are creating ambiguity here - "virtual fields" (=web2py) vs. "virtual fields" (=S3VirtualField). We should probably rename into something like "S3ComputedValue" or "S3DerivedValue" to prevent any confusion.

All in all this would give a clear declaration like:

    list_fields=["id",
                 S3ComputedValue(T("Email"), ...),
                 S3ComputedValue(T("Mobile Phone"), ...),
                 "type",
                 "job_title",
                 "status"]

Of course, you can also declare the computed values into variables and then just use the variables in the list_fields setting. Still, the virtual_fields declaration and the respective look-up can be avoided.

Can you fix it like that? Otherwise I'll do.

review: Needs Fixing

[Pat Tressel (ptressel)]

Dominic --
Sorry about the delay -- I'm just getting a new machine set up... I arrived
at RHoK Saturday morning, turned on my laptop, and...nothing. Not even the
boot loader screen. While they were having the meet'n'greet, and deciding
who was working on what project, I made a dash down to Fry's Electronics
(huge electronics store) and came back with another HP laptop. The old one
lasted ~6 years, and was gradually dying, so not failure was not
unexpected...just really really bad timing. So all I'm set up to do at the
moment is Android app development. Don't even have working security
software or backup yet. So, gimme a little while to either get my backup
running so I can recover files from backup, or get an enclosure to put my
old laptop's disk in... Fortunately, my laptop was on the night before it
failed, and I made no changes after backup ran, so (if either my backup or
the laptop's drive works) I didn't lose anything.

-- Pat

[nursix (nursix.org)]

review: Abstain

[nursix (nursix.org)]

If necessary, re-implement this in a more recent version of CRUD.

Basically that would mean to just extend S3CRUD.get_list_fields, whereas the extra field pre-loading is already implemented. The S3VirtualField class doesn't seem to be necessary anymore.

review: Disapprove

[Pat Tressel (ptressel)]

Dominic --

I assumed this was dead due to lack of interest -- seemed I was the only one
who had a use for arbitrary computations to be shown in lists, and I'm sure
the thing I wanted it for, which was something to do with regions /
incidents, is long obsolete.

-- Pat

On Fri, Sep 2, 2011 at 4:43 AM, nursix <email address hidden> wrote:

> Review: Disapprove
> If necessary, re-implement this in a more recent version of CRUD.
>
> Basically that would mean to just extend S3CRUD.get_list_fields, whereas
> the extra field pre-loading is already implemented. The S3VirtualField class
> doesn't seem to be necessary anymore.
>
> --
> https://code.launchpad.net/~ptressel/sahana-eden/devel/+merge/63361<https://code.launchpad.net/%7Eptressel/sahana-eden/devel/+merge/63361>
> You are the owner of lp:~ptressel/sahana-eden/devel.
>

Unmerged revisions

2266. By Pat Tressel on 2011-06-03

Merge from trunk.

2265. By Pat Tressel on 2011-06-03

Add fake virtual fields. For fk$fn for list_fields, allow records with empty fk if fk field isn't notnull.

=== modified file 'controllers/hrm.py'
--- controllers/hrm.py	2011-05-30 18:34:57 +0000
+++ controllers/hrm.py	2011-06-03 10:31:33 +0000
@@ -287,6 +287,8 @@
     s3mgr.configure(tablename,
                     list_fields = ["id",
                                    "person_id",
+                                   "email",
+                                   "mobile",
                                    "person_id$occupation",
                                    "organisation_id",
                                    "status",
@@ -482,6 +484,8 @@
     if session.hrm.mode is not None:
         s3mgr.configure(tablename,
                         list_fields=["id",
+                                     "email",
+                                     "mobile",
                                      "organisation_id",
                                      "type",
                                      "job_title",
@@ -492,6 +496,8 @@
     else:
         s3mgr.configure(tablename,
                         list_fields=["id",
+                                     "email",
+                                     "mobile",
                                      "type",
                                      "job_title",
                                      "status",

=== modified file 'models/06_hrm.py'
--- models/06_hrm.py	2011-06-01 21:10:57 +0000
+++ models/06_hrm.py	2011-06-03 10:31:33 +0000
@@ -443,12 +443,19 @@
                 hrm_update_staff_role(record, user_id, role_id)
             hrm_update_manager_role(record, user_id)
 
+    hrm_email = s3base.S3VirtualFieldPersonContact(
+        db, tablename, "email", T("Email"), 1)
+
+    hrm_mobile = s3base.S3VirtualFieldPersonContact(
+        db, tablename, "mobile", T("Mobile phone"), 2)
+
     s3mgr.configure(tablename,
                     super_entity = db.sit_trackable,
                     deletable = False,
                     search_method = human_resource_search,
                     onaccept = hrm_human_resource_onaccept,
-                    ondelete = hrm_human_resource_ondelete)
+                    ondelete = hrm_human_resource_ondelete,
+                    virtual_fields = [hrm_email, hrm_mobile])
 
     # Add Staff as component of Organisations & Incidents
     joinby = dict(org_organisation="organisation_id")

=== modified file 'modules/s3/s3crud.py'
--- modules/s3/s3crud.py	2011-05-30 16:00:35 +0000
+++ modules/s3/s3crud.py	2011-06-03 10:31:33 +0000
@@ -58,7 +58,7 @@
 from s3pdf import S3PDF
 
 from gluon.sqlhtml import SQLFORM
-from s3tools import SQLTABLES3
+from s3tools import SQLTABLES3, S3VirtualField
 from s3utils import s3_mark_required
 
 # *****************************************************************************
@@ -364,17 +364,6 @@
 
         editable = self._config("editable", True)
         deletable = self._config("deletable", True)
-        list_fields = self._config("list_fields")
-
-        # List fields
-        if not list_fields:
-            fields = resource.readable_fields()
-        else:
-            fields = [table[f] for f in list_fields if f in table.fields]
-        if not fields:
-            fields = []
-        if fields[0].name != table.fields[0]:
-            fields.insert(0, table[table.fields[0]])
 
         # Get the target record ID
         record_id = self._record_id(r)
@@ -479,7 +468,18 @@
             return exporter(r, **attr)
 
         elif representation == "xls":
-            list_fields = self._config("list_fields")
+            # @ToDo: list_fields isn't really intended for reports -- it's
+            # for letting a user see which record they want to open. And most
+            # list_fields have "id" in them, which we likely don't want, unless
+            # represent is applied. Fran says Grameme has put in a
+            # "report_fields", so maybe that would be more appropriate, both
+            # here and for pdf.
+            # @ToDo: Do we want to include indirect or generated fields like
+            # fk$fn or virtual fields? Probably yes, but omitting them for
+            # the moment. (Note cannot pass list_fields in to readable_fields()
+            # when we want to keep the non-table fields.)
+            fields = self.resource.readable_fields(self._config("list_fields"))
+            list_fields = [f.name for f in fields]
             exporter = resource.exporter.xls
             return exporter(resource, list_fields=list_fields)
 
@@ -790,15 +790,22 @@
         # List fields
         if not list_fields:
             fields = self.resource.readable_fields()
-        else:
-            fields = [table[f] for f in list_fields if f in table.fields]
-        if not fields:
-            fields = []
-
-        if fields[0].name != table.fields[0]:
-            fields.insert(0, table[table.fields[0]])
-
-        list_fields = [f.name for f in fields]
+            list_fields = [f.name for f in fields]
+        #else:
+            # @ToDo: Does this not prevent list fields of the form fk$fn
+            # from getting through?  In any case, _get_list_fields will take
+            # care of stripping out bogus fields (well, sort of -- see "other"
+            # in _get_list_fields)...and surely including a bogus field is a
+            # bug, and should cause an error rather than silently passing by.
+            # We're only doing this to insert "id" in front if it's not there.
+            #fields = [table[f] for f in list_fields if f in table.fields]
+        #if not fields:
+            #fields = []
+
+        field0 = table.fields[0]
+        # Make sure there's an "id" column on the front.
+        if list_fields[0] != field0:
+            list_fields.insert(0, field0)
 
         # Filter
         if response.s3.filter is not None:
@@ -821,7 +828,7 @@
                     del vars["iSortCol_0"]
                     del vars["sSortDir_0"]
                 if r.method == "search" and not orderby:
-                    orderby = fields[0]
+                    orderby = field0
 
             # Custom view
             response.view = self._view(r, "list.html")
@@ -862,7 +869,7 @@
 
             # In SSPag, send the first 20 records together with the initial
             # response (avoids the dataTables Ajax request unless the user
-            # tries nagivating around)
+            # tries navigating around)
             if not response.s3.no_sspag and items:
                 totalrows = self.resource.count()
                 if totalrows:
@@ -943,7 +950,7 @@
             if vars.iSortingCols and orderby is None:
                 orderby = self.ssp_orderby(table, list_fields, left=left)
             if r.method == "search" and not orderby:
-                orderby = fields[0]
+                orderby = field0
 
             # Echo
             sEcho = int(vars.sEcho or 0)
@@ -987,6 +994,8 @@
 
         elif representation == "xls":
             exporter = S3Exporter(self.manager)
+            # @ToDo: Add support for fk$fn and virtual fields in exporter.xls.
+            # @ToDo: Use "report_fields" instead of "list_fields".
             return exporter.xls(self.resource,
                                 list_fields=list_fields,
                                 report_groupby=report_groupby)
@@ -1039,7 +1048,12 @@
         if table._id.name not in fields:
             fields.insert(0, table._id.name)
 
-        lfields, joins = self.get_list_fields(table, fields)
+        lfields, joins, lefts, xfields, vfields = self._get_list_fields(table, fields)
+        if left is None:
+            left = []
+        elif not isinstance(left, (tuple, list)):
+            left = [left]
+        self._combine_lefts(left, lefts.values())
         colnames = [f.colname for f in lfields]
         headers = dict(map(lambda f: (f.colname, f.label), lfields))
 
@@ -1057,11 +1071,14 @@
         for j in joins.values():
             query &= j
         # Left outer joins
-        if left is not None:
+        if left:
             attributes.update(left=left)
 
         # Retrieve the rows
-        qfields = [f.field for f in lfields if f.field is not None]
+        qfields = [f.field for f in lfields if \
+                   f.field is not None and f.type != "s3_virtual_field"]
+        # Include extra fields requested for virtual fields.
+        qfields.extend(xfields)
         rows = db(query).select(*qfields, **attributes)
         if not rows:
             return None
@@ -1070,9 +1087,12 @@
         # Representation
         def represent(f, row):
             if f.field:
-                return self.manager.represent(f.field,
-                                              record=row,
-                                              linkto=linkto)
+                if isinstance(f.field, S3VirtualField):
+                    return f.field.represent(row)
+                else:
+                    return self.manager.represent(f.field,
+                                                  record=row,
+                                                  linkto=linkto)
             else:
                 if f.colname in row:
                     return row[f.colname]
@@ -1083,17 +1103,20 @@
         if as_page:
             # ...JSON page (for pagination)
             items = [[represent(f, row) for f in lfields] for row in rows]
-        elif as_list:
-            # ...Python list
-            items = rows.as_list()
         else:
-            # ...SQLTABLE
-            items = SQLTABLES3(rows,
-                               headers=headers,
-                               linkto=linkto,
-                               upload=download_url,
-                               _id="list",
-                               _class="dataTable display")
+            if vfields:
+                S3VirtualField.add_virtual_values_to_rows(rows, *vfields)
+            if as_list:
+                # ...Python list
+                items = rows.as_list()
+            else:
+                # ...SQLTABLE
+                items = SQLTABLES3(rows,
+                                   headers=headers,
+                                   linkto=linkto,
+                                   upload=download_url,
+                                   _id="list",
+                                   _class="dataTable display")
         return items
 
     # -------------------------------------------------------------------------
@@ -1583,7 +1606,13 @@
         wildcard = "%%%s%%" % context
 
         # Retrieve the list of search fields
-        lfields, joins = self.get_list_fields(table, fields)
+        lfields, joins, lefts, xfields, vfields = self._get_list_fields(table, fields)
+        # @ToDo: joins are not used -- are they relevant here?
+        if left is None:
+            left = []
+        elif not isinstance(left, (tuple, list)):
+            left = [left]
+        self._combine_lefts(left, lefts.values())
         flist = []
         for i in xrange(0, columns):
             field = lfields[i].field
@@ -1670,7 +1699,12 @@
 
         orderby = []
 
-        lfields, joins = self.get_list_fields(table, fields)
+        lfields, joins, lefts, xfields, vfields = self._get_list_fields(table, fields)
+        if left is None:
+            left = []
+        elif not isinstance(left, (tuple, list)):
+            left = [left]
+        self._combine_lefts(left, lefts.values())
         columns = [lfields[int(vars["iSortCol_%s" % str(i)])].field
                    for i in xrange(iSortingCols)]
         for i in xrange(len(columns)):
@@ -1708,18 +1742,51 @@
         return ", ".join(orderby)
 
     # -------------------------------------------------------------------------
-    def get_list_fields(self, table, fields):
+    def _get_list_fields(self, table, fields, include_virtual=True):
         """
             Helper to resolve list_fields
 
             @param table: the table
             @param fields: the list_fields array
+            @param include_virtual: set this to False to omit virtual fields
+            sqltable.represent() called.
+            
+            #returns: (lfields, joins, lefts, xfields, vfields) where
+                lfields is a list of Storage(), one per item in list_fields,
+                    containing:
+                    fieldname from list_fields,
+                    colname for Rows.colname,
+                    field -- the Field object,
+                    label,
+                    type is "field" for a normal field, or "s3_virtual_field"
+                        for a virtual field.
+                joins is a dict of foreign key fieldname vs. associated joins
+                    required by fk$fn items in list_fields. This includes an
+                    equijoin for notnull foreign key references and a test for
+                    not deleted if the table has that field.
+                lefts is a dict of foreign key fieldname vs. left outer joins
+                    for foreign key references that are *not* notnull, as
+                    needed by fk$fn items
+                xfields is a list of "extra" fields requested by any virtual
+                    fields
+                vfields is a list of S3VirtualField instances for the virtual
+                    fields on list_fields
+
+            The caller should merge joins and lefts with any they already have.
         """
 
         db = self.db
+        model = self.manager.model
+        table_vfields = dict(map(lambda f: (f.name, f),
+                                 model.get_config(table._tablename,
+                                                  "virtual_fields", [])))
 
-        joins = dict()
+        joins = {}
         lfields = []
+        lfieldnames = []
+        lefts = {}
+        xfields = []  # Non-displayed fields that should be included in select.
+        vfields = []  # S3VirtualField instances for any that are in fields.
         for f in fields:
             # Allow to override the field label
             if isinstance(f, tuple):
@@ -1729,6 +1796,11 @@
             field = None
             if "$" in fieldname:
                 # Field in referenced table
+                # If the foreign key field is notnull, we don't want to restrict
+                # to records that have the field filled in. This same choice is
+                # made elsewhere that a join is added for an fk ref, so we don't
+                # have to deal with fixing up an inconsistent choice.
+                type = "field"
                 fk, fn = fieldname.split("$", 1)
                 if fk in table.fields:
                     ftype = str(table[fk].type)
@@ -1736,11 +1808,21 @@
                         ftable = db[ftype[10:]]
                         if fn in ftable.fields:
                             field = ftable[fn]
-                            if fk not in joins:
+                            if fk not in joins and fk not in lefts:
                                 join = (table[fk] == ftable._id)
+                                query = None
+                                if not field.notnull:
+                                    lefts[fk] = ftable.on(join)
+                                else:
+                                    query = join
                                 if "deleted" in ftable.fields:
-                                    join &= (ftable.deleted != True)
-                                joins[fk] = join
+                                    deleted = (ftable.deleted != True)
+                                    if query:
+                                        query &= deleted
+                                    else:
+                                        query = deleted
+                                if query:
+                                    joins[fk] = query
                 if field is None:
                     continue
                 if label is None:
@@ -1748,20 +1830,60 @@
                 colname = str(field)
             elif fieldname in table.fields:
                 # Field in this table
+                type = "field"
                 field = table[fieldname]
                 if label is None:
                     label = field.label
                 colname = str(field)
+            elif table_vfields and fieldname in table_vfields:
+                if not include_virtual: continue
+                # These are our virtual field substitutes.
+                type = "s3_virtual_field"
+                field = table_vfields[fieldname]
+                vfields.append(field)
+                if label is None:
+                    label = field.label
+                colname = fieldname
+                if field.required_fields:
+                    # @ToDo: Want to let a virtual field include an fk$fn
+                    # reference? e.g. pull out above fk$fn join code into a
+                    # helper. Note that won't remove the need for some
+                    # virtual fields to do additional queries.
+                    xfields.extend([table[x] for x in field.required_fields])
             else:
-                # Virtual field?
+                # @ToDo: Add support for web2py virtual field?
+                type = "other"
                 colname = "%s.%s" % (table._tablename, fieldname)
 
             lfields.append(Storage(fieldname = fieldname,
                                    colname = colname,
                                    field = field,
-                                   label = label))
-
-        return (lfields, joins)
+                                   label = label,
+                                   type = type))
+            lfieldnames.append(fieldname)
+
+        # Strip any fields out of xfields that are already in lfields and
+        # remove duplicates.
+        seen = set()
+        xfields = [f for f in xfields if f.name not in lfieldnames and \
+                                         f.name not in seen and \
+                                         not seen.add(f.name)]
+        return (lfields, joins, lefts, xfields, vfields)
+
+    # -------------------------------------------------------------------------
+    def _combine_lefts(self, oldlefts, newlefts):
+        """
+            Combine two lists of left outer joins without adding duplicates.
+            Results end up in oldlefts.
+        """
+
+        # Web2py reorders the arguments in Table.on() so the order is the same
+        # in the resulting Expression, regardless of the order in the on() call.
+        # Comparing str() of the left outer join Expressions will determine if
+        # they are the same join.
+        oldleftstrs = [str(o) for o in oldlefts]
+        morelefts = [n for n in newlefts if str(n) not in oldleftstrs]
+        oldlefts.extend(morelefts)
 
 # END
 # *****************************************************************************

=== modified file 'modules/s3/s3tools.py'
--- modules/s3/s3tools.py	2011-06-02 11:08:10 +0000
+++ modules/s3/s3tools.py	2011-06-03 10:31:33 +0000
@@ -39,7 +39,9 @@
            "QueryS3",
            "FieldS3",
            "CrudS3",
-           "S3ReusableField"]
+           "S3ReusableField",
+           "S3VirtualField",
+           "S3VirtualFieldPersonContact"]
 
 import sys
 import datetime
@@ -550,3 +552,256 @@
 
 
 # =============================================================================
+class S3VirtualField(object):
+    """
+        Alternative to Web2py virtual field that is easier to use in a list.
+
+        Unlike a Web2py virtual field, this one has a name, only provides
+        one function, and knows which fields it needs from its associated
+        table / resource. So it doesn't need reflection to find out what
+        fake fields it represents, and it doesn't require the list code to
+        fetch entire rows because we don't know what fields it might want
+        (bad for tables with big fields like wkt).
+
+        If desired, these can be stored in the associated resource, so they
+        can be referenced by name in list_fields rather than inserting the
+        instance.
+
+        Example:
+
+        def get_email(xself, row, db):
+            pid = xself._get_field(row, "person_id")
+            query = (db.pr_person.id == pid) & \
+                    (db.pr_person.pe_id == db.pr_contact.pe_id) & \
+                    (db.pr_contact.contact_method == 1)
+            result = db(query).select(db.pr_contact.value, limitby=(0,1)).first()
+            if result:
+                return result.value
+            else:
+                return NONE
+
+        hrm_email = s3base.S3VirtualField(db,
+                                         "hrm_human_resource",
+                                         "email",
+                                         "Email",
+                                         get_email,
+                                         ["person_id"])
+
+        s3mgr.configure(db.hrm_human_resource,
+                        virtual_fields = [hrm_email, ...],
+                        list_fields = ["id", ..., "email", ...])
+
+        Alternatively, they can be called stand-alone to generate their value,
+        given the id of a record in the associated table:
+
+        # hr_id is an id in hrm_human_resource
+        email_value = hrm_email.represent(hr_id)
+
+        Like a Web2py virtual field, the specified function might still do
+        a query to get joined data, so it is not as efficient as an explicit
+        SQL query to fetch the list fields plus all needed joined values.
+        Note it's tricky to specify a query in Web2py query syntax that
+        would return all needed fields *and* all needed rows including those
+        that fail to have a value for the virtual field -- that needs a
+        one-sided outer join. The problem is that list can have a filter, and
+        it's the filtered set that would need to be outer-joined against, not
+        the full table. That can be done in SQL with a nested query /
+        temporary table, but Web2py query syntax, or its "left" outer join
+        option, do not support a nested query / temporary table. However,
+        nothing is stopping the supplied function from looking in the Row
+        it's given to see if the fields it wants from other tables are there.
+        This would require knowing the possible contexts in which it might be
+        called, to be sure that those fields were joined in the intended
+        manner. Since we're writing the function, and putting in the calls to
+        it, we do know if opportunistic use of scavenged fields is ok. But
+        it's safer to just do a query and fetch them... For some sorts of
+        fields from other tables, such as those with a direct foreign key
+        reference from the primary table, or components of this resource, we
+        could specify the needed fields. But that just pushes the need to do
+        an outer join against the filtered set upstream.
+
+        @ToDo: If we could make this behave enough like a Field not to confuse
+        whatever Field-like operations are needed, we could subclass Field.
+        Goal would be to put these directly in Table (or a Table subclass),
+        where they could be ignored for db operations, but used for display
+        in forms.
+    """
+    
+    def __init__(self, db, tablename, fieldname, label, method,
+                 required_fields, **attr):
+        """
+            @param db: A DAL instance.
+            @param tablename: The name of the table with which this "field" is
+                associated. (Needed to dereference values in a Row in case they
+                include joins.)
+            @param fieldname: The name of this "field". The name should not
+                contain a "." as that would make it look like
+                <tablename>.<fieldname>, which would fool SQLTABLE3.
+            @param label: Heading / label for use in lists / forms.
+            @param method: A function to compute the desired value. It will be
+                called as a method of the S3VirtualField instance, so its first
+                argument should be the instance (usually "self" but if the
+                function is defined inside a class, a different name should be
+                used). In addition it should take a Row. It should return a
+                value compatible with str(), for display in a list or form. It
+                can use any internal helpers or data from S3VirtualField, via
+                its instance argument, such as _get_field for extracting values
+                from the Row.
+            @param required_fields: Fields needed from the associated table.
+                (This avoids the need for a query just to get other fields in
+                the same table.)
+            @param attr: Anything else we might want.
+                Do not name these the same as any other parameters.
+        """
+
+        self.db = db
+        # @ToDo: If we want this to subclass Field, allow the user to set
+        # the type (and default to, e.g., "string"). Then use isinstance, not
+        # type, to check if this is an S3VirtualField.
+        self.type = "s3_virtual_field"
+        self.tablename = tablename
+        self.name = fieldname
+        if label:
+            self.label = label
+        else:
+            self.label = fieldname
+        self._represent = method
+        self.required_fields = required_fields
+        if attr:
+            for key, value in attr.iteritems():
+                self.__dict__[key] = value
+        self.writable = False
+
+    # @ToDo: If we put something like this out where we can all get it, we
+    # could share it... This one is very slightly pitched toward fake virtual
+    # fields, which put their values in Row._extra, but is intended to work
+    # for normal Row instances.
+    def _get_field(self, row, colname):
+        """
+            Get requested field's value from the row.
+
+            If colname id unqualified it's assumed to be a fieldname from the
+            table to which this virtual field belongs.
+        """
+
+        if table_field.match(colname):
+            (tablename, fieldname) = colname.split(".")
+        else:
+            fieldname = colname
+            tablename = self.tablename
+        if tablename in row:
+            return row[tablename].get(fieldname, None)
+        # Row's __getitem__ allows row["x"] and row.x if "x" is in row._extra,
+        # but does not find "x" via either "x" in row or row.get("x",...).
+        if "_extra" in row:
+            if fieldname in row._extra:
+                return row._extra[fieldname]
+            elif colname in row._extra:
+                return row._extra[colname]
+        return None
+
+    # @ToDo: Want this to be just __call__?
+    def represent(self, value):
+        """
+            Fetch a row if necessary, and call the supplied method.
+
+            @param value: either a Row containing the required fields, or an id,
+            in which case the row will be fetched.
+        """
+        
+        if isinstance(value, Row):
+            row = value
+        else:
+            db = self.db
+            query = db[self.tablename].id == value
+            row = db(query).select(*self.required_fields,
+                                   limitby=(0, 1)).first()
+        if row:
+            return str(self._represent(row))
+        else:
+            return ""
+
+    @staticmethod
+    def add_virtual_values_to_row(row, *virtual_fields):
+        """
+            Add values from the specified virtual fields to the row.
+            
+            @param row: a Row containing the required fields for the specified
+            S3VirtualFields.
+            @param virtual_fields: a list of S3VirtualField instances whose
+            values to insert.
+
+            It is the caller's responsibility to fix up colnames in the Rows,
+            if needed.
+        """
+
+        if not "_extra" in row:
+            row["_extra"] = Row()
+            
+        for v in virtual_fields:
+            row["_extra"][v.name] = v.represent(row)
+
+    @staticmethod
+    def add_virtual_values_to_rows(rows, *virtual_fields):
+        """
+            Add values from the specified virtual fields to each row in rows.
+
+            This does not add the colnames for the virtual fields to rows --
+            the caller should add whichever fields they want displayed.
+            
+            @param rows: a Rows where each Row contains the required fields for
+            the specified S3VirtualFields.
+        """
+
+        if not rows:
+            return
+
+        for row in rows:
+            S3VirtualField.add_virtual_values_to_row(row, *virtual_fields)
+
+# Example of a subclass that can be reused for various tables.
+
+class S3VirtualFieldPersonContact(S3VirtualField):
+
+    def _get_person_contact(self, row):
+        db = self.db
+        pid = self._get_field(row, "hrm_human_resource.person_id")
+        query = (db.pr_person.id == pid) & \
+                (db.pr_person.pe_id == db.pr_contact.pe_id) & \
+                (db.pr_contact.contact_method == self.contact_method)
+        result = db(query).select(db.pr_contact.value, limitby=(0,1)).first()
+        if result:
+            return result.value
+        else:
+            return ""
+
+    def __init__(self, db, tablename, fieldname, label, contact_method, **attr):
+        """
+            @param db: A DAL instance.
+            @param tablename: The name of the table with which this "field" is
+            associated. (Needed to dereference values in a Row in case they
+            include joins.)
+            @param fieldname: The name of this "field".
+            @param label: Heading / label for use in lists / forms. (This is
+            probably the value from pr_contact_method_opts corresponding to
+            the index supplied in the following arg, but we don't have access
+            to the opts here).
+            @param contact_method: The index of the desired contact type from
+            pr_contact_method_opts.
+            @param attr: Anything else we might want.
+            
+            If the field name for the reference to pr_person is not person_id,
+            it can be supplied as a keyword arg:
+            @param person_id_fieldname: The foreign key reference to pr_person,
+            if it's not person_id
+        """
+
+        if "person_id_fieldname" in attr:
+            self.person_id_fieldname = attr[person_id_fieldname]
+        else:
+            self.person_id_fieldname = "person_id"
+        self.contact_method = contact_method
+        S3VirtualField.__init__(self, db, tablename, fieldname, label,
+                                self._get_person_contact,
+                                [self.person_id_fieldname],
+                                **attr)