There have been many requests over the years to support non-gregorian calendar systems in Drupal. At the moment, Drupal is hard-wired to support only the Gregorian calendar. Any attempt to adjust that requires hacking core. While the actual provision of other calendars will most likely remain in contrib, core needs to be adjusted to make it possible to support other calendar systems more seamlessly. The main support that seems to be needed and required is to adjust the display of date information to match the calendar in question. Calendar systems each have their own idea of the names of the days of the week and the months of the year, how many days are in a month or a year, etc. Wherever we provide such lists, we should do it using a plugin so the values can be adjusted as needed.

The new Plugin system in D8 is a perfect time to make this change. I've created a patch that creates a calendar plugin system. Core would provide the gregorian calendar and the plugin infrastructure, and contrib can provide other calendars by implementing hook_datetime_calendar_info(). The plugin consists of an interface that defines methods like 'monthNames' and 'dayNames', which are populated by the selected calendar. Then everywhere we want to display things like option lists of day names, we use the plugin to retrieve those lists.

The patch also adds a new system configuration item for 'system.calendar' to store the desired default value, and adds that to the regional settings page, then uses it to determine which list of date elements to retrieve.

In addition to making it possible to support other calendar systems, this should be good for the multilingual system, because the translation of all those date elements is now concentrated in a single location and clearly specified, including not only the long names but also the abbreviations for those names.

This patch relies on and extends #1802278: Add a Date component to core, so that patch must be applied to be able to evaluate this one. It's going to fail tests unless and until that patch is applied, but I'm marking it for review to get eyes on it. I'm hoping that if we can get the first patch in we can follow it up with this one.

Files: 
CommentFileSizeAuthor
#26 1811912-calendar-26.patch47.07 KBKarenS
PASSED: [[SimpleTest]]: [MySQL] 48,075 pass(es).
[ View ]
#9 1811912-calendar-translation-9.patch110.24 KBKarenS
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1811912-calendar-translation-9.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#9 1802278-to-1811912-intediff.txt47.07 KBKarenS
#7 1811912-calendar-translation-7.patch116.4 KBKarenS
FAILED: [[SimpleTest]]: [MySQL] 46,189 pass(es), 180 fail(s), and 180 exception(s).
[ View ]
#7 1802278-to-1811912-diff.txt53.94 KBKarenS
#5 1811912-calendar-translation.patch117.13 KBKarenS
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/includes/theme.inc.
[ View ]
#5 1811912-to-1802278-interdiff.txt54.01 KBKarenS
#3 1811912-calendar-translation.patch134.25 KBKarenS
FAILED: [[SimpleTest]]: [MySQL] 42,710 pass(es), 193 fail(s), and 137 exception(s).
[ View ]
#3 1811912-to-1802278-interdiff.txt71.13 KBKarenS
#2 calendar2.patch71.13 KBKarenS
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch calendar2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
date_calendar.patch71.13 KBKarenS
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch date_calendar.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Status:Needs review» Needs work

The last submitted patch, date_calendar.patch, failed testing.

KarenS’s picture

StatusFileSize
new71.13 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch calendar2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Rerolling to catch up to head.

KarenS’s picture

Status:Needs work» Needs review
StatusFileSize
new71.13 KB
new134.25 KB
FAILED: [[SimpleTest]]: [MySQL] 42,710 pass(es), 193 fail(s), and 137 exception(s).
[ View ]

Another reroll. Since this is a follow-up to #1802278: Add a Date component to core, I've posted a patch that includes both these changes and the ones in that issue so I can see if tests are passing. Then I'm posting an interdiff that shows the part of the patch that this issue addresses. If that issue gets committed, the interdiff will become the patch for this issue. This way it's possible to see what's going on with tests and still keep a patch that only shows what's different here.

I will be posting a third patch at #501428: Date and time field type in core that will build on these two patches to actually add a date form element and field to core.

Status:Needs review» Needs work

The last submitted patch, 1811912-calendar-translation.patch, failed testing.

