Time should not be saved if time is disabled on Date field

[duncanmcclean]

This pull request resolves #2497, where the time would always be saved, even if the time_enabled config setting was set to false.

The issue seems to have come from the preProcess function of the Date fieldtype where it was formatting the date value to one that can then be understood by Moment on the frontend.

Essentially, it was just a case of adding a ternary to check if time is enabled, if yes the format will be Y-m-d H:i, otherwise it will be Y-m-d.

[robdekort]

Nice. That seems to do the trick just fine! Saves without time when time_enabled = false and with when it's true.

[sjclark]

YES! @damcclean you're the hero we need. Really appreciate you taking the time to look into this! ❤️

[duncanmcclean]

YES! @damcclean you're the hero we need. Really appreciate you taking the time to look into this! ❤️

You are very welcome!

[sjclark]

Hmmmm maybe I spoke to soon, I'm still having the time saved on a field
(this field is just at top level, although having the same issue on a deeper level)

  -
    handle: datetest
    field:
      mode: single
      time_enabled: true
      time_required: false
      earliest_date: '1900-01-01'
      full_width: false
      inline: false
      columns: 1
      rows: 1
      display: DateTEST
      type: date
      icon: date
      listable: hidden

Opening, reveals:

Results in:
datetest: '2021-01-21 00:00'

Naturally theres also no way to clear the date, likely due to this bug:
#2920

As far as I can tell theres still no differentiation between time enabled & time required?

[duncanmcclean]

In the field you shared, you have the time enabled so I'd expect the time to also save.

But basically what you'd like is if there's no time provided in your date and time required is set to false, it should just save the date and no time?

[sjclark]

@damcclean thanks for your help!

In the field you shared, you have the time enabled so I'd expect the time to also save.

Surely time enabled means that the option to use time is presented, not that its mandatory and will save. I'd expect that to be the option to save, not that you have to save?

But basically what you'd like is if there's no time provided in your date and time required is set to false, it should just save the date and no time?

Essentially I have a date field where the time is sometimes required.

Surely the point of the time enabled versus time required is exactly that situation?

Its totally possible I'm just missing the point of those options?

I'm fairly sure the previous behaviour (in v2) was to show a date field with an "Add Time" button? (possible I'm imagining that?) EDIT: below is the behaviour from v2 - it would show an optional button to add time when you had it enabled

I'm sure I remember an add time button at some point (maybe earlier in v3?)

[duncanmcclean]

Yeah, it makes sense, I think I just forgot about that use case.

I'll have a look at PR'ing the code to support the time required setting (after work).

[robdekort]

Oh nice find. Didn't use it either (yet). But def. makes sense to me.

[sjclark]

Yeah, it makes sense, I think I just forgot about that use case.

I'll have a look at PR'ing the code to support the time required setting (after work).

Amazing! No time stress whatsoever from me! I just appreciate you taking the time to look at it - is all over my head!