Hi, I'm using Scheduler module version 8.x-1.0-rc2 on Drupal 8.3.7.

When I check the 'Allow users to enter only a date and provide a default time' option and try to create a new content which has scheduler configured (in my case request just an unpublish date), the time field is not been populated with the default time, however, you can see in the message that the default time it has been set:

Enter a date. The time part is optional. The default time is 00:00:01.

(been 00:00:01 my default test time).

Trying to debug this I got to the method /src/Plugin/Field/FieldWidget/TimestampDatetimeNoDefaultWidget->valueCallback, the definition for this method is

public static function valueCallback(&$element, $input, FormStateInterface $form_state)

however, the $input variable is empty, that's why it doesn't pass the validations and it's never set. If I try to set the default time value for this field I also need to define a default date value which should be the value entered by the user, but since $input is empty I have no place to grab this value from (maybe from $form_state? but this doesn't seem to be right).

I also test the module in simplytest.me and got the same results. Form this point I'm not sure how to proceed to keep debugging this.

How to reproduce:

  • Check the 'Allow users to enter only a date and provide a default time' option and define a default time.
  • Create a new content which the scheduler option set.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

eworwa created an issue. See original summary.

eworwa’s picture

Issue summary: View changes
jonathan1055’s picture

Hi eworwa,
Sorry to hear you are having a problem. However, I cannot replicate it. Using 8.3 and 8.4, with Chrome and Firefox, it works OK. Also our automated testing passes the 'default time' test. So in principle it is working. Now, we need to discover what combination of things is causing your problem.
Jonathan

giordy’s picture

I have the same problem.

Thanks

gsquirrel’s picture

Also have same problem it just says "please fill in this field".

gsquirrel’s picture

I discovered that this for me is connected with the field being "required" in the settings for content type. If I untick "Require scheduled unpublishing" then it works and sets a default when only a date is selected.

When the field is required the validation prevents you from submitting the form without both a date and a time.

jonathan1055’s picture

Title: Allow users to enter only a date and provide a default time option not working » Conflict between allowing default tine and requiring the scheduled date
Version: 8.x-1.0-rc2 » 8.x-1.x-dev

Ah, thanks gsquirrel, that is useful information. Yes I can replicate this error now. It is strange because I am sure this was working before - it is such a basic feature it would be surprising for this bug not to have been found before. I know that browsers implement the HTML5 date widget differently and developers now have reduced access to manipulate the data. I recall trying to set the default time into the field when the form is first shown, but I think it failed because just entering a time was not allowed.

I will try again to find a solution for this.

RebeccaPossible’s picture

Any update on this issue? I still get the required message even though time is optional.

baddeed’s picture

Please fix it. It will be my life saver.

Stockticker’s picture

Assigned: Unassigned » Stockticker
Issue tags: +LutskGCW19
Stockticker’s picture

FileSize
2.87 KB

I've got the same issue on one of my projects and decided that it would be a good idea to populate default time on a frontend if those requirements are met:

  • publishing/unpublishing is set required
  • default time is set
  • time field is empty (relevant for new nodes)

Should work both for vertical tabs and a fieldset.
Please, take a look and test.

Stockticker’s picture

Assigned: Stockticker » Unassigned
Status: Active » Needs review
jonathan1055’s picture

Thanks Stockticker. Yes, using jquery to pre-populate the field might work. Before HTML5 we had the ability to set the default time, but when moving to Drupal8 we lost that. But your idea is good.

jonathan1055’s picture

Title: Conflict between allowing default tine and requiring the scheduled date » Conflict between allowing default time and requiring the scheduled date
FileSize
4.71 KB

Followig my work on #3126644: Create SchedulerSetupTrait for common test start-up processing this new javascript can have test coverage. Here's a patch with just the new test, not the new functionality so this should fail.

jonathan1055’s picture

Tests ran OK locally.
But WebdriverTestBase should be WebDriverTestBase so this could have caused the 'class not found' error.

jonathan1055’s picture

The new tests were not run. Need to add to the drupalci.yml:

+      run_tests.js:
+        types: 'PHPUnit-FunctionalJavascript'

Status: Needs review » Needs work

The last submitted patch, 16: 2913829-16.javascript-test-only.patch, failed testing. View results

jonathan1055’s picture

Status: Needs work » Needs review
FileSize
10.46 KB

That's good, the tests run and fail as expected. Patch #18 includes the new js file and functionality as is Stockticker's patch #11

Status: Needs review » Needs work

The last submitted patch, 18: 2913829-18.javascript-test-only.patch, failed testing. View results

jonathan1055’s picture

Status: Needs work » Needs review
FileSize
10.46 KB

Ah, that's the problem. Locally the tests run fine, but my Chromedriver calendar pop-up is set to accept format d/m/Y which is the default for my location (UK). I think the drupal.org testsbot, because it runs in Australia/Sydney timezone, is expecting m/d/Y.

For a user in Sydney timezone
1607023800 = Fri 04-Dec-2020 - 06:30:00 = 04/12/2020 = actual
1586896200 = Wed 15-Apr-2020 - 06:30:00 - 15/04/2020 = expected

I have now switched the format in the javascript test to be m/d/Y, and locally I now get the failures Failed asserting that 1607023800 matches expected 1586896200. which is exactly the error on d.o. Patch #20 may pass on drupal.org but clearly the test needs work as it should not depend on what location the browser thinks it is in.

jonathan1055’s picture

I've added a way to work out which HTML5 datepicker format is being used in the test. These run sucessfully locally, which uses d/m/Y. Testing on drupal.org needs m/d/Y as we discovered above.

Status: Needs review » Needs work

The last submitted patch, 21: 2913829-21.javascript-set-default-time.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jonathan1055’s picture

Status: Needs work » Needs review
FileSize
12.11 KB

Add screen-grab to try to find out why the node save failed.

Status: Needs review » Needs work

The last submitted patch, 23: 2913829-23.javascript-set-default-time.patch, failed testing. View results

jonathan1055’s picture

Status: Needs work » Needs review
FileSize
12.57 KB

Screen shot implied that the time format 'hh:mm:ss' was not enough and it need 'am' or 'pm'. This might be ok, as adding the extra text locally does not cause an input/form validation error.

Status: Needs review » Needs work

The last submitted patch, 25: 2913829-25.javascript-set-default-time.patch, failed testing. View results

jonathan1055’s picture

Status: Needs work » Needs review
FileSize
12.61 KB

Can't test this locally, hence all these attempts on d.o. Make it qucker (and use fewer resources) by only running the javascript.

jonathan1055’s picture

Now that we have tests running properly, here's a patch without the new js library, to prove that it fails for this and not the datepicker format problem.

Status: Needs review » Needs work

The last submitted patch, 28: 2913829-28.javascript-set-default-time.patch, failed testing. View results

jonathan1055’s picture

Status: Needs work » Needs review
FileSize
12.1 KB

This should be the final test.

  • jonathan1055 committed 63e247e on 8.x-1.x
    Issue #2913829 by jonathan1055: Javascript default time - check for...
jonathan1055’s picture

Status: Needs review » Fixed

Minor tweak to check for element.val() === "" instead of !element.val() because on the first loop the value is blank (so we set it) but on subsequent loops the value is 'undefined'. So testing for blank feels a better way to do it.

Thank you @Stockticker for the original patch (which pretty much made it entirely unchanged through to commit) and thanks @eworwa for raising the issue and @gsquirrel for help in narrowing down the cause of the problem.

Committed and fixed.

Status: Fixed » Closed (fixed)

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