We have a fairly complex form (something over 300 components) which is working fine when completed by an admin user. Another role is also allowed to submit the form, but when we try to complete and submit the form using a different login with the relevant role, we can't get passed the second page of the form. The first page saves fine as a draft and the user gets to the second page, but when clicking 'next page' the same second page just gets reloaded and the user sees the yellow warning message "Submissions for this form are closed". The for is sett o be open, and this was confirmed by checking the db table directly.

Clearly this is currently a showstopper for us at the moment, so any input, especially a fix, would be great.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

upperholme’s picture

Title: Ca't get passed "Submissions for this form are closed." » Ca't get past "Submissions for this form are closed."
upperholme’s picture

Title: Ca't get past "Submissions for this form are closed." » Can't get past "Submissions for this form are closed."
upperholme’s picture

On further investigation into this one, we've found that if we give the user the permission to edit their own submissions, then we don't see the warning any more and can go on to complete subsequent pages of the form. Whilst this is adequate as a short term solution, clearly there are issues here:

1. We don't want users to be able to edit their own form submissions once they have submitted the complete form, but we do want them to be able to actually complete this multi-page form and have it save as a draft when the user moves on to the next page (it's an 8 page form). Clearly there is a difference between these two things that the permission settings should be able to discern, but they aren't apparently doing so at present.

2. The warning message that appears is not actually useful or relevant. If we had closed submissions to the form then the message would be correct. If the user isn't allowed to edit their submission then they should see a message to that effect.

Hope this is useful, and that you can get this sorted in time for the 7.x-4.0 final release if not before.

Great module, by the way. Thanks.

Liam Morland’s picture

Version: 7.x-4.0-rc5 » 7.x-4.x-dev

I am seeing this too and am working to find the steps to reliably replicate it.

The problem is probably related to the line that follows comment "Check that the form is still open at time of submission". Conceptually, I don't understand why there is a call to webform_submission_access() here. Whether or not the user has access to submissions shouldn't impact whether or not the form is open.

Liam Morland’s picture

Steps to reproduce:

  1. Make a new form with a Save Draft button.
  2. Visit the form as an authenticated user with no other roles.
  3. Save Draft
  4. Submit

You will see "Submissions for this form are closed".

Possibly related: #1701610: Users are able to submit existing drafts when webform when status is set to 'Closed'

Liam Morland’s picture

The bug was introduced by 1a09891 which resolved #2277593: REGRESSION: Administrators can no longer edit closed submissions.

The first tag with this bug is 7.x-4.0-rc4.

Liam Morland’s picture

I think this line is the problem:

$closed = empty($form['#submission']) ? empty($node->webform['status']) : !webform_submission_access($node, $form['#submission'], 'edit');

Perhaps it needs to be:

$closed = empty($node->webform['status']) && !(webform_submission_access($node, $form['#submission'], 'edit') && <the current action is editing a draft> );

Though I question why webform_client_form_prevalidate() is called at all when the current action is editing a draft. None of the submission limit issues are applicable in that case.

Liam Morland’s picture

Status: Active » Needs review
FileSize
597 bytes

Here is a potential fix. It returns the "$closed =" line to how it was before 1a09891, which is OK because currently webform_client_form_prevalidate() doesn't get added to #validate if the submission is not a draft.

DanChadwick’s picture

@Liam - Thanks very much for your work. I expect to have some time to give to webform in a few days. I want to ponder what the absence of "Edit your own submission" permission means in terms of drafts. I *think* it should mean that you can continue to work on a saved draft from the node's view tab (node/NID), but not the submission's page (node/NID/submission/SID/edit). But maybe not. This would affect how the fix is implemented.

Liam Morland’s picture

With this patch, I tried editing a draft and it gave me the options of "save draft" and "submit". Both worked as expected.

DanChadwick’s picture

Re #1, yes but should you be allowed to, since you didn't have permission to edit your own submissions? Your answer implies that you think yes. I probably agree, but I'm still pondering.

EDIT: And apparently I suck at not working on webform. ;)

Liam Morland’s picture

