chx and I were discussing in IRC about http://drupal.org/node/144590#comment-652867
One of his ideas was to remove the multipage form entirely, since it adds all kinds of complication.
Basically:
- register our own simple page with a trivial project selector form at node/add/project-issue
- register a node_add() callback at node/add/project-issue/[arg]
- make project_issue_form() a single page form that assumes it knows the project
I've got this mostly working, but I haven't started ripping out all the multi-page form badness yet. Just wanted to post an issue to let the everyone know i'm working on it and it'll be coming soon.
Death to complicated FAPI in project*! ;)
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | pi_199138_breadcrumbs.patch | 873 bytes | dww |
| #7 | pi_no_multi_page.patch_4.txt | 21.21 KB | dww |
| #6 | pi_no_multi_page.patch_3.txt | 20.98 KB | dww |
| #5 | pi_no_multi_page.patch_2.txt | 20.9 KB | dww |
| #2 | pi_no_multi_page.patch_1.txt | 20.9 KB | dww |
Comments
Comment #1
webchick+1 :)
Comment #2
dwwThis patch looks way bigger than it really is: project_issue_form() now requires 1 less nested if(), so everything in that function is indented 2 spaces fewer. Aside from that, all this is doing is:
- Adding the new menu callbacks in project_issue_menu()
- Defining the new page (to set the right title) and form for the project picker (aka "page 1")
- Makes the node form require a valid project in the URL.
- Rip out all the crazy pre_render junk, button swapping, page manipulation, required field toggling, etc.
Seems to be working great, for both new issues and followups, with one exception: now, the JS when you modify a project which updates your choices for component/version is working on initial issue submit and editing issues, but not on comments. ;) So, we've got http://drupal.org/node/197281 in reverse with this patch... However, I think it's both desirable and possible to fix this without the full-blown $node namespace stuff, so I'm posting this now for initial review, testing, and in case anyone wants to track down the specific JS problem. Look in #197281 for more about that.
Comment #3
dwwTee hee, whoops. There's no bug after all, I just wasn't seeing it since I forgot that all the auto-generated projects in the d.o test profile have the same components (something else we should fix). Now that I modified a project with a different list, I see this is all working nicely. Please review/test. ;)
Comment #4
dwwBah, the other problem was http://drupal.org/node/199180 ;)
Comment #5
dwwhunmonk complained about the bogus
drupal_set_message(blah, 'warning'). That's only in this patch b/c of the indentation change, but sure, we can fix that here, too.Comment #6
dwwOh, and we shouldn't hard-code "Submit Issue", either. We should see what the current title of that content type is and use that.
Comment #7
dwwFixes from hunmonk:
- Removed the ", try again" part of a warning message. ;)
- Consolidated the validation so that non-existing and non-project are both lumped into "invalid project".
Fixes of my own:
- Fixed all the menu problems, so that "Issue" still shows up in the navigation menu
- Fixed the breadcrumbs on "page 2" so it still shows "Create content" as our parent.
The only remaining snag is that once you're on "page 2", the menu no longer thinks "Issue" is the active menu item (since we're really on a different menu item now). I'm not sure what, if anything, we can do about this, nor that I care. Just thought I should mention it.
Comment #8
hunmonk commentedcode looks good. ran some basic tests on creating issues and followups, and everything works fine. IMO, we're not really on the create issue page anymore, so i don't think the non-active item is really a problem.
let's get this puppy in -- it removes one of the ugliest sections of project code we have!
Comment #9
dwwCommitted to HEAD. Yay. I was strongly tempted to backport, but it's not really a bug, just cleaning the code and making it easier to port to D6, so 5.x-2.* is fine.
Comment #10
beginner commentedI just upgraded my test environment.
When I go to node/add/issue, firefox 1.5 stops processing:
Then, drupal message prints a long list of
Invalid project selected.Comment #11
dwwreset your menu and try again. e.g., save your admin modules page, or reset your menu via devel.module, etc.
Comment #12
beginner commentedI just did (save the module page), and the problem in the same.
I tried with Konqueror and have the same problem:
An error occurred while loading http://127.0.0.1/project_issue/?q=node/add/project-issue:
Found a cyclic link in http://127.0.0.1/project_issue/?q=node/add/project-issue.
I use PHP4, in case it matters.
Comment #13
beginner commentedSame problem if I turn off javascript.
I also noticed that just after the upgrade, the breadcrumbs when viewing an issue are wrong. They show
home > create contentinstead ofhome > projects > project name > issues.I see that the patch modifies the breadcrumbs so I wonder if it's related.
Comment #14
beginner commentedI only experience the problem with user 1
Comment #15
hunmonk commentedthis change is intended, and not a bug.
have you tried with a fresh install? i've been using user 1, and have not experienced any problems like this.
Comment #16
dwwOk, the breadcrumbs are clearly wrong. I forgot that project_issue_form() is called for comment followups, not just new issues. Attached patch solves it.
As for the other stuff, I'm not seeing any problems.
Comment #17
beginner commentedThe breadcrumbs patch is good.
The other problem is gone. I updated project* and everything works, now. I am surprised because my previous checkout of project_issue was only a few days old.
I apologize for wasting your time.
Comment #18
dwwGlad everything's cool. Committed to HEAD. Thanks.
Comment #19
(not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.