Problem/Motivation

Follow-up from #2829848: Random test failure in DateRangeFieldTest. It should be possible to remove some logic from \Drupal\datetime_range\Plugin\Field\FieldWidget\DateRangeWidgetBase::massageFormValues() but this wasn't necessary to fix the critical bug.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#7 2846963-07.patch6.49 KBmpdonadio
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

I think there's a lot of logic here that should occur in \Drupal\datetime_range\Plugin\Field\FieldType\DateRangeItem or in \Drupal\datetime\DateTimeComputed. It's also apparent that validation is occurring in the form layer - see \Drupal\datetime_range\Plugin\Field\FieldWidget\DateRangeWidgetBase::validateStartEnd() - this should be a constraint so Rest API etc get validated correctly.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio at DiD Agency commented
Related issues: +#2830094: Deprecate and remove usages of datetime_date_default_time()., +#2824717: Add a format constraint to DateTimeItem to provide REST error message, +#2832264: DateTimeItemTest is not valid

#2830094: Deprecate and remove usages of datetime_date_default_time(). may overlap a lot with this issue.

Two related issues, #2824717: Add a format constraint to DateTimeItem to provide REST error message and #2832264: DateTimeItemTest is not valid with similar work.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

Added #2847041: Add a format and start/end validation constraints to DateRangeItem to provide helpful REST/JSON:API error message

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio at DiD Agency commented
Status: Active » Needs review
FileSize
2846963-07.patch6.49 KB

Most of this was taken care of in #2830094: Deprecate and remove usages of datetime_date_default_time()., but this moves some inner loop invariants.

I think any other refactoring in this is contrived.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented

Queued for re-testing. Assuming it comes back green I think this looks good-to-go.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented
Status: Needs review » Reviewed & tested by the community

Tests are still green. This looks good!

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed 19d6457 and pushed to 8.7.x. Thanks!

  • catch committed 19d6457 on 8.7.x 
    Issue #2846963 by mpdonadio: Clean up DateRangeWidgetBase::...

Status: Fixed » Closed (fixed)

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