I mean as a user with permission to edit all submissions. The commit that caused the problem was intended to fix the inability of admins to edit submissions. With the patch in #8, they can, so I think both problems are solved.

DanChadwick’s picture

The purpose of this patch was to make sure that *at the time of submission* the user was authorized. You are removing that extra check. The situation is that the user was authorized to submit when the started, but (perhaps some hours later when they click "submit") are not at the time of submission.

Liam Morland’s picture

Are you referring to editing a submission? webform_client_form_prevalidate() doesn't get run when a submission is being edited.

When a user is continuing to work on their form submission which has been saved as a draft, webform_submission_access($node, $form['#submission'], 'edit') might be false for them, but $form['#submission'] will not be empty. Whether the form is open or closed for them to submit should still depend on empty($node->webform['status']).

DanChadwick’s picture

Priority: Critical » Normal

Are you referring to editing a submission? webform_client_form_prevalidate() doesn't get run when a submission is being edited.

Not so. Look at webform_client_form_process().

When a user is continuing to work on their form submission which has been saved as a draft, webform_submission_access($node, $form['#submission'], 'edit') might be false for them, but $form['#submission'] will not be empty. Whether the form is open or closed for them to submit should still depend on empty($node->webform['status']).

Two related questions.

1) Should there be different behavior for a draft went submitting via the node page versus the submission edit page?

2) What should happen if permission changes between sending the form to the browser and submitting the form?

tenken’s picture

Hi. I'm trying to update my site and ran into this issue in the last 2 days, i've been following this thread :). I have a student role allowed to submit webform(s) and staff roles allowed to edit them. My form spans many pages and is using Drafts -- I'm seeing this message after the 1st page when logged in as student.

I'm just giving my 2 cents.

1) Should there be different behavior for a draft went submitting via the node page versus the submission edit page?

In my opinion a webform should be submittable fully through the very last webform component. I view incomplete forms as not "edits" but being "yet to be completed". I suppose this rule could enforce altering fully submitted webforms from the Submissions Edit page.

In my scenario students aren't allowed to modify their completed submissions. I think it's odd that webform uses Draft to mean "in progress" webform, vs say a revision of a webform. From what i've seen in the DB "draft" is applied to a submission for all sub-pages of a multiple page webform. I just think the term "draft" is somewhat overloaded in this context and somewhat vague.

2) What should happen if permission changes between sending the form to the browser and submitting the form?

I expect the formstate to not be lost and display "Submissions for this form are closed -- please contact site moderator". I assume the webform_submissions will retain everything upto the final form submit button press. A utility (admin page form table) for the site admin or webform admin to allow / disallow these drafts as exceptions to the rule would be nice -- but is un-needed.

Thanks for this awesome module.

Liam Morland’s picture

Maybe we need:

$closed = empty($node->webform['status']) && !webform_submission_access($node, $form['#submission'], 'edit');

In other words, it is only closed if the form status is closed and the user doesn't have permission to edit. But I think there is still a problem: A user who has permission to edit submissions should not be able to get around the form being closed when they are making their own submissions.

Is there a way that we can tell if the current submission action is editing a completed submission versus a user making their submission? If the former, we can bypass any open/closed checks.

1) Should there be different behavior for a draft went submitting via the node page versus the submission edit page?

Off the top of my head, I can't think of a reason for making it different.

2) What should happen if permission changes between sending the form to the browser and submitting the form?

The new permissions should be in effect.

DanChadwick’s picture

$closed = empty($node->webform['status']) && !webform_submission_access($node, $form['#submission'], 'edit');

Definitely not. If an admin closes a webform, no further changes should be possible.

Liam Morland’s picture

OK, what about by administrators? I thought the point of #2277593: REGRESSION: Administrators can no longer edit closed submissions is that administrators need to be able to edit submissions after the form is closed.

From my perspective here at work, we don't allow editing submissions, so what is troubling us is just the "Submissions for this form are closed" message.

Liam Morland’s picture

1) Should there be different behavior for a draft went submitting via the node page versus the submission edit page?