KarenS’s picture

Status:Needs work» Needs review
StatusFileSize
new54.01 KB
new117.13 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/includes/theme.inc.
[ View ]

Let's try a re-roll

Status:Needs review» Needs work

The last submitted patch, 1811912-calendar-translation.patch, failed testing.

KarenS’s picture

Status:Needs work» Needs review
StatusFileSize
new53.94 KB
new116.4 KB
FAILED: [[SimpleTest]]: [MySQL] 46,189 pass(es), 180 fail(s), and 180 exception(s).
[ View ]

Another reroll, with the latest changes to #1802278: Add a Date component to core. I also switched the methods on the calendar class to camel case (I knew better, that was an oversight). And I wrapped the calendar plugin in a cache decorator that caches by language. It looks like caching by translation may still be broken due to #1722882: Plugin CacheDecorator caches globally, ignores request context, and does not specify tags for cache items, so we will want to check after that and add tests for translations.

Again, the patch includes changes in #1802278: Add a Date component to core so I can see if tests are passing, but the code in this patch is what has been added by the interdiff. As soon as the other patch is committed I'll switch this patch to be only the calendar plugin code.

Status:Needs review» Needs work

The last submitted patch, 1811912-calendar-translation-7.patch, failed testing.

KarenS’s picture

Status:Needs work» Needs review
StatusFileSize
new47.07 KB
new110.24 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1811912-calendar-translation-9.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Let's try these patches.

moshe weitzman’s picture

Issue tags:+D8MI

Add tag

Gábor Hojtsy’s picture

Added a note to this issue at http://groups.drupal.org/node/10607, #242965: Add support for pluggable calendars in Date API, #166234: Support non-Gregorian Calendars (once supported in core). and #1178342: Allow contributed modules to alter the format_date() function result.. I think the last one is probably a duplicate of this issue. Also posted in the Calendar systems queue at #1827978: Swappable calendars proposed for core. I don't feel experienced in this field to comment but I'm pretty sure there are people around those issues and projects that would be very qualified to help / review, etc.

sinasalek’s picture

Great! thanks Karen for working on this issue , this is indeed an important issue,
Generally i like this approach , as a matter of fact it's very similar to what i planed for the next major version of calendar systems.
The only difference perhaps is that i wanted to propose a similar patch to date module :)
I'm also working on a new submodule for calendar systems which makes it possible to do calculations by different calendar systems
considering that we have unified api for all entities in D8, implementing a generic solution is within reach
i'll notify you once i finished (probably tommorow)
I think i'll need to try it and see how well it works. i'll report back soon. How much time do we have till feature freeze?

KarenS’s picture

A month, and we can't wait til the last minute, so we need to move quickly. My thought is that a combination of this patch and some work on the format_date() function so that it uses the International Formatter, if available, would be a good goal for core. Then contrib could implement plugins and work on more functionality.

KarenS’s picture

Issue tags:+Datein8

Tagging with Datein8

sinasalek’s picture

Issue tags:-Datein8

Ok then i'll give you a review right now ;)

Calendar systems comes with a core patch in order to work, it does exactly what you're looking for i think. it passes the control to calendar system when the format_date is called and it decides which calendar to use for formatting the date.
http://drupalcode.org/project/calendar_systems.git/blob/ec71923e4790ef63...

However there are problems :
- Drupal uses format_date for everything including formatting date text field values and there is no way to tell where it's been used. This can cause trouble when using non Gregorian calendars because when the form saves, it can't convert the formatted date back to it's original value. It's addressed in calendar systems by altering forms submitted values and convert them back to Gregorian before they reach the original date validation. Yet it's not a total solution since currently it doesn't work for all kind of date fields specially the custom ones.

Solutions considering that date component is RTBC #1802278: Add a Date component to core :
- It's possible to automatically detect date fields and convert the values back to Gregorian using alteration. this alteration i think should happen by default even for Gregorian.
- Similar to previous solution but a simpler one is if the date component also has a separate hook for when it converts the date strings submitted by forms
- And an even simpler one is that core should always use strtotime method provided by date component. this way wherever something converts back to timestamp it gets converted using the methods provided by the same calendar system

