Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
create a forum, save.
Try then to edit > delete it.
Comment | File | Size | Author |
---|---|---|---|
#6 | forum_delete_form_2.patch | 4.12 KB | edmund.kwok |
#2 | forum_delete_form.patch | 3.36 KB | edmund.kwok |
Comments
Comment #1
webchickConfirmed. Though this does seem to work from the admin/category interface.
Comment #2
edmund.kwok CreditAttribution: edmund.kwok commentedAnother one of those codes that gets left behind with drupal_set_form's new syntax. The edit forum page is a callback to drupal_get_form() with the argument 'forum_form_forum'. The delete operation is checked in forum_form_forum() and returns a confirm form. The issue here is that the confirm form array not rendered, as in not passed to drupal_get_form.
It should be noted that the same problem exists with deleting containers so the title of this issue was changed accordingly.
This patch does these:
1. Changes callback of adding/editing forum and container page to forum_form_main()
2. forum_form_forum() would determine which form to render, including the confirm delete form
3. forum_form_forum() and forum_form_container() are only responsible for returning the form array for adding/editing forums and containers
You may need to reset your menu before the patch can work. Please review.
Comment #3
webchickProblem is now fixed with the patch.
However, should we be doing $form_values 'op' and 'confirm' rather than $_POST? Referencing your other patch here: http://drupal.org/node/83397 which looks like it was received favourably.
Comment #4
eaton CreditAttribution: eaton commentedwebchick, it appears that this situation is slightly more complicated. The code in question is doing its work in the page-building function, rather than the submit handler itself. Thus, it has no access to $form_values and peeks directly at $_POST. That's more of an underlying workflow problem with this set of forms, and should probalby be tackled separately once the 'blocking defect' is fixed...
Comment #5
webchickFair enough. :)
Comment #6
edmund.kwok CreditAttribution: edmund.kwok commentedAh, I was wondering how I would explain the situation of the code. A few other modules also follow a similar workflow, and we can only use $_POST. I agree with Eaton and we should definitely look into how this can be improved.
Anyway, one slight change to the patch; the Cancel link points to admin/content/forums instead of admin/content/forum. Apart from that, this patch should be good to go :-)
Comment #7
Dries CreditAttribution: Dries commentedShouldn't we use the _validate + _submit hooks here?
Comment #8
drummInstead of overloading that callback, the delete page should have a separate menu item.
Comment #9
edmund.kwok CreditAttribution: edmund.kwok commentedI think what Eaton said in #7 could applied to both Dries' and drumms's concerns.
Dries:
If I'm reading it right, in addition to what Eaton said, using _submit and _validate hooks in this issue is not possible with the current workflow. The confirm delete function returns a form array and the _submit and _validate hooks don't return rendered forms.
drumm:
I think for the time being, this patch is the best way to solve the issue as reported. Like Eaton said, we should look into the issue of reworking the workflow of deleting forums (e.g. separate menu callback, avoiding the use of $_POST) separately. I think comment and user multi delete also follow the same workflow and could benefit from a separate menu callback, but IMO should be left for another issue.
Comment #10
edmund.kwok CreditAttribution: edmund.kwok commentedImproving delete forum/container workflow using a separate menu callback is addressed in http://drupal.org/node/85539
Comment #11
drummCommitted to HEAD.
Comment #12
(not verified) CreditAttribution: commented