On second though, a user with permissions to edit submissions should be able to do so, but that shouldn't mean that they are still able to submit their own submissions after the form is closed. So, it should be different: On the node page, the rules that should apply are those that apply to everyone. On the submission edit page, the rules that should apply are the editing permissions.

  • DanChadwick committed 0381904 on 7.x-4.x
    Issue #2317273 by DanChadwick: Fixed insufficient privilege to submit a...
  • DanChadwick committed 0b88cea on 8.x-4.x
    Issue #2317273 by DanChadwick: Fixed insufficient privilege to submit a...
DanChadwick’s picture

Status: Needs review » Fixed
FileSize
1.08 KB

Fixed by allowing drafts to be submitted from node page. Committed to 7.x-4.x and 8.x

Liam Morland’s picture

Thanks. I think that solves the immediate problem, but still means that if the form is closed, people can still complete and submit their drafts. I think most administrators would expect that closed means no more submissions will happen, including no more drafts being finished.

The last submitted patch, 8: webform-forms_closed_msg-2317273-8.patch, failed testing.

DanChadwick’s picture

Status: Fixed » Needs work
Liam Morland’s picture

Status: Needs work » Fixed

The patch that failed is #8, which has already been committed, so now it can't be applied.

upperholme’s picture

I've kept up with this thread with interest, being the initiator of it.
However I'm now in the position of not being at all clear about where things stand. Can I now update my install and turn off the unwanted permission that allowed my users to edit their own submissions (this being the only way they could complete a multi-page form)?
Thanks for the prompt response to my original plea for help. Couldn't get things done with this great module.

DanChadwick’s picture

Status: Fixed » Needs work

Liam - I marked it as needs work because you are right -- if the form is closed it should have the message.

  • DanChadwick committed 607b56f on 7.x-4.x
    Issue #2317273 by DanChadwick: Fixed allowing submission of closed draft...
  • DanChadwick committed 0748399 on 8.x-4.x
    Issue #2317273 by DanChadwick: Fixed allowing submission of closed draft...
DanChadwick’s picture

Status: Needs work » Fixed
FileSize
1.59 KB

There is another bug triggered by this bug. If you have a form open and then attempt to submit a draft, and uninitialized variable, $allowed_roles, is passed to the theme function, raising a PHP notice. I'm putting this in this issue because while it is a different symptom, it is triggered by this same conditions as this issue and should be fixed at the same time.

Surely this has to be nearly a record for the most comments for the "simplest" fix!

Committed to 7.x-4.x and 8.x.

Liam Morland’s picture

Liam Morland’s picture

tenken’s picture

Sorry. I'm still seeing this message on my webform. Except that I get 5 pages into my multi-page webform. I'm trying to examine the issue locally -- I can see that the preceding page has a conditional validation (a single T/F question) and after successfully answering the step I get "Submissions for this form are closed."

Is there any debug information I can provide to help resolve the issue?

Liam Morland’s picture

@tenken Do you still see this after installing the latest development version of Webform? Today's commit may fix your problem.

tenken’s picture

@liam yes I saw the -dev update from around 6:30am today and run an upgrade attempt using that version of the module -- I've even checked the lines of code in the 7.x patch are live on the system prior to visiting the page url.

Liam Morland’s picture

Can you make steps to reproduce the problem with a simple form?

tenken’s picture

i'll try after my lunch break which begins shortly.

DanChadwick’s picture

@Liam -- Let me think this through with you. It is obviously much trickier than one might think. The test in prevalidate is to determine whether the form should proceed with submission. The code in _submission_access is to determine whether the given page should be displayed at all.

There is no problem with whether the page is displayed. The problem is only whether submission should be allowed. Reading _submission_access is helpful. Here it is for reference:

