issue validation is broken by multipage form bugs

upon further investigation, this is definitely a problem with the multi-page form stuff going on. there are numerous bugs. i still haven't had a chance to fully grok the multi-page stuff, but the code in issue.inc looks nothing like the example multi-page form in http://api.drupal.org/api/HEAD/file/developer/examples/multipage_form_ex...

the example shows how you should use pre_render() to increment/decrement a numeric element in the form called 'page', whereas the code in issue.inc is messing with a string element called 'wizard'. furthermore, issue.inc is setting 'wizard' to 'one' in 1 place, but then checking if it's set to 'two' in project_issue_node_form_validate(). this second problem is what explains the busted behavior i originally reported.

also, even if you hack around the wizard stuff, in project_issue_node_form_validate() there are a few places we're checking some form element with empty(). however, the elements in question exist, but are set to 0. since 0 is not empty(), we consider these elements (e.g. category and component) to be valid, even when they're not set. :(

i'm trying to see (for my own education about multi-page forms) if i can roll a patch for this... i'd like to keep the changes small, but i fear i'll have to port the entire multipage form stuff to use a similar approach to the one in the example mentioned above. :( that'd be a much larger diff than i was hoping for, so i'll see if i can get it to work with a more simple diff... so far, not much luck. all this 'wizard' stuff in issue.inc appears to be hopelessly fubar. alas. i can get all the validation to actually work, but without more drastic changes to pre_render(), FAPI tries to validate the entire form as soon as you hit page 2, which isn't right, either (since it marks fields as errors before the users even have a chance to enter them in the first place). looks like pre_render() needs massive surgery (the comments in there regarding validation don't even agree with the code). *sigh* time for bed, but i'll see if i can get this working tomorrow....

i got a 2nd wind and managed to get a patch working for this. i've tested heavily on my local site, and this fixes all the validate bugs i was seeing. however, this is my first stab at a multi-page form, so i hope all is well. hunmonk, morbus, et al, please review. ;) if it looks good, please RTBC and i'll commit it.

thanks,
-derek

at hunmonk's suggestion, i'm taking out the code to validate title, since FAPI already does that for us. here's the new version...

applied (with the comments capitalized) as revision 1.158 of issue.inc. glad to help, nedjo. ;)

whoops, i just noticed a corner case my previous fix didn't handle. if you're viewing a project's issue page and click "submit", the URL contains the project and the code sends you directly to page 2 of the form. however, pre_render was confused, and thought we were still on page 1, so instead of displaying "[preview] [submit]", it was displaying "[next]". attached patch fixes this. please review/test. thanks!

slight edit to make the comment a little more clear. i'm going to commit this since drupal.org is currently running a buggy version and this needs to be fixed ASAP.

commited as revision 1.160 of issue.inc.

ugh. something changed in FAPI that broke this again. :( if you just do "create content", "issue" and go through both pages of the input (so you select a project on the 1st page, then on the 2nd page, fill in everything else), as soon as you land on the 2nd page, all the validation stuff is wrong, the buttons are screwed up, etc. this is visible on drupal.org and a HEAD test site. i'll see if i can solve it once more... *sigh*

-derek

eureka. i solved this one (again). i don't really know what changed in FAPI, but i think the old behavior was depending on a bug that's since been fixed. anyway, we need to *not* set the #required attribute on our required page 2 fields in project_issue_form() until our pre_render() hook. since we *want* all these fields to be required immediately when creating a followup (which is always the 2nd page of the form), but we do not want to make the 'body' field required in this case, i made a private helper method that sets the appropriate #required attributes, so that can be shared both from project_comment_form() and project_issue_form_pre_render().

please review/test and set to RTBC so i can commit it. i've done heavy testing and i'm nearly sure it's fine, but i'd like confirmation before i commit.

thanks!
-derek

i fixed the after_build needs to be an array bug in #60119, here's a new patch that applies cleanly.

in IRC, hunmonk pointed out that the 'pid' field is always required, so we can just leave "'#required' => TRUE" in project_issue_form(), instead of having to mess with it in _project_issue_form_add_required_fields()...

project_issue_2page.patch.2 (from #14) applied and committed to cvs.