[OH-Dev] Problem solved for now -- Re: Trying to Use Form Validation in Class Based Generic Views

Tue Mar 27 13:49:01 UTC 2012

Thank you for the followup. I noticed you guys were chatting, I was
unconsciously signed into IRC. I saved the log to go back and look at the
convo.

I started making more changes to the class based view, and went to test and
realized
there is something I don't know about class based views, since it is
failing the tests :)
I now realize (again) that the documentation on generic class based views
is not great.
I did Google around and find a decent article, and now need to get back to
what I was
doing and read  it.

Thanks for the help!

On Tue, Mar 27, 2012 at 2:13 AM, Nathan Yergler <nathan at yergler.net> wrote:

> Just wanted to follow-up here on an IRC conversation Asheesh and I had
> last week (I think).
>
> I think the common goal is for missions to be more configuration than
> code, at least to the extent possible. On one extreme we have the
> Windows Setup mission, which is really just a series of templates.
> Putting together a mission like that should be as easy as listing the
> templates in a Python file and pointing the URL conf at that list (as
> opposed the having to write a view for each right now). The Svn
> Mission is more complex, but still encompasses a series to steps. It'd
> be nice to make that "configuration" aspect as obvious as possible.
>
> I'm not certain what abstractions are missing, but I'm going to try
> and write some mission-writing-docs (probably this weekend) so people
> have a better sense of how to apply class based views in OH.
>
> Happy to answer questions, etc, in the interim.
>
> NRY
>
> On Sun, Mar 18, 2012 at 7:10 PM, Nathan Yergler <nathan at yergler.net>
> wrote:
> > Comments inline...
> >
> > On Sun, Mar 18, 2012 at 11:32 AM, Asheesh Laroia <lists at asheesh.org>
> wrote:
> >> Excerpts from Nathan Yergler's message of Sun Mar 18 13:23:32 -0400
> 2012:
> >>> Sorry for the delayed reply, we had a major-minor meltdown at work on
> >>> Thursday that sort of sucked up all of my ability to think about
> >>> software for 48 hours. I took a look at the test in question, and
> >>> after pulling things apart for a bit, believe I found the issue. Well,
> >>> two issues.
> >>>
> >>> First, the magic keyword argument for overriding the context is
> >>> extra_context_data, not extra_context. So checkout_submit() was
> >>> passing in an extra_context kwarg and MissionBaseView.get_context_data
> >>> was looking for extra_context_data. That's the first problem.
> >>>
> >>> The second was/is that Checkout.get_context_data is clobbering the
> >>> svn_checkout_form with a new form instance. Making that conditional
> >>> (if 'svn_checkout_form' not in data: data['svn_checkout_form'] =
> >>> forms.CheckoutForm()) takes care of that problem. See the attached
> >>> diff (dont_clobber.patch) for both changes.
> >>
> >> for dont_clobber.patch:
> >>
> >> In classic open source poor-communication style, an equivalent patch
> >> to that was pushed to openhatch/oh-mainline:master a few days ago.
> >>
> >
> > No worries, it was actually useful to start with a failing test so I
> > had a way into the code, otherwise I think I would've been a little
> > lost.
> >
> >> But the reason I wanted your advice especially is your experience
> >> with generic Django class-based views, which your follow-on patch
> >> is about! (:
> >
> > Excellent.
> >
> >>
> >>> Thinking about the larger problem (using the same handler for get +
> >>> post, with some additional data from a post), I think we can clean
> >>> things up a little. Is this Checkout example a good one for the
> >>> problem? The only thing that seems to be getting passed in as
> >>> extra_context_data is a possible error message and form instance. My
> >>> first instinct is that this looks a lot like the FormResponseView. Is
> >>> that something you've looked at and discarded? I have a more invasive
> >>> patch attached, form_views.patch, that I *think* cleans things up and
> >>> eliminates the need to fake the request method. The form processing
> >>> views in Django can be complex and intimidating, but they're pretty
> >>> powerful. I'd love to hear what people think about the latter patch
> >>> (and I'm happy to help apply it more widely to the codebase over
> >>> time). Even if the problem isn't limited to forms, I suspect a similar
> >>> idiom can be applied: a .post() handler that calls .get() with the
> >>> result of a special method like self.get_extra_context(). I think
> >>> that'd make it easier to follow and debug the code (ie, no wondering
> >>> why your method is GET when you "know" you made a POST).
> >>
> >> I think this is a lot like FormResponseView, actually. When learning a
> >> new technology (like Django generic class-based views), I try to learn
> >> as much as fits in my head then and build something, so I feel like I
> >> have solid ground to stand on. That's how we ended up using the
> Template-
> >> related views even though this is really a FormResponseView.
> >>
> >> I think that moving all these to FormResponseView would be just great.
> >>
> >> My idea with using class-based views is to make the training missions'
> >> code be mostly configuration, not code; that way, we can make it very
> >> easy for people to write new training missions, without necessarily
> >> writing much code. And also, it minimizes bugs due to incorrect
> >> repetition.
> >>
> >
> > Concur. Getting the class inheritance right can be tough for people
> > who really want to leverage Django's class based views, so I think
> > having a base OpenHatch view makes a lot of sense.
> >
> >> e.g.
> >>
> >> * I would move success_url to be a string (the urls.py short name) in
> >>  the Form subclass.
> >
> > The view name or the resolved URL? When I initially wrote my patch I
> > had something like:
> >
> > class View:
> >    success_url = reverse('view-name')
> >
> > which didn't work since reversing a URL requires importing modules,
> > and importing the view module required reversing the URL (ka-boom). If
> > you want to make it the view name, I suggest naming the property
> > something like "success_view", and overriding get_success_url in your
> > parent view. That avoids assigning semantics to success_url that
> > differ from Django's and simplifies the child classes.
> >
> >>
> >> * class Checkout should have:
> >>    mission_step_name = 'svn_checkout'
> >>  and then the default form_valid() machinery should set the mission step
> >>  as completed.
> >
> > Makes sense.
> >
> >>
> >> * Perhaps all the missions forms should implicitly take the username as
> >>  a forms_kwarg? But then you couldn't use these mixins with non-Missions
> >>  forms, which would be odd.
> >>
> >
> > I could go either way with this one... if all the existing mission
> > forms (or > 80%) use a username right now, then I say do it. If you
> > want to further reduce the burden for contributors just give them a
> > form super class that has a different __init__. If you really want to
> > be magical you could write get_form_kwargs such that it sees if you're
> > dealing with a subclass of UserMissionForm and adds the username kwarg
> > if you are.
> >
> > How many of the mission forms do you think have username-dependencies
> right now?
> >
> >> * Then we could write a super-simple write-your-own-training-missions
> >>  HOWTO.
> >
> > Cool. I'd be happy to help with that from a class-based-view perspective.
> >
> >>
> >> This is the kind of thing that I intended for handling in these tickets:
> >>
> >> * http://openhatch.org/bugs/issue645
> >>
> >> * http://openhatch.org/bugs/issue666
> >>
> >> (surely others also)
> >>
> >> As I understand things, Jacquie plans to make some further code commits
> to
> >> do some of this move. I think we should keep conversing and sketching
> the
> >> code out here on OH-Dev -- Jacquie would probably appreciate the
> concreteness
> >> we're bringing to the conversation, as compared to my vagueness on those
> >> bugs!
> >>
> >> So Nathan (and all) -- do you think the above sorts of things I'd want
> >> to change are possible?
> >
> > Yup.
> >
> > And I've attached an updated patch with most of the code changes. It
> > also contains one other change, renaming template references from
> > "svn_checkout_form" to "form". That puts you in sync with what the
> > Django form views expect, and allows us to eliminate one more custom
> > override.
> >
> > NRY
> >
> >>
> >> -- Asheesh.
> >> _______________________________________________
> >> Devel mailing list
> >> Devel at lists.openhatch.org
> >> http://lists.openhatch.org/mailman/listinfo/devel
> _______________________________________________
> Devel mailing list
> Devel at lists.openhatch.org
> http://lists.openhatch.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openhatch.org/pipermail/devel/attachments/20120327/eb08fefb/attachment.html>