schooltool-developers team mailing list archive

[Justas <justas@xxxxxx>]

Hi,

On 11/19/2010 09:08 AM, Alan Elkner wrote:
> Hey Justus,
>
> I hope you're feeling better.  I decided to not to bother you for a
> while to give you a chance to recover, but I do want to ask you to
> take a look at my latest commit to my schooltool branch,
>
> https://code.launchpad.net/~aelkner/schooltool/demo_fields
>
> in which I added support for resource demographics.  I have only
> created the objects and adpaters and written the unit tests, but I was
> hoping you could look it over and tell me what you think.  You'll
> notice that I created the same containers for the demo fields and the
> demo data as in the basicperson package.
  Don't re-implement all ***FieldDescription, just import them from 
schooltool.basicperson.demographics.  Same goes for IDNAVocabulary.

  By the way, you'll need to implement DemographicsFormAdapter that 
adapts a resource.  IDemographicsForm should be imported from 
schooltool.basicperson.demographics

>    Also, I recreated the same
> classes, so there is some duplication of code there, but the question
> could be, what were those classes doing in basicperson in the first
> place?
  Historically, there are three separate implementations of persons in 
ST core: simple person, basic person (the one we use) and demographics 
person (one with so-extensive multi-page edit form).  Nobody uses simple 
and demographics persons anymore (hopefully).  IIRC, demographics person 
is somewhat broken at the moment.

  Classes you mentioned add demographics fields to the basic person, 
but not to other two person implementations.

  I know it... needs fixing.

> Also, consider that in the case of the resource demo fields,
> they are wired to be limited by resource_type instead of group_id as I
> did with the basicperson demo fields.  In both cases the filter is a
> key of sorts (one being a person id and the other a resource object
> type), and it's just the method name that differs between them.
  I'm still uneasy with that solution as it is now.  I'm quite happy 
with the implementation you suggested later on :)

> That brings me to a point I wanted to make in response to your
> objection to using group ids in the case of filtering basicperson demo
> fields.  I think there could be a point to be made for filtering them
> by the three core person types (admin, teacher, student) and forget
> about any additional groups that the user may or may not set up from
> year to year.
  Hmm, this is interesting...
> SchooTool does set up those groups by default anyway,
> and so much of the application is wired to take into account the role
> of the user,
  First, the groups are not set up until user creates a school year.  
It's a very annoying problem for me.
  Secondly, it's not written in stone that default groups in a year or 
so will work as they do now.  Or that will exist in all deployments.

> so why not have the filter of person demo fields go by a
> person type (so to speak), using a vocabulary key as the value that
> goes in the list.  This would make it the same concept as what I'm
> using with resource type, again potentially a vocabulary with keys
> ('resource', 'location', and 'equiptment') to represent the three
> types of resource objects.
  Now this part got me really interested.

  A vocabulary that takes DemographicsFields (container) as the context 
- to produce filter values.
  For person demographics it would give the hard-coded keys of default 
groups.
  For resource fields it can use the ResourceFactoryUtility.types list

  Then you can add fields property to DemographicsFormAdapter that 
returns a list of relevant fields for the person/resource (filters field 
descriptions and calls makeField for each).  And of course make 
PersonForm.generateExtraFields use those fields.

  Yes, that will work fine... Great idea! :)

> Incidentally, I will proceed with writing
> the resource edit forms to use the filter_resource_type method in my
> new resource demo fields in the meantime, and I will not set up the
> vocabularies in the short term, just relying on the enum widget for
> entering the limit_resource_types.
  Sure.
> If we are able to look at filtering both basicperson and resource demo
> fields by a vocabulary key (a simple unicode string), then perhaps we
> could move the whole set of demo fields out of basicperson and into
> the demographics package (don't they belong there anyway?), use a
> generic attribute name like limit_keys and method name like filter_key
> in the demo fields container.  Then the resource package could reuse
> the same classes, saving me from creating the duplicate code that I
> had to do in the resource package.
  As I mentioned before on IRC, I'd like this to become a way of adding 
extra fields to *any* objects in the future (e.g. courses).  So in 
general, I'm all for it, but I'd like to postpone this a bit.

  And ouch, moving.  I hate that nasty evolution - we'll be fixining 
references of *broken* persistent ***FieldDescription objects probably.
> Your thoughts?
  Spent half a day thinking how this fits into future design of ST.  
Thanks, good food for thought.

Cheers,
Justas