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

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.

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.

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.

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

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.

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.
>