function webform_submission_access($node, $submission, $op = 'view', $account = NULL) {
  global $user;
  $account = isset($account) ? $account : $user;

  $access_all = user_access('access all webform results', $account);
  $access_own_submission = isset($submission) && user_access('access own webform submissions', $account) && (($account->uid && $account->uid == $submission->uid) || isset($_SESSION['webform_submission'][$submission->sid]));
  $access_node_submissions = user_access('access own webform results', $account) && $account->uid == $node->uid;

  $general_access = $access_all || $access_own_submission || $access_node_submissions;

  // Disable the page cache for anonymous users in this access callback,
  // otherwise the "Access denied" page gets cached.
  if (!$account->uid && user_access('access own webform submissions', $account)) {
    webform_disable_page_cache();
  }

  $module_access = count(array_filter(module_invoke_all('webform_submission_access', $node, $submission, $op, $account))) > 0;

  switch ($op) {
    case 'view':
      return $module_access || $general_access;
    case 'edit':
      return $module_access || ($general_access && (user_access('edit all webform submissions', $account) || (user_access('edit own webform submissions', $account) && $account->uid == $submission->uid)));
    case 'delete':
      return $module_access || ($general_access && (user_access('delete all webform submissions', $account) || (user_access('delete own webform submissions', $account) && $account->uid == $submission->uid)));
    case 'list':
      return $module_access || user_access('access all webform results', $account) || (user_access('access own webform submissions', $account) && ($account->uid || isset($_SESSION['webform_submission']))) || (user_access('access own webform results', $account) && $account->uid == $node->uid);
  }
}

^^^ holy crap, that doesn't wordwrap well.

I think the logic should be:

closed ::= 
  is a submission AND it's not a draft AND not allowed to edit the submission i.e _access('edit')
  OR
  webform is closed AND not allowed to access all webform results

Does this cover it?

Liam Morland’s picture

I think the problem can only be fixed by changing more than just how $closed is defined. If one is completing the form on the form page, then $closed should depend only on the open/closed setting set for the Webform. Even an administrator with full editing rights should not be able to submit a closed form.

But an administrator with access to edit submissions should be able to edit and submit forms form the results tab regardless of the open/closed setting. In this case, the checks in webform_client_form_prevalidate() are not valid. Even if submission limits have been met, for example, the administrator should be able to edit a submission.

I think in the place where the form is displayed for editing on the results tab there needs to be a hidden field added. This would allow bypassing webform_client_form_prevalidate().

BTW: I will be on vacation for a week starting in a few minutes with minimal or no Internet connectivity.

DanChadwick’s picture

@Liam - Have a good vacation. Your comments are helping me make sense of this.

Let's look at these prevalidation checks one-by-one:

  1. Allowed role. This is checked only when the webform isn't finished. UID 1 is always allowed. Otherwise the user must always have an allowed role. I think this is correct. If you want an admin to submit or edit webforms, give him/her the role.
  2. Total limit. This is checked only if the webform isn't finished. The admin is subject to the total submission limit for adding new submissions, but not for editing them. I think this is correct as the code is.
  3. User limit. This too is checked only if the webform isn't finished, so like the above, I think this is correct as is.
  4. Closed. This is the tricky part. Let's discuss this more below.

The checks in prevalidate are there to protect against a stale form submission, such as when form status changed since the form was built. You can't depend upon having the form rebuild upon the form POST because it may come from the cache. An easy way to test this stuff is to use the browser back button or to make changes to the webform form settings in a different browser, then click Submit or Save Draft or whatever in the first browser.

The results tab cannot add new submissions, it can only edit them. This is the same as someone clicking the submission's EDIT tab.

Let's discuss when the webform is open. What should stop the user? If the page fails the view (node/NID) or edit (node/NID/submission/SID/edit) access routine, you will get an Access Denied and not get to prevalidate. (I want to test this to confirm.) I can't think of a reason why the validation should be fail the user. Draft? That's fine. New submission? That's fine. Editing a finished submission? That's fine (because we know that the access routine already passed).

So that leaves when the webform is closed. What should stop the user? An admin with edit all submissions permission should be allowed to edit or submit an existing draft or finished submission. I'm thinking that no one else should, even if they have permission to edit their own submissions. Closing the form should apply to everyone except at admin.

If this is right, it implies that

closed := 
  webform is closed 
  AND either 
  no submission yet OR no access to edit all submissions

or maybe more clearly:

