Problem/Motivation

Currently DateRangeCustomFormatter has a logic block:

        if ($start_date->getTimestamp() !== $end_date->getTimestamp()) {
          $elements[$delta] = [
            'start_date' => $this->buildDate($start_date),
            'separator' => ['#plain_text' => ' ' . $separator . ' '],
            'end_date' => $this->buildDate($end_date),
          ];
        }
        else {
          $elements[$delta] = $this->buildDate($start_date);
        }

This is to prevent a date string being duplicated w/ a separator. Since this formatter accepts custom format strings, this can result in weird output like

2016 - 2016

when the format string is 'Y' or anything other than the full granularity of the type.

The DateRangeDefaultFormatter has the same problem, when used with a date format that doesn't display all date components, e.g. `html_year`.

Proposed resolution

Instead of comparing using ->getTimestamp(), use the ->formatDate() method to check the actual rendered string (adjusted for timezone).

Steps to reproduce

  1. Install Drupal 10.1.x-dev (Standard profile) in English language
  2. Enable "Datetime Range" module
  3. Create a new date range field for articles:
    1. Go to Structure > Content types > Article > Create new field
    2. In "Add a new field" select "Date range"
    3. As "Label" set "Date range"
    4. Click "Save and continue"
    5. Click "Save field settings"
  4. Configure either the default or custom date range formatter with a date format that doesn't show all date components:
    1. Go to Structure > Content types > Article > Manage display
    2. Verify that the selected formatter for the "Date range" field is set to "Default".
    3. Expand the formatter settings by clicking on the gear icon to the right of the select field.
    4. As "Date format" select a date format that doesn't show all date components, e.g. "Olivero Medium" or "HTML year".
    5. Click "Update"
    6. Click "Save"
  5. Create a new article with a date time range with two dates where the date components displayed by your selected date formats are equal, but the rest of the components not displayed are not equal:
    1. Go to Content
    2. Click "Add content"
    3. Click "Article"
    4. Enter a title, e.g. "123".
    5. In the "Date range" field, enter 2023-05-08 17:00:00 as "Start date".
    6. In the "Date range" field, enter 2023-05-08 19:30:00 as "End date".
    7. Click "Save"
  6. Verify that on the canonical view of the article, the date range is shown as "8 May, 2023 - 8 May, 2023".
  7. Apply patch.
  8. Clear all caches.
  9. Verify that on the canonical view of the article, the date range is shown as "8 May, 2023".

Actual result

The date range field displays as START DATE SEPRATOR END DATE even though both start date and end date are the same.

Date range field in node before patch

Expected result

The date range field displays as START DATE since both dates are equal.

Date range field in node after patch

Remaining tasks

User interface changes

API changes

Data model changes

Merge request to commit

MR 5398

CommentFileSizeAuthor
#46 2823847-nr-bot.txt47.26 KBneeds-review-queue-bot
#44 2823847-44.patch80.47 KBfeyp
#44 2823847-44-tests-only.patch78.76 KBfeyp
#44 interdiff-2823847-27-44.txt91.81 KBfeyp
#44 2823847-44-no-indentation.patch60.35 KBfeyp
#44 interdiff-2823847-27-44-no-indentation.txt41.37 KBfeyp
#37 2823847-27.patch60.13 KBfeyp
#30 2823847-after.png16.29 KBfeyp
#30 2823847-before.png17.08 KBfeyp
#27 2823847-27.patch60.13 KBfeyp
#27 2823847-27-tests-only.patch58.42 KBfeyp
#26 2823847-26.patch60.13 KBfeyp
#26 2823847-26-tests-only.patch58.42 KBfeyp
#10 interdiff-2823847-5-10.txt9.54 KBjungle
#10 compare-dates-2823847-10.patch9.83 KBjungle
#5 interdiff-2823847-4-5.txt777 bytesjungle
#5 compare-dates-2823847-5.patch2.82 KBjungle
#4 compare-dates-2823847-4.patch2.82 KBjungle