If we want to make it more flexible and allow different fields to have different calendar systems or have separate calendar system than the default one. A new attribute can be introduced by date component, meaning that we can override site's default calendar system by adding a new parameter to indicate he in use calendar system, this way we can accurately and safely convert it back to timestamp

sinasalek’s picture

Issue tags:-D8MI

reverting the removed Datein8 tag

KarenS’s picture

Using a plugin is the right approach for D8. This patch just provides a way for contrib modules to create alternative collections of 'days' or 'months' and to translate those values. It also will create a list of supported calendar systems (i.e. all the plugins provided by core and contrib). So we have a way to swap in different values by calendar and we have a way to indicate which calendar system to use. That's all this patch does and I think doing this as a plugin is the best way to go.

We do NOT want to be using old functions like strtotime() -- they are broken in a million ways. We should be using the new DateTime class, which is what the core component does. Ultimately we probably want to use the new International Date Formatter instead, and the date component patch allows for that.

With this patch, the calendar systems module would implement hook_datetime_calendar_info() to indicate it has more calendars to add, then create each calendar as a plugin, using Gregorian.php as a module. Then each calendar's date parts become available to the system, and all the translation for each of those date parts is handled as well.

There is more to do than this, this is just a start -- a concept of 'calendar' and a translated collection of the appropriate date parts for that calendar that any contrib module can add to.

KarenS’s picture

Also we definitely need to do some work on format_date() to make sure it takes calendars into account, but there is a patch going in to convert the regional settings to CMI and it's making big changes to format_date(), so I don't want to mess with that function until the other patch gets in.

tim.plunkett’s picture

Issue tags:+D8MI, +Datein8

Fixing tags...

sinasalek’s picture

Issue tags:-D8MI, -Datein8

I think you didn't get my point :)
By strotime i didn't mean to actually using it, what i meant was a general "something to time conversion". it can even be a dropdown multi part date widget. but the problem still stands. When we represent the date in non Gregorian in ANYWAY and in forms we also need a generic method to convert it back to time (machine readable)
Drupal already has a text date field on node forms, it won't be converted back to time correctly if format_date returns anything other than a standard Gregorian date string
The solutions i proposed were to address this problem and nothing more.

As i'm sure you already know the different between calendar systems is not just the name of months and days, the whole algorithm is different. some calendar systems even have more parts than the regular year, month, day

Does your patch contain a way make it possible to contributed modules like calendar systems to change the default calendar system ?
Also if this patch is only about new plugin architecture for date then it should probably be address after this one has landed

sinasalek’s picture

Issue tags:-D8MI, -Datein8

Correcting the tags again

sinasalek’s picture

Issue tags:+D8MI, +Datein8

Correcting the tags again

sinasalek’s picture

Issue tags:+D8MI, +Datein8

Karen S #1802278: Add a Date component to core is committed , whould you plz re-roll the patch? it fails on latest 8.x

sinasalek’s picture

Issue tags:-D8MI, -Datein8

#9: 1811912-calendar-translation-9.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+D8MI, +Datein8

The last submitted patch, 1811912-calendar-translation-9.patch, failed testing.

KarenS’s picture

Status:Needs work» Needs review
StatusFileSize
new47.07 KB
PASSED: [[SimpleTest]]: [MySQL] 48,075 pass(es).
[ View ]

Try this.

sinasalek’s picture

Thanks,
I reviewed the patch, generally i like what i saw , well done :)
I have some of questions and then i'm going to write an Iranian plugin for it to try it out in action.
- How does the strings (month names, week names) are going to be extracted from the plugins
- Shouldn't calendar name have a prefix like GregorianCalendar or CalendarGregorian?
- Also i think it's better to use context for all week,month,etc names in Gregorian calendar plugin
- Regarding format_date, i couldn't find any clear information about International Formatter, but i guess you're referring to ISO date formats, plz correct me if i'm wrong? if that's the case then i'm agree with you. But right now all that needs to be done is using the new date classes instead of the default one in format_date function, i already did this in calendar systems module
Also this change causes a problem in forms with text date field like "Authored on" because when the form is submitted Drupal always uses the default Date class to validate the field and since the date is not in Gregorian , validation always fails. also even if the validation passes it can't be converted to timestamp because again Drupal uses the default date class.

