A bug with the use of db_query() breaks the module for Webform when no sid is given. Patch attached.

CommentFileSizeAuthor
#5 fillpdf_webform_query-D6.patch879 bytesliam morland
#3 fillpdf_webform_query.patch894 bytesliam morland
fillpdf_webform_query.patch894 bytesliam morland

Comments

wizonesolutions’s picture

wizonesolutions
he/him
Primary language English
Location Norway
commented
Status: Needs review » Needs work 
+++ b/fillpdf.moduleundefined
@@ -252,9 +252,9 @@ function fillpdf_merge_pdf($fid, $nids = null, $webform_arr = null, $sample = nu
+      if (empty($webform['sid'])) { // user didn't specify submission-id, meaning they want most recent

Let's make the comment:

User did not specify submission ID, meaning they want most recent.

+++ b/fillpdf.moduleundefined
@@ -252,9 +252,9 @@ function fillpdf_merge_pdf($fid, $nids = null, $webform_arr = null, $sample = nu
+          where nid = :nid and uid = :uid order by submitted desc', array('nid' => intval($webform['nid']), 'uid' => intval($user->uid)))->fetchField();

The array keys should be :nid and :uid, just like the placeholders in the query.

Can we use (int) instead of intval()? Cleaner and does same thing.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

The array keys should be :nid and :uid, just like the placeholders in the query.

It works with or without the colon in the array keys. Since placeholders always start with colon, it seems redundant to include them. The Drupal Coding Standards don't seem to provide a standard. I'm happy to go with whatever you prefer.

Can we use (int) instead of intval()? Cleaner and does same thing.

Fine with me.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Needs work » Needs review
StatusFileSize
new fillpdf_webform_query.patch894 bytes

Updated patch with your changes included. I note that the colons in the placeholders are there in core when this function is called.

wizonesolutions’s picture

wizonesolutions
he/him
Primary language English
Location Norway
commented
Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Needs review » Patch (to be ported)

I'm happy with this. Committed.

Does this need backport to 6.x?

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new fillpdf_webform_query-D6.patch879 bytes

D6 version attached.

wizonesolutions’s picture

wizonesolutions
he/him
Primary language English
Location Norway
commented
Status: Needs review » Needs work 
+++ b/fillpdf.moduleundefined
@@ -220,9 +220,9 @@ function fillpdf_merge_pdf($fid, $nids = null, $webform_arr = null, $sample = nu
+          WHERE nid = :nid AND uid = :uid ORDER BY submitted DESC', array(':nid' => (int) $webform['nid'], ':uid' => (int) $user->uid)));

You sure these query placeholders work in 6?

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

No; I don't know if they work or not. I don't have a D6 site.

alexborsody’s picture

alexborsody commented

I will test this and if it works commit it

  • Commit 67a960c on 7.x-1.x, 7.x-2.x, 7.x-2.x-tests1, 7.x-1.x-ubercartbackporttest authored by Liam Morland, committed by wizonesolutions: 
    Issue #1356304: Fix default submission detection.

wizonesolutions’s picture

wizonesolutions
he/him
Primary language English
Location Norway
commented
Issue summary: View changes
Status: Needs work » Fixed
Issue tags: -

None of the D6 maintainers seem to be active, and this is more than a year old; closing.

Status: Fixed » Closed (fixed)

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