I am using the fullcalendar_view module. With that, it provides a field for having events repeat each week. And that is a dropdown list of values 0-6 to represent Sun-Sat.

I was able to get prepopulate working with this field.
?edit[field_weekly_event][widget]=4
would set the field to Thursday as expected. In fact, it worked for all days except Sunday which has a value of 0.

So this
?edit[field_weekly_event][widget]=0
would not work. It would cause the passed in variable of $form and $request_slice not to work with each other and thus not set anything.

After some digging, I tracked this down to a logic issue in src/Populate.php in a recursive function named populateForm. There was an if statement that read the 0 value causing it to return false when that wasn't the intent of the if. I rewrote that if statement to better handle the NULL check and in a way that I felt was more readable. It now works for me on Sunday and the other days. Hoping folks can confirm this is a good fix. I'll post my patch file shortly.

CommentFileSizeAuthor
#16 3060341-16.patch5.36 KBheddn
#16 3060341-16_test_only.patch4.72 KBheddn
#14 prepopulate-fix_zero_parameter-3060341-14.patch5.36 KBeahonet
#13 prepopulate-fix_zero_parameter-3060341-13.patch5.35 KBeahonet
#12 prepopulate-fix_zero_parameter-3060341-12.patch5.35 KBeahonet
#11 prepopulate-fix_zero_parameter-3060341-11.patch3.03 KBeahonet
#9 prepopulate-fix_zero_parameter-3060341-9.patch3.04 KBeahonet
#8 prepopulate-fix_zero_parameter-3060341-8.patch2.18 KBeahonet
#5 prepopulate-fix_zero_parameter-3060341-5.patch2.17 KBeahonet
#2 prepopulate-fix_zero_parameter-3060341-2.patch660 byteseahonet

Comments

eahonet created an issue. See original summary.

eahonet’s picture

eahonet commented
StatusFileSize
new prepopulate-fix_zero_parameter-3060341-2.patch660 bytes

Here's the patch I made. I hope I made it correctly.

eahonet’s picture

eahonet commented
Status: Active » Needs review
heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Could we add some test coverage of this?

eahonet’s picture

eahonet commented
StatusFileSize
new prepopulate-fix_zero_parameter-3060341-5.patch2.17 KB

I can try creating a test. This is my first attempt at modifying the tests and I wasn't able to determine a way to test it on my environment.

eahonet’s picture

eahonet commented

I welcome guidance on how to properly setup the tests for this. The code change is sound, but I obviously am missing something on the testing code. I'm not sure how to test locally, and don't want to spam d.o with tests.

If someone is running into this issue right now, #2 has a solution.

heddn’s picture

heddn
Primary language English
Location Illinois
commented 
+++ b/tests/modules/prepopulate_test/config/install/field.storage.node.field_select_list_numeric.yml
@@ -0,0 +1,29 @@
+      value: 0
+      label: 0
...
+      value: 1
+      label: 1
...
+      value: 2
+      label: 2

Try with single quotes around these values i.e. '0', etc.

eahonet’s picture

eahonet commented
StatusFileSize
new prepopulate-fix_zero_parameter-3060341-8.patch2.18 KB

Trying an updated patch with single quotes around the numbers. Thank you @heddn

eahonet’s picture

eahonet commented
StatusFileSize
new prepopulate-fix_zero_parameter-3060341-9.patch3.04 KB

Realized I was missing a file. Going to try without single quotes around value too.

heddn’s picture

heddn
Primary language English
Location Illinois
commented 
+++ b/tests/modules/prepopulate_test/config/install/field.storage.node.field_select_list_numeric.yml
@@ -0,0 +1,29 @@
+      label: '0'
...
+      label: '1'
...
+      label: '2'

The label should be what you have in the expectations, "Select List Numeric 0", etc.

eahonet’s picture

eahonet commented
StatusFileSize
new prepopulate-fix_zero_parameter-3060341-11.patch3.03 KB

Ah. So perhaps having the ' around the label caused the output to be different. Modeling this off of the field_select_list and originally I had missed the field definition file. So going back to no single quotes and with the correct definition file (all modeled off of field_select_list test). Trying again. Thank you for patience @heddn.

eahonet’s picture

eahonet commented
StatusFileSize
new prepopulate-fix_zero_parameter-3060341-12.patch5.35 KB

Found two other files that had field_select_list but not my field_select_list_numeric. Trying new test included patch.

eahonet’s picture

eahonet commented
StatusFileSize
new prepopulate-fix_zero_parameter-3060341-13.patch5.35 KB

Changing type from list_string to list_integer

eahonet’s picture

eahonet commented
StatusFileSize
new prepopulate-fix_zero_parameter-3060341-14.patch5.36 KB

Changing 0 to Zero and adjusting the test php file.

eahonet’s picture

eahonet commented
Status: Needs work » Needs review

Well, that was an embarrassing level of patch submits to get the test working. But changing to "Needs review" now that my test works.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
StatusFileSize
new 3060341-16_test_only.patch4.72 KB
new 3060341-16.patch5.36 KB

Adding a test only patch to see the failures. This is coming together quite nicely. Thanks for your efforts.

The last submitted patch, 16: 3060341-16_test_only.patch, failed testing. View results

  • heddn committed 82a85a8 on 8.x-2.x 
    Issue #3060341 by eahonet, heddn: Issue when value passed is "0" zero
heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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