- Open on Drupal.org →Environment: PHP 8.1 & MySQL 5.7
15:59 15:57 Queued - Open on Drupal.org →Environment: PHP 8.1 & MySQL 5.7
15:59 15:57 Queued - 🇩🇪Germany FeyP
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.
- last update
over 1 year ago 29,403 pass - last update
over 1 year ago 29,372 pass, 1 fail The last submitted patch, 27: 2823847-27-tests-only.patch, failed testing. View results →
- Status changed to Needs work
over 1 year ago 1:33pm 8 May 2023 - 🇺🇸United States smustgrave
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.
- Status changed to Needs review
over 1 year ago 4:18pm 8 May 2023 - 🇩🇪Germany FeyP
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.
- Status changed to RTBC
over 1 year ago 5:58pm 8 May 2023 - 🇺🇸United States smustgrave
Kudos for the best steps to reproduce I may have seen.
I think this is good for committer review now.
- last update
over 1 year ago 29,373 pass, 1 fail The last submitted patch, 27: 2823847-27-tests-only.patch, failed testing. View results →
- last update
over 1 year ago 29,376 pass, 1 fail The last submitted patch, 27: 2823847-27-tests-only.patch, failed testing. View results →
- last update
over 1 year ago 29,381 pass, 1 fail The last submitted patch, 27: 2823847-27-tests-only.patch, failed testing. View results →
- Open on Drupal.org →Environment: PHP 8.1 & MySQL 5.7last update
over 1 year ago Waiting for branch to pass The last submitted patch, 27: 2823847-27-tests-only.patch, failed testing. View results →
- last update
over 1 year ago 29,411 pass, 2 fail - 🇩🇪Germany FeyP
Re-uploading patch #27 so that the correct patch is re-tested every 2 days. RTBC per #32.
The last submitted patch, 37: 2823847-27.patch, failed testing. View results →
- last update
over 1 year ago 29,412 pass - 🇩🇪Germany FeyP
Random test failure, see 🐛 [random test failure] InstallerExistingConfig[SyncDirectory]MultilingualTest::testConfigSync Fixed , retest was green. Back to RTBC per #32.
- last update
over 1 year ago 29,412 pass - last update
over 1 year ago 29,412 pass 38:57 36:24 Running- last update
over 1 year ago 29,423 pass - last update
over 1 year ago 29,423 pass - last update
over 1 year ago 29,424 pass - last update
over 1 year ago 29,433 pass - Open on Drupal.org →Environment: PHP 8.1 & MySQL 5.7last update
over 1 year ago Waiting for branch to pass - last update
over 1 year ago 29,442 pass - last update
over 1 year ago 29,444 pass - last update
over 1 year ago 29,444 pass 23:57 20:42 Running- last update
over 1 year ago 29,453 pass - last update
over 1 year ago 29,454 pass - last update
over 1 year ago 29,454 pass - last update
over 1 year ago 29,460 pass - last update
over 1 year ago 29,460 pass - last update
over 1 year ago 29,460 pass - last update
over 1 year ago 29,465 pass - last update
over 1 year ago 29,468 pass - last update
over 1 year ago 29,467 pass - last update
over 1 year ago 29,467 pass - last update
over 1 year ago 29,463 pass - last update
over 1 year ago 29,463 pass - last update
over 1 year ago 29,463 pass - last update
over 1 year ago 29,468 pass 53:57 50:09 Running- last update
over 1 year ago 29,470 pass - last update
over 1 year ago 29,470 pass - last update
over 1 year ago 29,470 pass - last update
over 1 year ago 29,470 pass - last update
over 1 year ago 29,474 pass - last update
over 1 year ago 29,477 pass - last update
over 1 year ago 29,478 pass - Status changed to Needs review
over 1 year ago 9:47am 29 July 2023 - 🇫🇮Finland lauriii Finland
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.
- Status changed to Needs work
over 1 year ago 1:29pm 29 July 2023 - 🇩🇪Germany FeyP
Thanks @laurri 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 🐛 Invalid argument exception when changing language of node with menu link to und or zxx Fixed ?
- 🇫🇮Finland lauriii Finland
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.
- Status changed to Needs review
over 1 year ago 7:20am 20 August 2023 - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago 30,049 pass - last update
over 1 year ago 30,042 pass, 1 fail - 🇩🇪Germany FeyP
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 →
- Status changed to Needs work
over 1 year ago 3:02pm 20 August 2023 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.
- Status changed to Needs review
over 1 year ago 3:09pm 20 August 2023 - 🇫🇷France nod_ Lille
He does need a little help to know which patch is relevant, hiding the extra patches lets him know what to ignore :)
- 🇮🇳India kpoornima
May be patch #44.Patch #44(2823847-44.patch) is worked for me. After applied patch date range view is
21 August, 2023 - Status changed to RTBC
over 1 year ago 9:45pm 22 August 2023 - last update
over 1 year ago 30,056 pass - last update
over 1 year ago 30,058 pass - last update
over 1 year ago 30,060 pass - last update
over 1 year ago 30,060 pass 23:56 20:15 Running- last update
over 1 year ago 30,135 pass - 🇳🇿New Zealand quietone
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.
- last update
over 1 year ago 30,135 pass - last update
over 1 year ago 30,136 pass - last update
over 1 year ago 30,146 pass - last update
over 1 year ago 30,144 pass, 1 fail The last submitted patch, 44: 2823847-44.patch, failed testing. View results →
- @feyp opened merge request.
- Status changed to Needs review
about 1 year ago 1:46am 15 November 2023 - 🇩🇪Germany FeyP
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.
- Status changed to RTBC
about 1 year ago 3:39pm 15 November 2023 - 🇺🇸United States smustgrave
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.
- Status changed to Needs work
12 months ago 1:04pm 11 January 2024