On Saturday 11 February 2006 08:51, Russell Keith-Magee wrote:
> Comments? Have I got it right this time?
It looks OK to me. I think the main issue is whether we want methods
that can alter data on the QuerySet directly. This kind of relates to
a change I mentioned before - having the 'RelatedManager' classes act
as QuerySets (probably by inheritance). This would change whether you
would have to do things like reporter.article_set.all(), or just
reporter.article_set. Also, without your patch, you could do
reporter.article_set.delete() (whether this was desired or not), with
it you would have to do reporter.article_set.all().delete().
Personally I'm for removing the need to do .all() with the
RelatedManagers, in both cases.
On the safety issue, there is still the problem of being able to do
{{ reporters.delete }} in the template (assuming you've set a QuerySet
of reporters as 'reporters' in the context). This can be easily fixed
by setting the 'alters_data' attribute on the delete() function.
Nitpicking: these line in the tests need to be changed:
# the article is represented by "<Article object>", because we haven't
given
# the Article model a __repr__() method.
Regards,
Luke
--
Life is complex. It has both real and imaginary components.
Luke Plant || L.Plant.98 (at) cantab.net || http://lukeplant.me.uk/
