Purpose

  • Support empty time range values

Designed User Experience

  1. A user has already configured a field for time range and is editing content (in my example "event")
  2. When editing the time range field sometimes end times are not known so the user should be able to enter only a start time
  3. Once the user clicks save only the start time should show

Possible Bug Fixed

  • Currently when the user clicks save with only the start time the current time is displayed, this issue will fix this behavior and support empty values
CommentFileSizeAuthor
#26 time-error.png16.9 KBjan-e
#10 time_field-support-empty-time-range-3074674-10.patch3.7 KBgeneralredneck
#5 support-empty-time-range-3074674-5.patch3.93 KBccjjmartin
#2 support-empty-time-range-3074674-2.patch3.74 KBccjjmartin

Issue fork time_field-3074674

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

ccjjmartin created an issue. See original summary.

ccjjmartin’s picture

ccjjmartin commented
StatusFileSize
new support-empty-time-range-3074674-2.patch3.74 KB
ccjjmartin’s picture

ccjjmartin commented

I am thinking there are probably better ways to handle this then picking the number 86401 (one second after 24 hours). I would love some feedback from the community on other approaches here.

ccjjmartin’s picture

ccjjmartin commented
Status: Active » Needs review
ccjjmartin’s picture

ccjjmartin commented
StatusFileSize
new support-empty-time-range-3074674-5.patch3.93 KB
ccjjmartin’s picture

ccjjmartin commented

Added another possibility which is to allow the time separator to be removed when only a single time is displayed.

labboy0276’s picture

labboy0276 commented
Status: Needs review » Reviewed & tested by the community

This works well when you don't have an end time. Thank you for this.

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Needs work

Care to elaborate the int 86401 ?

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented
Version: 8.x-1.x-dev » 2.x-dev
generalredneck’s picture

generalredneck
him/his
Primary language English
Location Texas, USA 🇺🇸
commented
StatusFileSize
new time_field-support-empty-time-range-3074674-10.patch3.7 KB

Rerolling the patch done by ccjjmartin. I know that it works but I don't understand all the logic either.
86401 is Friday, January 2, 1970 12:00:01 AM GMT. I'm not sure what the need is there but probably could help figure that out at some time. Likely some random value that will never be used that's in the past that can be validated against since the date widget select something?

ccjjmartin’s picture

ccjjmartin commented

@generalredneck I picked a value that would not normally be used. So 60 seconds * 60 minutes * 24 hours = 86400 seconds in a day.

I then added 1 to that value so that I knew it wasn't a valid value but could still be referenced. Probably better ways to do it but that was the original thought process.

nickdickinsonwilde’s picture

nickdickinsonwilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
commented
Status: Needs work » Reviewed & tested by the community

Works good and I think the 86401 is a reasonable value

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented
Issue tags: +Needs tests

The only thing we are missing now is an automated test to verify this functionality :)

nagy.balint’s picture

nagy.balint commented

I also needed this patch.

It seems to work fine!

xaa’s picture

xaa commented

hi, thank you generalredneck. It seems I also need patch in #10 after a d9.4.8 > d9.5.11 update, possible?

When trying to access a node the patch from #10 solves the error

"InvalidArgumentException: Provided value is out of range. in Drupal\time_field\Time::assertInRange() (line 64 of modules/contrib/time_field/src/Time.php)".

The node edit page was also broken (impossible to access/edit the node). Should I open a new ticket for this report ? (as it may need more attention and a higher priority level as the error fully broke the access to the /node/xx/edit page).

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented
Assigned: ccjjmartin » bramdriesen

I'll be trying to make the tests

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Needs work

Just noticed that this patch actually completely bypasses the field validation. If you set the field as required you can simply save without entering any value at all...

BramDriesen opened merge request !9

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented
Status: Needs work » Needs review

Added kernel test

  • BramDriesen committed 80005a0b on 2.x 
    Issue #3074674 by BramDriesen, ccjjmartin, generalredneck,...
bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented
Assigned: bramdriesen » Unassigned
Status: Needs review » Fixed
Issue tags: -Needs tests

Status: Fixed » Closed (fixed)

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

jan-e’s picture

jan-e commented

After the update from 2.1.0 to 2.1.1 I am getting this error during a cron:

InvalidArgumentException: Provided value is out of range. in Drupal\time_field\Time::assertInRange() (line 64 of modules/contrib/time_field/src/Time.php).

A few lines of the stack trace:

#0 modules/contrib/time_field/src/Time.php(145): Drupal\time_field\Time::assertInRange('86401', 0, 86400)
#1 modules/contrib/time_field/src/Plugin/Field/FieldFormatter/TimeFormatter.php(36): Drupal\time_field\Time::createFromTimestamp('86401')
#2 modules/contrib/time_field/src/Plugin/Field/FieldFormatter/TimeFormatter.php(47): Drupal\time_field\Plugin\Field\FieldFormatter\TimeFormatter->viewValue(Object(Drupal\time_field\Plugin\Field\FieldType\TimeType))

Looks like the same error as in #15. I am running Drupal core 10.2.3. And reverted the update to 2.1.1 for now.

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented

@Jan-E What is the value you're trying to submit?

jan-e’s picture

jan-e commented

@BramDriesen I really would not know. It also happened when a cron was invoked from outside the site.

jan-e’s picture

jan-e commented
StatusFileSize
new time-error.png16.9 KB

This was the first time it happened, on a time that almost nothing was done on the site:

time out of range

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented

I've been testing this for a bit but I can't seem to reproduce it :/ if you would happen to be able to debug a bit and see what the values are/where they come from that would help.

jan-e’s picture

jan-e commented

@BramDriesen thanks for getting back on this.

I just had this once again. Somehow the exact value 86401 gets entered in a time field. When cron runs it apparently tries to clear up the entity cache and falls into the error.

The previous occasion I remedied it by manually deleting the field instance and the revision instance from the database. I will try to change the value into 86400 now. And will try to find out how users manage to enter 86401.

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented

I think there is someone else with a similar issue in #3423941: After 2.1.1 update all time fields are treated as required; empty values not accepted.