- Issue created by @paul121
- Merge request !6185Do not check if the parent entity is new before using the field delta value → (Closed) created by paul121
- Status changed to Needs review
over 1 year ago 12:12am 16 January 2024 - Status changed to Needs work
over 1 year ago 12:38am 16 January 2024 - 🇺🇸United States smustgrave
Thank you for reporting.
Moving to 11.x as the current development branch, MR should be updated also.
This will also require test coverage.
- 🇦🇺Australia acbramley
acbramley → changed the visibility of the branch temp to hidden.
- 🇦🇺Australia acbramley
acbramley → changed the visibility of the branch 3414883-datetimetimestamp-widget-does to hidden.
- Merge request !6187Issue #3414883: Datetime_timestamp widget does not use default field value → (Closed) created by acbramley
- 🇦🇺Australia acbramley
Recreated the MR against 11.x, please close your one @paul121
- 🇦🇺Australia acbramley
Apologies @paul121 I just saw you were fixing that branch at the same time.
- 🇺🇸United States paul121 Spokane, WA
No worries, I'll close that one. I won't have time to work on tests anytime soon.
Attaching a patch we can use in farmOS.
- Assigned to acbramley
- Status changed to Needs review
over 1 year ago 1:28am 16 January 2024 - 🇦🇺Australia acbramley
Added test coverage, fails with the following without the fix:
The field "field_timestamp[0][value][date]" value is "", but "2024-01-16" expected. - Issue was unassigned.
- Status changed to RTBC
over 1 year ago 5:42pm 16 January 2024 - 🇺🇸United States smustgrave
As a regression this seemed higher to me.
Running the tests get the same as @acbramley does in #13
Was able to confirm the issue and that the MR fixes it.
Again as a regression believe it's good to mark now vs waiting a week.
- 🇳🇿New Zealand quietone
As a regression this seemed higher to me.
Can you elaborate? Higher than what?
Again as a regression believe it's good to mark now vs waiting a week.
I do not understand this. What happens in a week?---
Thanks for the complete issue summary!I compared the change here to the commit for the other issue and confirmed that it doing as stating in the proposed resolution.
I tested and confirmed the problem. Then, I applied the diff and ran the test, which does fail without the fix. The test could be added to \Drupal\FunctionalTests\Datetime\TimestampTest::testWidget() and would not interfere with that test. I am in two minds about that but will leave that for now.
Leaving at RTBC
- First commit to issue fork.
- 🇬🇧United Kingdom catchThe test could be added to \Drupal\FunctionalTests\Datetime\TimestampTest::testWidget() and would not interfere with that test. I am in two minds about that but will leave that for now.
I agree with this. If we add assertions to the test method, we save an entire Drupal install, which can make a meaningful difference to gitlab ci runtimes, so when it's easy to do that, it's worth doing.
Went ahead and made the change locally, ran the test, and pushed. Assuming the whole pipeline comes green I'll go ahead and commit this, since it's only deleting three lines or so.
- Status changed to Fixed
over 1 year ago 11:46am 1 March 2024 - 🇬🇧United Kingdom catch
Committed/pushed to 11.x and cherry-picked to 10.3.x and 10.2.x, thanks!
Automatically closed - issue fixed for 2 weeks with no activity.
