I am seeing this message when creating an election:

Warning: strtotime() expects parameter 1 to be string, array given in ElectionDefaultController->save() (line 26 of .../modules/election/includes/election-default.controller.inc).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Liam Morland created an issue. See original summary.

pjcdawkins’s picture

OK, this is probably because of an unexpected output from the date widget... I'll have a look. Also this handling should be in the form submit function, it's too magic to be in the controller.

pjcdawkins’s picture

This moves the logic out of the controller, but I don't know the cause of your issue yet.

Do you have the date_popup module enabled? Have you modified the election form via ...form_alter at all?

Liam Morland’s picture

Status: Active » Needs work

The patch does not fix it. We are using the date_popup module, so the $value is an array of date and time. Setting the schedule still works.

Liam Morland’s picture

Status: Needs work » Needs review
FileSize
668 bytes

This patch converts array to string before strtotime() is called. Array to string conversion code is borrowed from election_form_validate().

pjcdawkins’s picture

Status: Needs review » Needs work
+++ b/includes/election-default.controller.inc
@@ -23,6 +23,9 @@ class ElectionDefaultController extends EntityAPIController {
+          $value = $value['date'] . ' ' . ($value['time'] ? '00:00' : $value['time']);

It looks like the logic is backwards in the ternary conditional: there should perhaps be an empty() around $value['time']

Liam Morland’s picture

Thanks. Corrected version attached.

This raises a question about election_form_validate() which contains the same backwards logic several times.

pjcdawkins’s picture

OK, here's something which also fixes election_form_validate(), via a separate function

Liam Morland’s picture

This should probably be in a child issue. One comment: election_normalize_datetime() can return 0 if the date/time is empty. But 0 is a valid timestamp. Perhaps it should return NULL in this condition.

Liam Morland’s picture

Here is a version of your patch in #8 with minor adjustments to election_normalize_datetime().

Status: Needs review » Needs work

The last submitted patch, 10: election-strtotime_array-2636366-10.patch, failed testing.

The last submitted patch, 10: election-strtotime_array-2636366-10.patch, failed testing.

Liam Morland’s picture

Status: Needs work » Needs review
pjcdawkins’s picture

+++ b/election.module
@@ -932,3 +932,30 @@ function _election_pathauto_update_alias_multiple(array $election_ids, $op, arra
+  elseif ((string) $datetime === '') {

I wonder if we should just return FALSE early, if empty($datetime)?

Liam Morland’s picture

Do you mean adding something like this to the top of election_normalize_datetime():

if (empty($datetime)) {
  return FALSE?
}

That is probably OK in practice, though technically zero is a valid datestamp and it would cause it to return false for that.

Liam Morland’s picture

Or the empty string check could be done first. The would cause many empty() value to return FALSE right away.

Liam Morland’s picture

Integer dates are probably more common, so the current order is probably best.

pjcdawkins’s picture

I'd find it unlikely that anyone is scheduling an election for 1970-01-01 00:00:00. But you're right that 0 should return 0. So I'd say this is good as-is.

A unit test would be nice.

  • Liam Morland committed 8255d8f on 7.x-1.x
    Issue #2636366: Create and use _election_normalize_datetime().
    
Liam Morland’s picture

Assigned: Unassigned » Liam Morland
Status: Needs review » Fixed

Complete with unit test.

Status: Fixed » Closed (fixed)

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