First thing is if you try to save node with required scheduler field with a blank time field then you will have same error:

The value input for field Unpublish on is invalid:
The value 2012-09-26 does not match the expected format.

We are really didn't use 'Time' field, because of this I tried to hide it into scheduler settings:

But I have got an error:

Comments

mrded’s picture

Version:7.x-1.0» 7.x-1.x-dev
mrded’s picture

Title:Change date format» Change date format (hide Time field)
mrded’s picture

StatusFileSize
new1.85 KB

Please, check my patch.

gaheinrichs’s picture

I checked your patch. It work for me really well. Thanks!

mrded’s picture

Status:Active» Reviewed & tested by the community
jonathan1055’s picture

Status:Reviewed & tested by the community» Needs work

Hello,
The first part of the patch looks good, ie only validate the time format if some time letter elements have been entered.

But I am not sure about the second part, when you say the $date_format is valid if it matches SCHEDULER_DATE_FORMAT or the hard-coded value 'Y-m-d'. If you need to use 'Y-m-d' then that value should be allowed in SCHEDULER_DATE_FORMAT.

Or maybe I am missing the point. But I do not think that Eric would like to hard-code a vaidation value, there should be a clearer way to do it.

Good work, though. This does need to be changed. Haven't we seen this issue or something like it before?

Jonathan

jonathan1055’s picture

Hi,
I have looked at this in more detail, and I think there might be a better solution. There should be no reason why we cannot set 'Y-m-d' as the scheduler format setting and still use date_popup with no trouble. The value is set in #date_format in the popup field settings and everything should just flow through. However, I think the reason it was not done like that in Scheduler is due to a slight bug in the date_pop module where it always returns the string in its own standard format Y-m-d H:i:s regardless of what you have requested in #date_format. So in scheduler we had to use exactly that format. You were managing to get away with it by luck because your date format matched the first part of this.

I have raised an issue with date_pop #1855810: Define #return_value_format for date_popup to set the form value. If that is resolved then we can simply allow Y-m-d as the value of scheduler_date_format for both popup and non-popup options and not have to reset date_format in _scheduler_strtotime(). This will also be more flexible because it will allow others formats for the date part of the datetime, eg d-m-Y H:i:s which currently would fail in scheduler (so we do not allow it).

This will also simplify some of the code where we use $internal_date_format but that can wait until later.

Thanks for starting this discussion. I hope that the date_popup issue can be implemented (it is only a one-line correction) then we'll code the scheduler change.

Jonathan

jonathan1055’s picture

When we have committed #1069668: Default time with user override then the solution for removing the time entry box as requested above is almost trivial. It only requires your very first change:

-    if (!in_array($time_format, $acceptable)) {
+    if (!empty($time_format) && !in_array($time_format, $acceptable)) {
       form_set_error('scheduler_date_format', t('The Date Popup module only accepts the following formats: !formats', array('!formats' => implode($acceptable, ', '))));
     }
   }

Two additional benefits of this combined solution are (1) the Admin can set the default time which does not need to be 00:00:00 and (2) we do not need to wait for the date_popup fix #1855810: Define #return_value_format for date_popup to set the form value.

When making this change, we could also improve the text of the validation message, by add 'time' in front of 'formats'

Jonathan

jonathan1055’s picture

Title:Change date format (hide Time field)» Allow for no time field on date popup
Issue summary:View changes
Status:Needs work» Closed (duplicate)
Related issues:+#2123103: Allow more popup formats, +#1069668: Default time with user override, +#1855810: Define #return_value_format for date_popup to set the form value

The code change above to cater for empty time formats was included in #1069668: Default time with user override hence this issue can be closed as a the functionality is now available.

Also see #2123103: Allow more popup formats which further enhances the flexibility of times in date_popup.