Issue fork drupal-2823847

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

mpdonadio created an issue. See original summary.

elakiyasamuel’s picture

elakiyasamuel commented
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Assigned: Unassigned » jungle
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Assigned: jungle » Unassigned
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new compare-dates-2823847-4.patch2.82 KB
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
StatusFileSize
new compare-dates-2823847-5.patch2.82 KB
new interdiff-2823847-4-5.txt777 bytes

fixed a typo.

Status: Needs review » Needs work

The last submitted patch, 5: compare-dates-2823847-5.patch, failed testing.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Status: Needs work » Needs review
mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Thanks for working on this.

+++ b/core/modules/datetime_range/src/Tests/DateRangeFieldTest.php
@@ -557,11 +557,20 @@ public function testAlldayRangeField() {
-    $expected = $start_date->format($this->displayOptions['settings']['date_format']) . ' THESEPARATOR ' . $end_date->format($this->displayOptions['settings']['date_format']);
-    $this->renderTestEntity($id);
-    $this->assertText($expected, new FormattableMarkup('Formatted date field using daterange_custom format displayed as %expected.', array('%expected' => $expected)));
-    $this->assertText(' THESEPARATOR ', 'Found proper separator');
-
+    $start_date_rendered = $start_date->format($this->displayOptions['settings']['date_format']);
+    $end_date_rendered = $end_date->format($this->displayOptions['settings']['date_format']);
+    if ($start_date_rendered !== $end_date_rendered) {
+      $expected = $start_date_rendered . ' THESEPARATOR ' . $end_date_rendered;
+      $this->renderTestEntity($id);
+      $this->assertText($expected, new FormattableMarkup('Formatted date field using daterange_custom format displayed as %expected.', array('%expected' => $expected)));
+      $this->assertText(' THESEPARATOR ', 'Found proper separator');
+    }
+    else {
+      $expected = $start_date_rendered;
+      $this->renderTestEntity($id);
+      $this->assertText($expected, new FormattableMarkup('Formatted date field using daterange_custom format displayed as %expected.', array('%expected' => $expected)));
+      $this->assertNoText(' THESEPARATOR ', 'Not found proper separator');
+    }
   }

I think we need some additional test coverage here. For all three field flavors, we should check a format with everything for that flavor (the ones that are there should be fine), and then also test 'Y'. This way we can make sure we test both paths through the if statement that we adjusted.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Assigned: Unassigned » jungle
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Assigned: jungle » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new compare-dates-2823847-10.patch9.83 KB
new interdiff-2823847-5-10.txt9.54 KB

Tests added for all three field flavors.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
commented
Status: Needs review » Needs work

I am getting local fails on line 177 of DateRangeFieldTest.

  1. +++ b/core/modules/datetime_range/src/Tests/DateRangeFieldTest.php