sinasalek’s picture

I started writing the Drupal8 version of calendar system by depending on this patch.
How do i override the default calendar system from within a module?

sinasalek’s picture

I implemented the following hook, but when i change the calendar to Iranian it throws "Drupal\Component\Plugin\Exception\PluginException: Plugin (iranian) instance class "Iranian" does not exist"
Class is in module's root folder
i tried giving it absolute path as well, same result
Any idea?

<?php
function calendar_systems_datetime_calendar_info() {
    return array(
   
'iranian' => array(
     
'label' => t('Iranian'),
     
'class' => 'Iranian',
    ),
  );
}
?>
tim.plunkett’s picture

Classes use PSR-0 standards, with namespaces.

See core/lib/Drupal/Core/Datetime/Plugin/Type/Gregorian.php

----
One question, why are these using hooks and not annotations?
If you'd rather switch to using annotations in a follow-up, can that be opened?

sinasalek’s picture

This is the implementation of this hook for Gregorian in core, the question is
what the value of "class" should be if Gregorian class was inside a module
I'm aware that Drupal 8 is using PSR but i couldn't find any documentation
regarding this use case

<?php
function system_datetime_calendar_info() {
  return array(
   
'gregorian' => array(
     
'label' => t('Gregorian'),
     
'class' => '\Drupal\Core\Datetime\Plugin\Type\Gregorian',
    ),
  );
}
?>
tim.plunkett’s picture

<?php
function calendar_systems_datetime_calendar_info() {
    return array(
   
'iranian' => array(
     
'label' => t('Iranian'),
     
'class' => '\Drupal\calendar_systems\Iranian',
    ),
  );
}
?>

That would work if your file was lib/Drupal/calendar_systems/Iranian.php inside your module folder.

sinasalek’s picture

Thanks tim.plunkett i appreciate the help
I added exit; inside the Iranian.php file to make sure that it's included and it does now.
However it get this error now "instance class "\Drupal\calendar_systems\Iranian" does not exist"
This much i could figure out myself, i'm sharing it for anyone else having the same problem :)
New name space should be added to the new Plugin as well.
And it should be something similar to this
Iranian.php :

<?php
namespace Drupal\calendar_systems;
?>
sinasalek’s picture

First real review,
It seems that it has some serious issues or don't know how to use it at all :)
1. Plugins should a base class like DrupalDateTime because when i started using Iranian plugin i encountered several missing methods like canUseIntl()

2. Using DrupalDateTime constructor as factory might be easier to use. so can just do this to get the default calendar object ,
also new $calendar variable can also be added to DrupalDateTime constructor to override the default one when nessassary

<?php
$date_time
= new DrupalDateTime($timestamp, $timezones[$timezone]);
$date_time = new DrupalDateTime($time = 'now', $timezone = NULL, $format = NULL, $settings = array(), 'Iranian');
?>

3. I tried adding new calendar to format_date function , but i didn't know how to pass required parameters to the constructor

<?php
use Drupal\Component\Datetime\DateTimePlus;
use
Drupal\Core\Datetime\Plugin\DateCalendar;
function
format_date($timestamp, $type = 'medium', $format = '', $timezone = NULL, $langcode = NULL) {
  ...
 
$calendar = config('system.calendar')->get('calendar');
 
$plugin = new DateCalendar();
 
$date_time = $plugin->createInstance($calendar, array($timestamp, $timezones[$timezone]));
?>

It seems impossible to pass them due to way createInstance works

<?php
 
public function createInstance($plugin_id, array $configuration) {
   
$plugin_class = $this->getPluginClass($plugin_id);
    return new
$plugin_class($configuration, $plugin_id, $this->discovery);
  }
?>
tim.plunkett’s picture

1) Your plugin should implement DateCalendarInterface. How is that related to canUseIntl()?

