schooltool-developers team mailing list archive

[Alan Elkner <aelkner@xxxxxxxxx>]

Hey Justas,

Could you please look at the last two commits I made to my refactoring
branch, lp:~aelkner/schooltool.intervention/test_coverage, the first
being for fixing the views and the second for evolution.  I'd like to
discuss them in detail at the meeting.

For the view changes, they mostly have to do with updating the widget
to work with the new attributes.  I moved the getPersonsResponsible
method out of schooltool/intervention/intervention.py into the widget
__init__ method itself since it was only being used there anyway.  The
widget code is cleaner now.

I thought it was a good idea to replace my redundant form key logic
with the supplied adapter from the contact package, but that does lead
to a problem with testing in that the form keys are not easily
dissected for their parts.  Perhaps we should have another '.' in the
'persons.%s%x' to allow us to get the user id easily using split('.').
 I had to write a new method in my tests called choiceControl that
finds the checkbox control based on the user id and parent index, but
it has to use startswith() which breaks if I have ids like 'teacher'
and teacher2', one starting with the other.  Fortunately I don't have
such ids, but it is a vulnerability for anyone changing the tests in
the future.

Another thing that would be nice is if we had an adapter that goes
from the form key back to the contact.  I just loop through all the
possible contacts in my widget to find a match which works but is less
efficient.

One thing that I left out of the evolution is the persons_responsible
catalog which is now keyed by the relationship property rather than
the list of ids as it was before.  I didn't write that evolve step yet
because I wanted to discuss it first to make sure it was okay to have
the index carrying the relationship properties in the first place.
Doesn't that cause the problem of having to visit too many objects
while looping though the index?  Isn't that why we try to use strings
like ids in an index when possible?  If so, what strings could we
carry there, InitIds?  You can look at my commit from earlier in the
week for the catalog changes and also the InterventionFilterWidget
class that uses the catalog for comparison.

Anyway, besides the small catalog evolution step which I can add to
evolve7 after we decide what to do, I think evolve7 looks good and is
well tested.  Let me know if you think otherwise.  I tested it
manually as well with a database I created using a checkout of trunk,
and it worked well there, too.  Only thing that didn't work was the
intervention tab view which will work better after we resolve the
catalog issue.

Thanks,
Alan