@@ -165,6 +165,17 @@ public function testDateRangeField() {
+      $this->displayOptions['type'] = 'daterange_custom';
+      $this->displayOptions['settings'] = array('date_format' => 'Y') + $this->defaultSettings;
+      entity_get_display($this->field->getTargetEntityTypeId(), $this->field->getTargetBundle(), 'full')
+        ->setComponent($field_name, $this->displayOptions)
+        ->save();
+      $start_date_rendered = $start_date->format($this->displayOptions['settings']['date_format']);
+      $end_date_rendered = $end_date->format($this->displayOptions['settings']['date_format']);
+      $expected = $start_date_rendered . ' - ' . $end_date_rendered;
+      $this->renderTestEntity($id);
+      $this->assertText($expected, new FormattableMarkup('Formatted date field using daterange_custom format displayed as %expected.', array('%expected' => $expected)));

    This is where I am getting the fail locally, but we need to test this behavior when different dates in the same year get folded down to a single year on output. 

  2. +++ b/core/modules/datetime_range/src/Tests/DateRangeFieldTest.php
@@ -232,6 +243,17 @@ public function testDateRangeField() {
+      $this->displayOptions['type'] = 'daterange_custom';
+      $this->displayOptions['settings'] = array('date_format' => 'Y') + $this->defaultSettings;
+      entity_get_display($this->field->getTargetEntityTypeId(), $this->field->getTargetBundle(), 'full')
+        ->setComponent($field_name, $this->displayOptions)
+        ->save();
+      $start_date_rendered = $start_date->format($this->displayOptions['settings']['date_format']);
+      $expected = $start_date_rendered;
+      $this->renderTestEntity($id);
+      $this->assertText($expected, new FormattableMarkup('Formatted date field using daterange_custom format displayed as %expected.', array('%expected' => $expected)));
+      $this->assertNoText(' THESEPARATOR ', 'Not found proper separator');
     }

    We need to test this behavior when different dates in the same year get folded down to a single year on output. The dates are the same here, so this doesn't really cover the code change. 

  3. +++ b/core/modules/datetime_range/src/Tests/DateRangeFieldTest.php
@@ -324,6 +346,18 @@ public function testDatetimeRangeField() {
+    $this->displayOptions['type'] = 'daterange_custom';
+    $this->displayOptions['settings'] = array('date_format' => 'Y') + $this->defaultSettings;
+    entity_get_display($this->field->getTargetEntityTypeId(), $this->field->getTargetBundle(), 'full')
+      ->setComponent($field_name, $this->displayOptions)
+      ->save();
+    $start_date_rendered = $start_date->format($this->displayOptions['settings']['date_format']);
+    $end_date_rendered = $end_date->format($this->displayOptions['settings']['date_format']);
+    $expected = $start_date_rendered . ' - ' . $end_date_rendered;
+    $this->renderTestEntity($id);
+    $this->assertText($expected, new FormattableMarkup('Formatted date field using daterange_custom format displayed as %expected.', array('%expected' => $expected)));

    We need to test this behavior when different dates in the same year get folded down to a single year on output. The dates are the in different years here, so this doesn't really cover the code change. 

  4. +++ b/core/modules/datetime_range/src/Tests/DateRangeFieldTest.php
@@ -399,6 +433,29 @@ public function testDatetimeRangeField() {
+
+    $this->displayOptions['type'] = 'daterange_custom';
+    $this->displayOptions['settings'] = array('date_format' => 'm/d/Y') + $this->defaultSettings;
+    entity_get_display($this->field->getTargetEntityTypeId(), $this->field->getTargetBundle(), 'full')
+      ->setComponent($field_name, $this->displayOptions)
+      ->save();
+    $start_date_rendered = $start_date->format($this->displayOptions['settings']['date_format']);
+    $expected = $start_date_rendered;
+    $this->renderTestEntity($id);
+    $this->assertText($expected, new FormattableMarkup('Formatted date field using daterange_custom format displayed as %expected.', array('%expected' => $expected)));
+    $this->assertNoText(' THESEPARATOR ', 'Not found proper separator');
+
+    $this->displayOptions['type'] = 'daterange_custom';
+    $this->displayOptions['settings'] = array('date_format' => 'Y') + $this->defaultSettings;
+    entity_get_display($this->field->getTargetEntityTypeId(), $this->field->getTargetBundle(), 'full')
+      ->setComponent($field_name, $this->displayOptions)
+      ->save();
+    $start_date_rendered = $start_date->format($this->displayOptions['settings']['date_format']);
+    $expected = $start_date_rendered;
+    $this->renderTestEntity($id);
+    $this->assertText($expected, new FormattableMarkup('Formatted date field using daterange_custom format displayed as %expected.', array('%expected' => $expected)));
+    $this->assertNoText(' THESEPARATOR ', 'Not found proper separator');
+

    We need to test this behavior when different dates in the same year get folded down to a single year on output. The dates are the same here, so this doesn't really cover the code change. 

  5. +++ b/core/modules/datetime_range/src/Tests/DateRangeFieldTest.php
@@ -485,6 +542,17 @@ public function testAlldayRangeField() {
+    $this->displayOptions['type'] = 'daterange_custom';
+    $this->displayOptions['settings'] = array('date_format' => 'Y') + $this->defaultSettings;
+    entity_get_display($this->field->getTargetEntityTypeId(), $this->field->getTargetBundle(), 'full')
+      ->setComponent($field_name, $this->displayOptions)
+      ->save();
+    $start_date_rendered = $start_date->format($this->displayOptions['settings']['date_format']);
+    $end_date_rendered = $end_date->format($this->displayOptions['settings']['date_format']);
+    $expected = $start_date_rendered . ' - ' . $end_date_rendered;
+    $this->renderTestEntity($id);
+    $this->assertText($expected, new FormattableMarkup('Formatted date field using daterange_custom format displayed as %expected.', array('%expected' => $expected)));
+

    We need to test this behavior when different dates in the same year get folded down to a single year on output. The dates are the same here, so this doesn't really cover the code change. 

  6. +++ b/core/modules/datetime_range/src/Tests/DateRangeFieldTest.php
@@ -553,14 +621,28 @@ public function testAlldayRangeField() {
-    $this->displayOptions['settings']['date_format'] = 'm/d/Y';
+    $this->displayOptions['settings'] = array('date_format' => 'm/d/Y') + $this->defaultSettings;

    Why was this needed? 

  7. +++ b/core/modules/datetime_range/src/Tests/DateRangeFieldTest.php
@@ -553,14 +621,28 @@ public function testAlldayRangeField() {
+    $this->assertNoText(' THESEPARATOR ', 'Not found proper separator');
+
+
+    $this->displayOptions['type'] = 'daterange_custom';
+    $this->displayOptions['settings'] = array('date_format' => 'Y') + $this->defaultSettings;

    Nit, exrta blank line.

The last submitted patch, 10: compare-dates-2823847-10.patch, failed testing.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
commented
Issue summary: View changes 
+++ b/core/modules/datetime_range/src/Plugin/Field/FieldFormatter/DateRangeCustomFormatter.php
@@ -48,7 +48,7 @@ public function viewElements(FieldItemListInterface $items, $langcode) {
-        if ($start_date->format('U') !== $end_date->format('U')) {
+        if ($this->formatDate($start_date) !== $this->formatDate($end_date)) {
           $elements[$delta] = [

And this one is on me. `$this->formatDate` is incorrect here. It doesn't take timezone into account, and it doesn't apply the date-only logic properly. We need to use buildDate(). I think we can use the render arrays from buildDate, and compare the '#plain_text' entries, and then just reused the build arrays in the final version we return.

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.

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.

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

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.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.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

feyp’s picture

feyp commented
Title: DateRangeCustomFormatter should compare rendered dates instead of raw timestamps » DateRange formatters should compare rendered dates instead of raw timestamps
Category: Feature request » Bug report
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new 2823847-26-tests-only.patch58.42 KB
new 2823847-26.patch60.13 KB

I don't think this is a feature request, I think this is a bug. Since the module already compares the time stamps, clearly the intended functionality is to only display a single date, if the dates don't differ.

I'd also like to rescope the issue to also include the default date formatter. It has the same problem and I think it makes sense to fix both in one go. Updating title and IS.

W/r/t #13, I think the remarks are outdated. buildDate() now just calls formatDate() and looking at the code, it takes the time zone into account and works with the date-only logic. So I think instead of buildDate() we should stick to using formatDate(). Please correct me, if I'm wrong. Updated the proposed resolution in the IS.

I updated the patch for Drupal 10.1. For the test coverage, I added data providers to cover more date ranges and additional code to test the different formatters with the html_year date format. This should address #11.

feyp’s picture

feyp commented
StatusFileSize
new 2823847-27-tests-only.patch58.42 KB
new 2823847-27.patch60.13 KB

Hopefully fixing the cspell errors...

The last submitted patch, 27: 2823847-27-tests-only.patch, failed testing. View results

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

This was much easier to review applying locally vs the diff haha.

Bulk of the changes are just indent moves (for others who review).

Taking a look at the changes and conceptually makes sense to me.

But could we had explicit steps for how this could appear? Then can add some before/after screenshots to show the problem and now that it's fixed.

Will keep an eye out for this one.

feyp’s picture

feyp commented
StatusFileSize
new 2823847-before.png17.08 KB
new 2823847-after.png16.29 KB

Added screenshots for before/after.

feyp’s picture

feyp commented
Issue summary: View changes
Status: Needs work » Needs review

Thanks for your review @smustgrave, it is really appreciated! I updated the issue summary with steps to reproduce for the default formatter and actual result and expected result including before/after screenshots.

smustgrave’s picture

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

Kudos for the best steps to reproduce I may have seen.

I think this is good for committer review now.

The last submitted patch, 27: 2823847-27-tests-only.patch, failed testing. View results

The last submitted patch, 27: 2823847-27-tests-only.patch, failed testing. View results

The last submitted patch, 27: 2823847-27-tests-only.patch, failed testing. View results

The last submitted patch, 27: 2823847-27-tests-only.patch, failed testing. View results

feyp’s picture

feyp commented
StatusFileSize
new 2823847-27.patch60.13 KB

Re-uploading patch #27 so that the correct patch is re-tested every 2 days. RTBC per #32.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 37: 2823847-27.patch, failed testing. View results

feyp’s picture

feyp commented
Status: Needs work » Reviewed & tested by the community
Related issues: +#3361121: [random test failure] InstallerExistingConfig[SyncDirectory]MultilingualTest::testConfigSync

Random test failure, see #3361121: [random test failure] InstallerExistingConfig[SyncDirectory]MultilingualTest::testConfigSync, retest was green. Back to RTBC per #32.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs review

The new behavior makes sense. Not going to ask for subsystem maintainer review because I see that @mpdonadio filed this issue and has been active here.

+++ b/core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php
@@ -47,244 +47,320 @@ protected function getTestFieldType() {
+  public function provideDateRangeFieldData() {
...
+   * @dataProvider provideDateRangeFieldData

I didn't review all of the test changes in detail yet because I'm wondering if we need to convert this to a data provider? This is a browser based test meaning that every individual data set requires a new installation of Drupal. The test currently takes 43 seconds to run, and the new test takes almost 9 minutes.

feyp’s picture

feyp commented
Status: Needs review » Needs work

Thanks @lauriii for the review. Fair enough, I'll change this to foreach loops. Not sure yet if I have time left this weekend, might have to wait until next weekend.

Should we do the same in #2904899: Invalid argument exception when changing language of node with menu link to und or zxx?

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

That other one has just 2 iterations at the moment so it isn't as critical. Usually it's not a big deal, and it's fine to use providers for their improved DX but on this issue the difference was significant enough to warrant the DX hit.

feyp’s picture

feyp commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2823847-27-44-no-indentation.txt41.37 KB
new 2823847-44-no-indentation.patch60.35 KB
new interdiff-2823847-27-44.txt91.81 KB
new 2823847-44-tests-only.patch78.76 KB
new 2823847-44.patch80.47 KB

Thanks @lauriii for the clarification in #43, very helpful.

I replaced the data providers with foreach loops, this should address #41. Patch and interdiff without indentation changes provided for convenience of reviewers.

The last submitted patch, 44: 2823847-44-tests-only.patch, failed testing. View results

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 2823847-nr-bot.txt47.26 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

feyp’s picture

feyp commented
Status: Needs work » Needs review
Issue tags: +no-needs-review-bot

Sorry, bot, but you tested the wrong patch...

nod_’s picture

nod_
Primary language French
Location Lille
commented
Issue tags: -no-needs-review-bot

He does need a little help to know which patch is relevant, hiding the extra patches lets him know what to ignore :)

kpoornima’s picture

kpoornima commented

May be patch #44.Patch #44(2823847-44.patch) is worked for me. After applied patch date range view is
21 August, 2023

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC

quietone’s picture

quietone commented

I'm triaging RTBC issues. I read the IS and the comments. There are new tests and a fail test. I didn't find any unanswered questions or other work to do.

Leaving at RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 44: 2823847-44.patch, failed testing. View results

FeyP opened merge request !5398

feyp’s picture

feyp commented
Issue summary: View changes
Status: Needs work » Needs review

I wasn't able to reproduce the test fail locally, so I used the opportunity to switch to an MR based on patch #44. Let's see what the pipeline says.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community 
There were 3 failures:
1) Drupal\Tests\datetime_range\Functional\DateRangeFieldTest::testDateRangeField
Formatted date field using <em class="placeholder">html_year</em> format has no end component <em class="placeholder">2012</em> with <em class="placeholder">2012-12-31T12:00:00Z</em> attribute for date range <em class="placeholder">Pacific/Kwajalein: same year</em>.
Failed asserting that '\n
            <div>gDkJ6AxF</div>\n
      \n
            <div><time datetime="2012-06-06T12:00:00Z">2012</time>\n
 THESEPARATOR <time datetime="2012-12-31T12:00:00Z">2012</time>\n
</div>\n
      ' does not contain "<time datetime="2012-12-31T12:00:00Z">2012</time>".
/builds/issue/drupal-2823847/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121
/builds/issue/drupal-2823847/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/builds/issue/drupal-2823847/core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php:211
/builds/issue/drupal-2823847/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
2) Drupal\Tests\datetime_range\Functional\DateRangeFieldTest::testDatetimeRangeField
Formatted date field using <em class="placeholder">html_year</em> format has no end component <em class="placeholder">2012</em> with <em class="placeholder">2012-12-31T00:00:00Z</em> attribute for date range <em class="placeholder">same year</em>.
Failed asserting that '\n
            <div>tegy33Jb</div>\n
      \n
            <div><time datetime="2012-06-06T00:00:00Z">2012</time>\n
 THESEPARATOR <time datetime="2012-12-31T00:00:00Z">2012</time>\n
</div>\n
      ' does not contain "<time datetime="2012-12-31T00:00:00Z">2012</time>".
/builds/issue/drupal-2823847/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121
/builds/issue/drupal-2823847/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/builds/issue/drupal-2823847/core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php:478
/builds/issue/drupal-2823847/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
3) Drupal\Tests\datetime_range\Functional\DateRangeFieldTest::testAlldayRangeField
Formatted date field using <em class="placeholder">long</em> format has no end component <em class="placeholder">2012</em> with <em class="placeholder">2012-12-31T12:59:59Z</em> attribute for date range <em class="placeholder">same year</em>.
Failed asserting that '\n
            <div>ymy7ICAn</div>\n
      \n
            <div><time datetime="2012-06-05T14:00:00Z">2012</time>\n
 THESEPARATOR <time datetime="2012-12-31T12:59:59Z">2012</time>\n
</div>\n
      ' does not contain "<time datetime="2012-12-31T12:59:59Z">2012</time>".
/builds/issue/drupal-2823847/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121
/builds/issue/drupal-2823847/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/builds/issue/drupal-2823847/core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php:753
/builds/issue/drupal-2823847/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
FAILURES!
Tests: 7, Assertions: 439, Failures: 3.

Posting test-only results here

Restoring RTBC status from before.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

MR needs rebasing.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.