2) I don't see how a factory would be more useful

3) You would need to swap out the calendar plugin, it has nothing to do with how format_date() or createInstance() work.

I would recommend reading through http://drupal.org/node/1637614 and its subpages.

sinasalek’s picture

1) Different calendar systems use different algorithms, to be able to represent the date in selected calendar systems format, calendar plugin like the Iranian needs to implement its own date formater. so i copied the format function in DrupalDateTime and placed it in Iranian calendar plugin. however as you mentioned it seems that it's not the way it works. There are many multi calendar libraries out there and all of them work like this. one of the popular one is this http://keith-wood.name/calendarsPicker.html
The usually have gregorianToX and xToGregorian method as well. how do i achieve this using the current architecture ?

2) Using factory like that makes it transparent to anyone uses the new date plugin system. module developer wouldn't even need to know that there is a multi calendar support.

3 ) I read that already, that's why i'm asking. as also described here http://drupal.org/node/1637730 , it's not possible to pass constructor parameters via createInstance method. Unless we create a new factory specially for date that supports that.
However i managed to do it using the following code and it seems to be working even thought it might no be the correct way of doing it :

<?php
  $calendar
= config('system.calendar')->get('calendar');
 
$plugin = new DateCalendar();
 
$plugin_definition = $plugin->getDefinition($calendar);
 
$date_time = new $plugin_definition['class']($timestamp, $timezones[$timezone]);
?>
sun’s picture

This issue has been stated to be a blocker/dependency for #501428: Date and time field type in core, but I cannot really see, read, or understand why that is. @KarenS, any chance to clarify why this is a dependency and cannot be dealt with independently from a Date field type?

KarenS’s picture

@sum, This issue is no longer a blocker. I changed my code to not require this patch and removed the note in the issue summary for #501428: Date and time field type in core that says it is a blocker. This can definitely be dealt with as a separate issue now.

@sinasalek, For now this plugin is simply a way to describe calendar days, months, and weeks and indicate what calendar system is in use. You're trying to jump ahead and use it for more than it was designed to do.

I wanted to start with something simple, the way that dates are displayed, as a first step. This plugin would make that possible. We would still have gregorian values stored in the database and just transform them on display. That is what the IntlDateFormatter does, the IntlDateFormatter is not a library for modifying dates. If the IntlDateFormatter knows what calendar you are using, it will display a date formatted correctly for that calendar, and this plugin makes that possible by giving us a way to indicate what calendar to use.

Plugins allow you to provide alternate calendars that use the calendar interface. That means they provide their own implementation of all the methods in that interface. You could also add new methods like GregorianToX. Core won't use them but your code could. You should not be trying to paste other classes inside it. That is definitely not the way plugins were designed to work.

As you said, you also will need a way to manipulate dates differently. You want to change the way that format_date and DrupalDateTime behave. That's going to require more work but my thought was that the whole process can build on this as a first step. Once we can display dates correctly for different calendars, then we can start to see how to allow other modules to manage date manipulation.

sinasalek’s picture

Thanks for the explanation @KarentS
To me the whole point of this patch is to eliminate the need for a core patch for supporting multi calendars
Currently in Drupal 7 a small patch is required.
However according to what you explained even a bigger patch will be required in Drupal8 because if we want to use the new date plugin system it also has to be hacked
IMHO it doesn't need to be simple because it's not something that we don't know about, there are already very nice multi calendar implementation like jquery world calendar by keith, that address the same issue perfectly. We just need to implement something similar. You actually did most of the job only some changes are required.
Also Intl is not enabled by default since it's a PECL extension. I'm not sure if it's a good idea to rely on that. We should at least be able to implement standalone calendar plugins when such a extension does on exist on server (specially on shared hostings)

