How about making it easier to have a preview also? As per today,
FormWizard and FormPreview aren't really combinable. Should probably
be it's own ticket...
HM
Thanks for taking the time to look at it.
> First and foremost, the latest patch doesn't even work. There's
> several naming conflicts, and mismatches in the docs. There's a few
> things I like about this, that I think would work well in the base
> wizard class (which needs to be created):
If you want you can message me offlist about this. I'd be interested
to work with you to resolve any issues. I wrote all the code and am
quite familiar with it, though I'm not a django or python guru. I
know that my most recent patch passes all the tests I wrote, but I
have not spent too much time with the code recently because the
project I was using this for changed direction.
> - The possibility to interact with the GET request to get form data (I
> want to sometimes skip a step if they come from a location which
> provides that information).
There's a hook that can be used to interact with GETs, it's called
_show_form. This is also called on validation failure. My thinking
was that POSTs will be where people need to make workflow decisions,
so most of the hooks are in the POST processing.
> And a few of the obvious issues:
>
> - The API is *completely* different.
> - Much of the code is specifically written for sessions (which is good
> and bad).
Yes it is. This is why I didn't see a clear route to a base class.
I actually started with a base wizard class, but removed it when there
was no common functionality.
> I've got it working locally myself, but I've already begun making
> tweaks (to make it work in our environment, and fix the bugs). I'm
> going to refactor a BaseWizard class, and try to keep all the original
> FormWizard methods, such as render_template, done, etc..
I think if you decide to use render_template for both hidden field
form wizards and session-based form wizards you're going to be doing
conditional "is this is a session wizard or a hidden fields wizard"
type of checking. But I haven't seen your work so can't say for sure.
> Most of the original base methods are still possible without tweaking
> the design, but one of the very obvious differences is the done
> method. This may be up for debate, but I believe the done method
> should be what it says, done. This means that the session data should
> automatically wipe once the done method has been called. There is also
> the issue that the forms are not passed via the done method, and this
> needs changed so that it is the same API.
Sounds good to me. Thanks.
-Dave
I think you mean validated and visited. The difference is that if
someone submits data for a page/form, then it's been visited. If
they've submitted valid data, then it's both valid and visited. My
examples in the docs didn't really draw attention to this point, but
here's the usage:
<ul>
{% for page_key,page in pages.items %}
<li class="{% if page.valid %}valid{% endif %}
{% if page.current_page %}current{% endif %}">
{% if page.visited %}
<a href="{{ URL_base }}{{ page_key }}">{{ page.title }}</a>
{% else %}
{{ page.title }}
{% end if %}
</li>
{% endfor %}
</ul>
In my example, visited pages get a link, otherwise no link. Valid
pages and the current page have a particular stylesheet rule applied.
Just an example of the kind of thing you might want to do with page
data in a wizard.
> I've fixed an issue with URL
> base, it seemed to only allow URLs which didn't end with a / (mine
> do).
>
> (r'^verify-number/(:?(?P<page_key>\w*)/)?$', 'verification.index'),
Yeah, allowing both trailing / versions and non-trailing / is better.
It wasn't a show-stopper for me, but that's a nice change. Think
there's a get_URL_base hook that could be overridden for odd URLs. Of
course, less work repetitive code is better.
> I've changed the done() method to pass form_classes just as it would
> originally. I've also changed _validate_all_forms to support this
> change in the new _handle_post_request (renamed from POST) method. GET
> is renamed to _handle_get_request. get_URL_base is renamed to
> get_url_base.
I didn't really see much of a point to pass the forms to done, since
there is already a method to retrieve forms. I was using this:
form_data = self._get_all_cleaned_data(request.session)
But I suppose everyone will be doing that early on in done(), so it
makes sense to make it a parameter.
> When the done() method is called, the session is also cleared unless
> done(). Maybe this should only happen if say a ValidationError isn't
> raised? Maybe a setting that makes this not happen? In all of my
> situations it makes sense to erase it when it's complete, and not have
> to call methods manual. Let's decide which is more common.
I think clear it automatically. I thought about this, but just didn't
implement it.
-Dave
If you look at my patch, you'll see this line:
page_data = self._get_cleaned_data(request, page_key)
....
if issubclass(form_class, forms.ModelForm):
form = form_class(instance=form_class.Meta.model(**page_data))
I tried using the POST data with initial, but it failed for ModelForm
.. the foreign key fields weren't handled properly. Maybe you've
solved that in a different way.
-Dave