Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
datetime_datetime_widget_validate() is an #element_validate callback used in DateTimeDefaultWidget
- it could be a method in DateTimeDefaultWidget
- it actually only transforms the submitted values, and thus could be moved to massageFormValues()
Comment | File | Size | Author |
---|---|---|---|
#26 | 2327363-datetime-26.patch | 14.36 KB | andypost |
#19 | interdiff.txt | 700 bytes | andypost |
#18 | 2327363-datetime-18.patch | 14.34 KB | andypost |
#18 | interdiff.txt | 12.69 KB | andypost |
#14 | 2327363-datetime_widgets_validate-14.patch | 10.29 KB | yched |
Comments
Comment #1
yched CreditAttribution: yched commentedSame for datetime_datelist_widget_validate(), actually.
Comment #2
yched CreditAttribution: yched commentedLet's see what breaks.
Comment #4
yched CreditAttribution: yched commentedForgot a somewhat important part...
Comment #6
yched CreditAttribution: yched commentedGee, I'm rusty.
Comment #7
yched CreditAttribution: yched commentedReroll after #2328061: Move datetime's FormElement #type classes in Core
Comment #8
BerdirLooks like an awesome cleanup, two questions...
In the node widget issue, I removed the hasErrors() check because that's afaik part of the element validation and it will never get this far if you don't have a valid date by then, but I could be wrong.
Wondering why you're not seeing the problem I had with the different structure here, maybe there are no tests for a datetime field that is not accessible?
Comment #9
yched CreditAttribution: yched commented->hasErrors() : yes, that seemed a bit wonky, but I'm not too familiar with DDT and what kind of errors ->hasErrors() is about, so I left that untouched.
It does look like Core\Datetime\Element\Datetime::validateDatetime() and Core\Datetime\Element\Datelist::validateDatelist() place a form error in case ->hasErrors() is TRUE, so this check looks redundant indeed. Removed.
Yes, seems likely...
Comment #11
BerdirAh, now you left out the instanceof check too, that might be needed, because AFAIK, that happens when validation does fail.
Comment #12
yched CreditAttribution: yched commentedRight, too optimistic.
Comment #14
yched CreditAttribution: yched commentedSigh.
Comment #15
BerdirLooks like those two methods are now identical apart from the field type name in the comment? Is there a reason for not making a base class for those widgets and moving it there?
Comment #16
yched CreditAttribution: yched commentedI kind of wondered that too, wasn't sure. Seemed a bit overkill to me, but I could go both ways.
AFK though, I cannot code it myself anymore :-)
Comment #17
andypostwill try to decouple this into base class this weekend
#2331961: Move datetime module into Core/Field is postponed on this
Comment #18
andypostmoved common parts to
DateTimeWidgetBase
Comment #19
andypostclean-up use
Comment #20
BerdirThis looks nice, will probably need some manual testing, not sure how good the test coverage for datetime is?
Comment #21
andypostAlready done that:
1) the datelist widget throws the same exception when no data selected,
2) datetime works fine
But glad to get more peers on that
Comment #22
BerdirYep, seems to work as good as before. Also wasn't able to reproduce any issues with #access FALSE fields.
Comment #24
yched CreditAttribution: yched commentedContext changed, bit should otherwise be a straight reroll :-)
Comment #26
andypostre-roll
Comment #27
alexpottCommitted 6bad26f and pushed to 8.0.x. Thanks!