Hi,

When a module adds additional submit functions to a node submit, and the module's alter hook run's after this module's alter hook, it might happen that some submit functions won't run on submit, which might cause data loss.

There are two ways to solve it: do it with a reference, or use a submit handler on the draft button which calls all submit hooks. I think the second one is better, because it allows other modules to do submit hooks on just the draft saving.

Patch attached for the second solution.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

David_Rothstein’s picture

Status: Active » Needs work
+    $submit($form, &$form_state);

This will cause "Call-time pass-by-reference has been deprecated" notices. But otherwise, the patch looks good and worked fine when I tested it.

David_Rothstein’s picture

Status: Needs work » Needs review
FileSize
971 bytes

Rerolled to remove the "&".

David_Rothstein’s picture

Title: Better integration with 3rd party modules » Submit handlers added by 3rd party modules aren't called when the form is submitted using the "save as draft" button
nedjo’s picture

Status: Needs review » Reviewed & tested by the community

This is indeed a bug. Another approach that could be used to address it would be to set save_draft to be the last module invoked in hook_form_node_form_alter() via hook_module_implements_alter() so that all other submit handlers have already been added.

But rather than messing with hook ordering the current patch looks like the better approach and is ready to go.

askibinski’s picture

on a side note, here is how publish_button does this: ( similar module by nodeone)

http://drupalcode.org/project/publish_button.git/blob/bae35e4caf57c3dfa3...

NancyDru’s picture

Has anyone thought about using array_unshift() to stick the draft submit before the node submit handler?

David_Rothstein’s picture

@NancyDru: Which form element are you referring to, that the array_unshift() would be performed on?

The standard node submit button provided by Drupal core has its own (button-level) submit handlers. This module adds a new button, and that button needs to trigger the exact same thing. So it would seem to need submit handlers of its own in order to do that.

NancyDru’s picture

I haven't looked specifically at this code, but in several modules that I've written, I had to replace the standard $form['buttons']['submit']['#submit'][] = 'mymodule_submit'; with array_unshift($form['buttons']['submit']['#submit'], 'mymodule_submit') to force my handler to run before Node's.

David_Rothstein’s picture

Got it. Yeah, I've done a lot of that too, but in this case I don't think it will help, since we are constructing a totally new button and adding handlers to that.

deanflory’s picture

When applied to 7.x-1.4:

patch < save-as-draft-submit-handler-1446730-2.patch
patching file save_draft.module
Hunk #1 FAILED at 54.
Hunk #2 succeeded at 70 (offset -11 lines).
1 out of 2 hunks FAILED -- saving rejects to file save_draft.module.rej

When the latest individual file commits are copied over 7.x-1.4 since there is no dev version available on the project page:
save_draft.js
save_draft.module
save_draft.test

patch < save-as-draft-submit-handler-1446730-2.patch
patching file save_draft.module
Hunk #1 FAILED at 54.
Hunk #2 FAILED at 81.
2 out of 2 hunks FAILED -- saving rejects to file save_draft.module.rej

So, 1 out of 3 patches work. I'm thinking this project is dead.

MrHaroldA’s picture

@deanflory: if you clone the git repository, the patch will apply.

patching file save_draft.module
Hunk #1 succeeded at 64 (offset 10 lines).
Hunk #2 succeeded at 91 (offset 10 lines).

rooby’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

Thanks for the patch and your patience.

This has been committed.

re comment #5, that solution is not ideal and would not help fix this issue.

rooby’s picture

Oh, I changed the name of the submit function as I felt nervous about save_draft_submit not being specific enough and risking hook collisions.

rooby’s picture

Version: 7.x-1.x-dev » 6.x-1.8
Status: Fixed » Patch (to be ported)

Drupal 6 could use this too.

rooby’s picture

Version: 6.x-1.8 » 7.x-1.x-dev
Status: Patch (to be ported) » Fixed

Since D6 is done now, setting back to D7 and marking fixed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.