The ?destination redirect works if the Submit button is used but not the Save button when saving drafts is enabled. It currently reports that the save operation is successful and remains in the webform.

The use case is a view with a list of submissions with links to edit a submission. It would be useful to return to the view when any buttons are utilized.

CommentFileSizeAuthor
#18 3057138-18.patch5.29 KBbwong
#15 3057138-15.patch4.87 KBbwong
#13 3057138-5.patch1.74 KBbwong
#4 3057138-4.patch1.69 KBjrockowitz
#3 3057138-3.patch1.71 KBjrockowitz
#2 3057138-2.patch898 bytesjrockowitz

Comments

bwong created an issue. See original summary.

jrockowitz’s picture

jrockowitz commented
Status: Active » Needs review
StatusFileSize
new 3057138-2.patch898 bytes

The attached patch still needs test coverage around any form rebuilds with ?destination.

jrockowitz’s picture

jrockowitz commented
StatusFileSize
new 3057138-3.patch1.71 KB

Slightly better approach

jrockowitz’s picture

jrockowitz commented
StatusFileSize
new 3057138-4.patch1.69 KB

The attached patch removes the isAjax check which is not needed.

bwong’s picture

bwong commented
Status: Needs review » Reviewed & tested by the community

Works great. Nice refinement.

jrockowitz’s picture

jrockowitz commented
Status: Reviewed & tested by the community » Fixed

  • jrockowitz committed 21e5be9 on 8.x-5.x 
    Issue #3057138 by jrockowitz: Save draft ignores ?destination=/...
jrockowitz’s picture

jrockowitz commented
Status: Reviewed & tested by the community » Fixed
If you enjoy and value Drupal and the Webform module, get involved, consider joining the Drupal Association, and backing the Webform module's Open Collective.
jrockowitz’s picture

jrockowitz commented
Status: Fixed » Needs work

This patch is causing an unexpected regression where when updating a multi-step form with a ?destination clicking back or next is redirecting to the destination.

  • 1384892 committed on 8.x-5.x 
    Revert "Issue #3057138 by jrockowitz: Save draft ignores ?destination=/...
jrockowitz’s picture

jrockowitz commented

For now, I am reverting the patch. I will try to fix the issue with saving drafts with breaking back/next handling.

jrockowitz’s picture

jrockowitz commented
Status: Needs work » Closed (won't fix)

Right now the current behavior is when the form is saved the ?destination is used otherwise the form is rebuilt with a message when a draft is saved. I think this is the right behavior.

bwong’s picture

bwong commented
Status: Closed (won't fix) » Needs review
StatusFileSize
new 3057138-5.patch1.74 KB

I think this patch will work. It does not do the rebuild only if the ::save value is in the #submit entry of the triggering element. It works even if you have multiple save buttons. It works normally if the next/previous buttons in a multipage webform.

This patch is the same at 3057138-4.patch with the additional condition.

jrockowitz’s picture

jrockowitz commented
Status: Needs review » Closed (works as designed)
bwong’s picture

bwong commented
Status: Closed (works as designed) » Needs review
StatusFileSize
new 3057138-15.patch4.87 KB

I need this feature to handle the workflow where forms are partially filled in and then saved.

I understand that the default mode of operation does not work this way so this patch adds a setting to explicit enable the feature. The default is to operate as it does now but if you check the new Enable ?destination= with Save button option in the Webform Form Behaviors/Navigation settings then it honors the ?destination= argument. The global setting is included as well.

I did not include changes to the settings for all the unit tests as the default flag value is false so they appear to run as is. It is just a matter of adding:

form_submit_save_destination: false

after form_unsaved to all the .yml files. This should have its own unit test but I am still not getting my unit tests to work properly.

Status: Needs review » Needs work

The last submitted patch, 15: 3057138-15.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jrockowitz’s picture

jrockowitz commented

@bwong I find this feature a little confusing and addressing a very specific edge case. My recommendation is that you create a very simple work-around patch, post it here, and see if other people want this feature.

bwong’s picture

bwong commented
StatusFileSize
new 3057138-18.patch5.29 KB

Here is the scenario. I am surprised it is not more common but if most people always finalize a form then I can see why it would be less common.

The application is for a science fair where the students need to fill in an application. This can be done in the fall through the spring. The application includes dates and fields that should only be filled in when the application is complete. Normally they can start the application process in the fall by providing details like the project title and their contact information that they then save. They may add more information over time before the application is finally submitted.

All applications are listed when they log in (a user can have more than on application started, often a teacher starts the applications for each student). They can then click on the link to edit the webform submission/application. This link is where the ?destination= argument is used. Clicking Save on the application form brings them back to the list of applications. There are other places where this is used but essentially that Save button is supposed to close the form and bring the user back to the page where they started. In our initial tests the users were confused when the Save button simply left them in the form since they had to navigate back to where they started using menu items.

I think this patch (#18) fixes the failing tests. It includes the default_form_submit_save_destination setting initialization in the config/install/webform.settings.yml configuration file.

I can see where this mode of operation with the Save button would not be something desirable in a multistep form where the user simply wants to make sure the information entered thus far is retained even if the browser tab is closed or they navigate away from the webform. The patch allows the feature to be enabled on a per webform basis. The default settings are to operate as if the feature was not installed so it should not affect existing installations unless they modify the webform settings.

I would have done this as a separate module but there are no rebuild hooks that I could access and it seems like a useful option to have in general.

jrockowitz’s picture

jrockowitz commented
Status: Needs work » Closed (won't fix)

I think the best immediate solution is to use the below custom code to remove the ::rebuild callback when a draft is saved with a destination specified.

/**
 * Implements hook_webform_submission_form_alter().
 */
function MODULE_webform_submission_form_alter(array &$form, \Drupal\Core\Form\FormStateInterface $form_state, $form_id) {
  if (isset($form['actions']['draft']) && \Drupal::request()->get('destination'))  {
    \Drupal\webform\Utility\WebformArrayHelper::removeValue($form['actions']['draft']['#submit'], '::rebuild');
  }
}
bwong’s picture

bwong commented

Sorry for the late response. This works great and it does not disrupt a multiple page wizard flow. It would be handy to toggle in settings. I'll have to see about doing that in my module.