This doesn't allow me to add custom submit callbacks - unless I completely alter the $form['#submit'] array to set mine first, but that's more lines. Either way, drupal_goto() is bad practice there anyway, $form_state['redirect'] should be used. Patch coming up.

CommentFileSizeAuthor
#6 don_t_use_drupal_goto-1916036-6.patch1.37 KBjeroent
#1 1916036-1.patch635 bytesswentel

Comments

swentel’s picture

swentel commented
Title: Don't use drupal_goto in a submit delete callback » Don't use drupal_goto in a submit callback
Status: Active » Needs review
StatusFileSize
new 1916036-1.patch635 bytes

Better title + patch - I haven't checked for other occurences of drupal_goto(), but this once fixes it for me in case a site is deleted where I pick in with custom functionality.

malc0mn’s picture

malc0mn commented

Thanks for that one! Will apply later today.

(Until recently, had the same prob in one of my other modules, I didn't know this existed in D7. This should really be added to the D7 FAPI docs, I learned it from the comments.)

malc0mn’s picture

malc0mn commented
Assigned: Unassigned » malc0mn
Status: Needs review » Fixed

applied, fixed some others as well.

malc0mn’s picture

malc0mn commented
Version: 7.x-1.4 » 6.x-1.4
Status: Fixed » Patch (to be ported)

Should remove these from the D6 version as well...

malc0mn’s picture

malc0mn commented

D6 prod_mon still has them in:

prod_monitor_overview_form_submit()
prod_monitor_flush_form_submit()
prod_monitor_delete_form_submit()

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Issue summary: View changes
Status: Patch (to be ported) » Needs review
StatusFileSize
new don_t_use_drupal_goto-1916036-6.patch1.37 KB

Created patch for D6 version of this module. Patch attached.

malc0mn’s picture

malc0mn commented
Version: 6.x-1.4 » 6.x-1.x-dev
Status: Needs review » Closed (fixed)

You missed the removal of one drupal_goto:

+++ b/prod_monitor/includes/prod_monitor.admin.inc
@@ -504,6 +504,7 @@ function prod_monitor_overview_form_submit($form, &$form_state) {
         if ($form_state['values']['fetch']) {
           $site_info = _prod_monitor_get_site($site->id, TRUE);
           _prod_monitor_retrieve_data($site->id, $site_info, TRUE);
+          $form_state['redirect'] = 'admin/reports/prod-monitor/site/' . $site->id;
           drupal_goto('admin/reports/prod-monitor/site/'.$site->id);
         }
       }

Applied your patch and removed the drupal_goto mentioned above as well, thanks!!!

mlc.

malc0mn’s picture

malc0mn commented

prod_monitor_overview_form_submit() redirect was no longer working. $form_state['storage'] needs to be empty in D6 for the $form_state['redirect'] to work. Learn something new about D6 still :-s.

Fixed.