[D8] Compact date/time range formatter [#2875485]

The compact date/time range formatter module builds on the core date & time range module, by providing the ability to display date/time range fields in a more compact form, by omitting the month or year where it would be duplicated.

Examples:
24–25 January 2017 (range is within a single month)
29 January–3 February 2017 (range is within a single year)
9:00am–4:30pm, 1 April 2017 (range is within a single day)

This module defines a new configuration entity, date_range_format, to store patterns for date range formats, in a similar way to Drupal's core date formats. Variations can be configured for use where the day, month or year is the same for both start and end date.

Both date-only and date & time ranges are supported.

Comments

Comment #1

3 May 2017 at 22:03

erik.erskine created an issue. See original summary.

Comment #2

PA robot CreditAttribution: PA robot commented 3 May 2017 at 22:23

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Comment #3

heddn

English

Nicaragua

CreditAttribution: heddn at MTech, LLC, Acro Commerce commented 8 June 2017 at 16:18

Status:

Needs review

» Needs work

Not at all related to the code, but a suggestion is to rename the module from daterange_compact to daterange_formatters. Or add it as a patch to https://www.drupal.org/project/datetime_extras. Naming is hard, but to think that only compact formatters will ever exist for daterange seems odd.

Code review:

In DateRangeFormatListBuilder, I typically prefer protected over private in API or framework code. You don't know when someone wants to override your code. And private make that difficult. Plus, I think using DateRangeFormatter (daterange_compact.date_range.formatter) as a service instead of the one-off examples would make for cleaner code. Then the call to htmlspecialchars isn't necessary.
private function dateTimeExamples(DateRangeFormatInterface $format) {
Which leads me to a theoretic security vulnerability. Which would be good to display in the examples. If someone adds something that when concacinates leads to an XSS, then they won't see it when they add the format. Because on the example page we escape html, but not on other pages. DateRangeFormatter just contacts all the parts together. So something could be added to the start and end that when combined cause XSS or other badness. I'm not suggesting we pass the result through html escaping, because that would mean legitimate spans, etc in the date would get removed. But something should be done here. Maybe just posting the actual markup in the examples is sufficient.

For my theoretic vulnerability, I'm marking as needs works.

Comment #4

erik.erskine CreditAttribution: erik.erskine commented 12 June 2017 at 21:53

Status:

Needs work

» Needs review

heddn, thanks very much for your review.

Calling this module "daterange_formatters" sounds a bit too broad to me. I've never intended it to become a suite of many possible formatters, but rather addresses a single problem.

I looked at https://www.drupal.org/project/datetime_extras - perhaps it would be a good place for this. I notice that one of the aims is to help mentor new contributors, and get code to be a good enough standard for core. That would be valuable for me so I'm open to adding it there.

I couldn't see any plan for a stable release though, so in the meantime have submitted this module as a discreet piece of functionality in order to be reviewed.

1) In DateRangeFormatListBuilder, I typically prefer protected over private
in API or framework code. You don't know when someone wants to override
your code. And private make that difficult. Plus, I think using
DateRangeFormatter (daterange_compact.date_range.formatter) as a service
instead of the one-off examples would make for cleaner code. Then the
call to htmlspecialchars isn't necessary.
private function dateTimeExamples(DateRangeFormatInterface $format) {

Not sure I follow - would `dateTimeExamples` be a function inside DateRangeFormatter (the service)?

It's hard to know where to put these. They are intended to make the DateRangeFormatListBuilder more useful - showing concrete examples rather than the pattern itself. Core does this for date formats, but is slightly flawed if the current date happens to be ambiguous (eg 1/1). We also need to show a variety of ranges (example of same day, same month, same year etc) - hence the hardcoded values.

2) Which leads me to a theoretic security vulnerability. Which would be good
to display in the examples. If someone adds something that when
concacinates leads to an XSS, then they won't see it when they add the
format. Because on the example page we escape html, but not on other
pages. DateRangeFormatter just contacts all the parts together. So
something could be added to the start and end that when combined cause
XSS or other badness. I'm not suggesting we pass the result through html
escaping, because that would mean legitimate spans, etc in the date would
get removed. But something should be done here. Maybe just posting the
actual markup in the examples is sufficient.

You're right, this is a potential vulnerability.

Core's main date formatter doesn't allow any HTML - it always escapes the output. You can't put a span in the date format. The intention here was to follow suit, so I have changed the field formatter to escape the output by using `#plain_text` instead of `#markup` which should remove the vulnerability.

Comment #5

John Pitcairn CreditAttribution: John Pitcairn commented 20 June 2017 at 07:17

In my opinion this formatter would be a useful addition to the core date module.

That said, getting this released as a full project with security coverage will be a lot easier than getting it into core, so let's focus on that. I'm adding it to a soon-to-be-live website and will provide further feedback or RTBC soon.

Comment #6

John Pitcairn CreditAttribution: John Pitcairn commented 20 June 2017 at 23:30

Status:

Needs review

» Reviewed & tested by the community

I think ultimately this would be best in datetime_extras, and we push for a stable release with security support there. I'll open issues there about that. Note there is an issue open there about a possibly similar formatter, though that appears to be buggy and progress looks stalled. #2834016: Add 'Compact' datetime range formatter

I agree with the protected vs private comment from @heddn, and have opened an issue in your sandbox, with patch. #2887922: Use protected not private to allow extending classes

Finally, this module appears to be well coded and with the potential security issue removed I see no reason why you shouldn't be allowed to make a full release with security support.

Comment #7

lias CreditAttribution: lias commented 30 July 2017 at 15:15

Is there any movement on the review of this module?

Use of this module would be very helpful for those of us adopting Drupal 8 for new sites using date range as it stands now for events. This module seems like a happy medium to move forward using Date in core rather than using an external FullCalendar type of solution.

Thanks for an update ia

Comment #8

apaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

CreditAttribution: apaderno as a volunteer commented 6 May 2018 at 07:56

Assigned:	Unassigned	» apaderno
Status:	Reviewed & tested by the community	» Fixed

Thank you for your contribution!
I am going to update your account so you can opt into security advisory coverage now.
These are some recommended readings to help with excellent maintainership:

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

I thank all the dedicated reviewers as well.