← Back to team overview

schooltool-developers team mailing list archive

Re: Removing intervention dependency on gradebook

 

Hey Justas,

I changed my intervention package to use the zcml condition and
created the gradebook_integration zcml and py files.  All the tests
pass and manual tests with my new gradebook branch,
lp:~aelkner/schooltool.gradebook/intervention_integration, also work.
Only problem is that I had to add <include
package=”schooltool.gradebook”> (which I didn't commit) right before
the zcml condition statement in the intervenion config or the
condition would never be True.  I tried putting it in the
instance/school.zcml file, but it wouldn't work that way.  What is the
proper method in a sandbox environment for getting the gradebook
package loaded in time for the zcml condition?

Also, I added changes to both branches to no longer redirect to
app/no_current_term.html as per your suggestion.  Instead the views in
question just return the template themselves in the __call__ method.
I changed the tests in both branches to reflect this.

After testing my intervention branch against a really old Data.fs, it
became clear that I could not assume that a catalog existed before
updatePersonResponssible got called.  Also, I needed to fix evolve7.py
to not add contacts to the relationship properties if they were
already there.

Anyway, please have a look at my latest commits to the branches for
the upcoming meeting.

Thanks,
Alan


On Fri, Mar 25, 2011 at 11:17 AM, Justas <justas@xxxxxx> wrote:
> Hey,
>
>> I made the two changes we discussed at the meeting, fixing evolve7 and
>> fixing updatePersonsResponsible while adding test update views for
>> better test coverage.  Additionally, I made a couple more quick fixes
>> that involved re-organizing the code and zcml files in line with what
>> you had suggested at the sprint and moving all the templates into a
>> templates directory.  I like how much easier it is to find things this
>> way, and it was timely, too, because Filip will be building a similar
>> package in schooltool.courseinfo.  This way he has a better example in
>> schooltool.intervention.
>
> Cool :)
>
>> Please take a close look at my most recent commit because I'd like to
>> fix anything you find wrong before the weekend is out.  The commit is
>> for removing dependency on schooltool.gradebook.  It involves three
>> packages being merged at the same time, the intervention branch and
>> two new branches I pushed,
>> lp:~aelkner/schooltool/independent_activities  and
>> lp:~aelkner/schooltool.gradebook/independent_activities.  This
>> requires changing buildout.cfg temporarily to point to a checkout of
>> my new core branch, so my sandbox has all three branches side by side,
>> and I tested adding and removing schooltool.gradebook from
>> buildout.cfg.
>
>  I noticed you committed your sandbox buildout.cfg versions for gradebook
> and intervention.  Please fix!
>
>> The way I removed the dependency was by defining an interface in
>> schooltool core called IIndependentActivites.  The intervention views
>> that used to look for IActivities(section), a gradebook dependency,
>> now adapt to IIndependentActivites.  If schooltool.gradebook is not in
>> the build, the adapter lookup will fail, returning None which the
>> intervention views will be able to handle.  If schooltool.gradebook is
>> in the build, it will supply the requested adapter, returning
>> IActivities(section) which is what the intervention views need.
>
>  Sorry, a very firm no for IIndependentActivities going into core.
>  At the moment we don't have a built-in way to define a "soft" dependency on
> another plug-in.
>
>  Ideally, there should be zcml files in the intervention package, that
> include certain python modules (say,
> schooltool/intervention/gradebook_integration.py) only if
> schooltool.gradebook is included.
>
>  Basically, in gradebook's main zcml file, there should be a line:
> <meta:provides feature="plugin-schooltool.gradebook" />
>  And of course a namespace definition at the top of the file, next to
> others:
>   xmlns:meta="http://namespaces.zope.org/meta";
>
>  Then, in intervention package, configure.zcml, there should be an
> conditional include:
> <include zcml:condition="have plugin-schooltool.gradebook"
>      file="schooltool.gradebook-integration.zcml" />
>
>  And the namespace definition again:
>      xmlns:zcml="http://namespaces.zope.org/zcml";
>
>  This way schooltool.gradebook-integration.zcml will be included only if
> schooltool.gradebook is included.  Classes and methods that depend on
> imports from schooltool.gradebook should be moved to separate files that are
> used from schooltool.gradebook-integration.zcml.
>
>  As a simplest step, you can do this:
>  - keep the IIndependentActivities, but define it in
> schooltool/intervention/interfaces.py.
>  - put an adapter to IIndependentActivities to
> intervention/gradebook_integration.py
>  - define the adapter in schooltool.gradebook-integration.zcml and
>  - do the include with zcml:condition in intervention/configure.zcml
>
>  As far as the testing of integration goes... It's a deeper issue.  Let me
> think over it and get back at you on Monday.
>
>  For now you could keep your tests that register the stub adapter to
> IIndependentActivities and show that when such adapter exists, the views
> show pull correct gradebook information.  Keep gradebook_integration.py and
> schooltool.gradebook-integration.zcml untested.  Just check manually, that
> when gradebook is included, interventions work.
>
>  To sum up:
>  - few small changes for gradebook that provide the feature
>  - moving the interface and adapter from core to the intervention package
>  - a small check that provides the adapter when gradebook is included in
> intervention package
>
>  This would be a great step forward to standardized plugin interoperability.
>  I'd really appreciate that.
>
>> I created stubs for this in ftests.py, only registered via
>> ftesting.zcml, that deliver a fake IActivities(section) heirachy,
>> allowing for more complete coverage of the views that formerly had the
>> gradebook dependency but didn't have any test coverage.  For this, I
>> created a functional test file, independent_activities.txt which, in
>> chosing the section title, is able to test how those views would
>> handle schooltool.gradebook presense or absense.
>>
>> One last thing had to change, and it has been long overdue.  Until
>> now, schooltool.gradebook had a view for ISchooltoolApplication that
>> simply gives a message telling the user to set up a term.  I know that
>> you may have some ideas for what the view should say, so now would be
>> a good time for that.  In any event, I needed to move that view out of
>> schooltool.gradebook and into core so that the intervention package
>> could get at it.  Again, this is a reason that all three of my
>> branches have to merged at the same time.
>
>  I still strongly oppose that view getting into core.  There are too many
> things I don't like about it, and if you need to duplicate code, then code
> duplication it is.
>
>  - it's our software design issue in general that we can access stuff that
> is not configured
>  - it is not good that when users create the term and refresh the view - it
> still shows "please create a term"
>   - it's also bad that after creating the term you need to reload all pages
> that have, say, "Intervention" top navigation links - or else they get a
> message "please create a term" if they click them
>
>  So please either make a duplicate view with a different name in
> intervention package, or better - modify the relevant intervention views to
> display the message (say, render different template).
>
>  In the end there should be no changes in core, 1 small change in gradebook
> to provide "plugin-schooltool.gradebook" feature and the rest of the changes
> done in schooltool.intervention.
>
> Hope this helps,
> Justas
>
> _______________________________________________
> Mailing list: https://launchpad.net/~schooltool-developers
> Post to     : schooltool-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~schooltool-developers
> More help   : https://help.launchpad.net/ListHelp
>



Follow ups

References