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

[Asheesh Laroia]

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.

But the reason I wanted your advice especially is your experience
with generic Django class-based views, which your follow-on patch
is about! (:

> 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.

e.g.

* I would move success_url to be a string (the urls.py short name) in
  the Form subclass.

* class Checkout should have:
    mission_step_name = 'svn_checkout'
  and then the default form_valid() machinery should set the mission step
  as completed.

* 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.

* Then we could write a super-simple write-your-own-training-missions
  HOWTO.

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?

-- Asheesh.