open ::=
  webform is open
  OR
  there is a submission AND the user has access to edit all submissions

This does imply that if an admin is in the middle of creating a new submission from the node's page and another admin closes the webform, they can only complete the webform if a draft has been started. Seems okay to me.

I welcome comments from anyone, especially quicksketch.

DanChadwick’s picture

Status: Fixed » Needs work
DanChadwick’s picture

Status: Needs work » Fixed
FileSize
2.46 KB

This patch makes four changes:

  1. $form['#node'] is refreshed from the database because it could have been changed by an admin while the form was cached. In specific, it might not have the current webform open/closed status.
  2. The test for prevalidating if a form is closed is as per #40.
  3. A form that failed pre-validation is not validated; there is no point is showing messages that the user can't fix.
  4. The code that prevents a form that failed pre-validation from being auto-saved was corrected. It was using the test isset($errors['']), but the value of $errors[''] is null, so isset will always be false. key_exists() is used instead.

Committed to 7.x-4.x and 8.x

  • DanChadwick committed ba3a4a0 on 7.x-4.x
    Issue #2317273: Fixed preventing editing of a recently-closed stale form...
  • DanChadwick committed 0b2f2fa on 8.x-4.x
    Issue #2317273: Fixed preventing editing of a recently-closed stale form...
tenken’s picture

FileSize
40.11 KB

Can I get some clarification on this fix please.

Prior to my updating to webform-4.x-dev only "eap: staff" had any values set for any webform permissions. With 4.x-dev-rc6 if I do not have "Access own webform submissions" and "Edit own webform submissions" -- then my non-administrator user roles see "Submissions for this form are closed." when attempting to submit a webform with 31 pagebreaks.

  • Can you please clarify what each of these 2 permissions do. Put another way, in my attached screenshot can the Permissions page please have a #description added for these 2 permissions?

How does "Access own webform results" differ from "Access own webform submissions" -- rhetorically is it not a completed submission until the last page/component is reached in the webform?

I essentially want students to be able to submit webforms -- but not make edits to them later, or even see them necessarily.

I attempted to make a small 5 page example with a single webform_validation component -- but webform-rc6 and webform_validation-1.5 or 1.6-rc2 dont seem to notice the validation component at all, so my attempt at creating a testcase to help debug kinda poofed on me ...

/var/www/2014/debug/webform4xtest/www/sites/all/modules > drush pml --package=webform
 Name                                     Type    Status   Version     
 Webform (webform)                        Module  Enabled  7.x-4.0-rc6 
 Webform Validation (webform_validation)  Module  Enabled  7.x-1.6-rc2 
DanChadwick’s picture

Access own webform results give access to the RESULT tab, but limited to one's own submissions. Access own submissions gives access to the VIEW tab on a particular submission, as well as the list of submissions.

tenken’s picture

Thank you for the clarification.

I'm not sure then why both area needed to proceed-thru and submit a Submission from /node/NID as any user. I've also tried permutations of these 2 permissions values, submissions are only allowed if both sets of permissions are granted to the user role.

I'll try again to create a simple canned example, or repeatable steps, but my last attempt met with failure when trying to use the latest webform_validation.

Thanks for your time.

DanChadwick’s picture

I can't reproduce this. If I deny authenticated users any webform permissions, I can still log in as a non-admin and submit a multi-page webform via the node/NID page. I cannot review my submissions after (i.e. node/NID/submissions is access denied).

I'm not sure what you are seeing or what you are trying to accomplish.

Status: Fixed » Closed (fixed)

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

tenken’s picture

Just a friendly fyi. In my case I had webform installed via a a profile in /profiles/MYPROFILE/modules/contrib/webform ... and at some point had installed /sites/all/modules/webform as well. This update applies cleanly, and removing the un-needed sites/all/modules webform installation causes the webform to function properly.

When a module is installed in via a profile vs "sites all" i'm not sure which wins out on page render -- I believe maybe the sites/all at this time.

I simply wanted to provide clarification on how my issue was resolved in case others see a similar issue and have a similiar module (site setup) mishap.