Right now in order to have a full multi calendar system (at least for representing dates) there are some limitations that can be overcome without too much trouble, let me know if this makes sense to you (you can consider it the second step) :

1) format_date function to use new date plugin system
2) making it possible for contributed modules to override default calendar system
3) making it possible for date plugins to format date, allowing them to provide alternative even when IntlDateFormatter is not available
4) making it possible for date plugins to validate the submitted dates
5) converting dates back to Gregorian upon submission for core date fields (the rest can be handled by contributed modules)

I've already done all this in calendar systems module, so it's clear how it can be done.
P.S : It these things can be done after the feature freeze then we can focus on the basic functionality only

KarenS’s picture

Assigned:KarenS» Unassigned

OK, I can see we don't really have agreement about how to approach this, so I'm unassigning myself. If you want to propose a different patch, please go ahead. If you propose a patch we can discuss it, it's hard to talk in generalities about this and be sure we all understand what it means.

KarenS’s picture

Also I would say that for core it would be reasonable to assume that the IntlDateFormatter must be available. This matches the approach we are taking in core for other things and it avoids having core trying to do things that PHP can already do. Or perhaps there is something in Symfony we can use (but I suspect not).

Hadi Farnoud’s picture

Where are we with this patch? It's a MUST HAVE for Persian users. I can imagine this being useful for many others

sinasalek’s picture

Hadi, Read comment #36 to #41 to findout what's going on

sinasalek’s picture

@KarenS , since there is only 9 days till feature freeze and i don't have time to write a proper patch till then i move on to this issue #1178342: Allow contributed modules to alter the format_date() function result.

KarenS’s picture

Status:Needs review» Needs work

The linked issue does nothing more than allow other modules to change the format_date function. That doesn't address anything that was raised as an objection to this patch and this patch provides what I think is a much cleaner approach to creating a pluggable system of calendars, where we expressly identify the calendar system to use and the calendar systems that can be used.

I'm marking this needs work because we don't have agreement on this though.

webchick’s picture

FWIW, I believe the progress demonstrated here is sufficiently substantial to qualify for the feature freeze extension: http://buytaert.net/drupal-8-feature-freeze-extended This is a really important feature, and it's also important to get it right.

arlinsandbulte’s picture

Is there a tag signifying "qualifies for the feature freeze extension"?
Might be useful.

webchick’s picture

Sometime tomorrow / over the weekend I'm hoping to create a 9.x release node, which we can then start classifying things that are out of scope. That seems a bit more clear than a tag to me.

sinasalek’s picture

Assigned:Unassigned» sinasalek

That's great, now that we have time with some minor changes i think it will be done

starchild’s picture

Any news on this?

tim.plunkett’s picture

Version:8.x-dev» 9.x-dev

Though this was given an exception for feature freeze in #46, no progress was made in the last three months, and the feature freeze extension window closed yesterday.

klonos’s picture

pity :/

starchild’s picture

Damn :-/

This is quit an important feature.

I wish KarenS would have gone with her original plan.

sinasalek’s picture

If you're interested join this issue #1178342: Allow contributed modules to alter the format_date() function result. (which is the required patch for calendar systems module to work) and help out.
Unfortunately i couldn't find enough time to rewrite the patch in a way that it could also be used for other calendar systems. Karen's patch was mostly useful for multilingual purposes and not multi calendar purpose, i was afraid that if it gets in in that shape we will probably needed several more core patches to have multicalendar instead of only a tiny one.
But not to worry If #1178342: Allow contributed modules to alter the format_date() function result. gets in you can use calendar systems module for all your multi calendar needs without a need to patch the code and i'll see if i can work with Karen on date module for proper multi calendar architecture we both agree. as a matter of fact i already look into date module and it seemed quite possible
And always remember that you can always help by joining in anyway you can (documenting , patching , reviewing, testing, reporting bugs, etc)

klonos’s picture

Issue summary:View changes

Adjust the description to match the current state of the patch.

catch’s picture

Version:9.x-dev» 8.1.x-dev

API addition.