#501428: Date and time field type in core added datetime as a hard dependency of node.module to improve the "Authored on" field.

In addition to being unnecessarily tight coupling, since the testing profile always enabled node.module, datetime is added to every test run.

Many modules might want to improve the node form, datetime can serve as an example of how to do that.

Files: 
CommentFileSizeAuthor
#38 2006484-38--correct_comment.patch389 bytesdrunken monkey
PASSED: [[SimpleTest]]: [MySQL] 57,010 pass(es).
[ View ]
#31 borked_datetime_format.png6.76 KBPancho
#31 interdiff_28_31.txt2.91 KBPancho
#31 2006484-second_followup-31.patch3.41 KBPancho
PASSED: [[SimpleTest]]: [MySQL] 58,220 pass(es).
[ View ]
#28 2006484-second_followup-28.patch3.24 KBPancho
FAILED: [[SimpleTest]]: [MySQL] 57,671 pass(es), 0 fail(s), and 316 exception(s).
[ View ]
#29 borked_date_format.png86.94 KBPancho
#25 2006484-followup.patch1.31 KBamateescu
PASSED: [[SimpleTest]]: [MySQL] 57,631 pass(es).
[ View ]
#25 author-date-before.png11.09 KBamateescu
#25 author-date-after.png13.96 KBamateescu
#19 datetime-2006484-19.patch7.62 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 57,242 pass(es).
[ View ]
#19 interdiff.txt780 bytestim.plunkett
#13 node-2006484-13.patch7.39 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 57,535 pass(es).
[ View ]
#12 drupal-2006484-12.patch7.38 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 57,224 pass(es).
[ View ]
#12 interdiff.txt934 bytesdawehner
#11 interdiff.txt841 bytestim.plunkett
#11 datetime-2006484-11.patch7.36 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 55,881 pass(es).
[ View ]
#7 datetime-2006484-7.patch7 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 57,673 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#7 interdiff.txt2.91 KBtim.plunkett
#3 datetime-2006484-3.patch4.09 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 57,204 pass(es), 25 fail(s), and 0 exception(s).
[ View ]
#3 interdiff.txt352 bytestim.plunkett
#1 datetime-2006484-1.patch4.05 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new4.05 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

This restores NodeFormController to its original state, and uses hook_form_alter() and hook_node_prepare() to make the exact same changes.

If you use standard.profile, you will still see the awesome new datetime picker.

Instead of deleting, I emptied out the update hook. I think that's the standard practice?

+++ b/core/modules/node/node.installundefined
@@ -808,11 +808,9 @@ function node_update_8013() {
+ * Empty update.

Is it worth to explain why this update is empty? Why not just remove it totally?

StatusFileSize
new352 bytes
new4.09 KB
FAILED: [[SimpleTest]]: [MySQL] 57,204 pass(es), 25 fail(s), and 0 exception(s).
[ View ]

There is a precedent for not removing update hooks. I tweaked this one to mimic image_update_8001(), which is also empty.

StatusFileSize
new2.91 KB
new7 KB
FAILED: [[SimpleTest]]: [MySQL] 57,673 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Well, crap. I hate mucking with testModuleEnableOrder().
Maybe I should kill comment's dependency on datetime too...

StatusFileSize
new7.36 KB
PASSED: [[SimpleTest]]: [MySQL] 55,881 pass(es).
[ View ]
new841 bytes

Missed one part change in the dependency test.

StatusFileSize
new934 bytes
new7.38 KB
PASSED: [[SimpleTest]]: [MySQL] 57,224 pass(es).
[ View ]

This feels more sane.

StatusFileSize
new7.39 KB
PASSED: [[SimpleTest]]: [MySQL] 57,535 pass(es).
[ View ]

Reroll for Drupal::state() and update_module_enable()

+++ b/core/modules/datetime/datetime.moduleundefined
@@ -1120,3 +1120,23 @@ function datetime_range_years($string, $date = NULL) {
+function datetime_node_prepare($node) {

Can and should be typehint here?

StatusFileSize
new780 bytes
new7.62 KB
PASSED: [[SimpleTest]]: [MySQL] 57,242 pass(es).
[ View ]

Yes! admin/content added a NodeBCDecorator, so its safe to typehint.

Status:Needs review» Reviewed & tested by the community

Cool. Thank you very much

Status:Reviewed & tested by the community» Needs work
Issue tags:-Datein8, -Test suite performance

The last submitted patch, datetime-2006484-19.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Datein8, +Test suite performance

#19: datetime-2006484-19.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

failed to checkout from [git://git.drupal.org/project/drupal.git]

Status:Reviewed & tested by the community» Fixed

Committed 9806049 and pushed to 8.x. Thanks!

Status:Fixed» Needs review
StatusFileSize
new13.96 KB
new11.09 KB
new1.31 KB
PASSED: [[SimpleTest]]: [MySQL] 57,631 pass(es).
[ View ]

This patch introduced two (identical) notices on the node edit page:

Notice: Array to string conversion in datetime_form_node_form_alter() (line 1132 of core\modules\datetime\datetime.module).

While I was there, I also had to fix theme_datetime_wrapper() to actually use the description provided by datetime_form_node_form_alter(). Attached the before/after screenshots.

Before:
author-date-before.png

After:
author-date-after.png

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Fixed

Committed 033451a and pushed to 8.x. Thanks!

StatusFileSize
new3.24 KB
FAILED: [[SimpleTest]]: [MySQL] 57,671 pass(es), 0 fail(s), and 316 exception(s).
[ View ]

This doesn't seem to work with DateTime disabled and intl being available.

In this case, the custom, hard-coded formatter completely borks format_date(), so when editing any node I 'm getting now a

Warning: date_format() expects parameter 1 to be DateTime, boolean given in Drupal\node\NodeFormController->form() (line 195 of core/modules/node/lib/Drupal/node/NodeFormController.php).

Subsequently, editing nodes doesn't work anymore, therefore critical.

datetime_form_node_form_alter() did a:

<?php
$config
= Drupal::config('system.date');
$format = $config->get('formats.html_date') . ' ' . $config->get('formats.html_time');
?>

and we have to do that just as well, not only as a DateTime form alter, but in any case.

Here's a very quick fix. Seemed to work, but I'm in a hurry, so can't do more testing now.

Note that datetime_default_format_type() despite its name lives in common.inc, so we still not need Datetime.

Priority:Normal» Critical
Status:Fixed» Needs review
StatusFileSize
new86.94 KB

Forgot to add the screenshot. Here it is.

Status:Needs review» Needs work

+++ b/core/modules/datetime/datetime.moduleundefined
@@ -1137,13 +1137,9 @@ function datetime_range_years($string, $date = NULL) {
+  $form['author']['date']['#description'] = t('Format: %format. Leave blank to use the time of form submission.', array('%format' => datetime_format_example($date_format)));

It appears that $date_format would not be defined at this point...

Status:Needs work» Needs review
StatusFileSize
new3.41 KB
PASSED: [[SimpleTest]]: [MySQL] 58,220 pass(es).
[ View ]
new2.91 KB
new6.76 KB

Thank you, that's true.
Took another approach making the field a real 'datetime' field by merging in the defaults, so we can use datetime_html5_format().

However, with Datetime enabled, this still doesn't work, see new screenshot with Datetime enabled:
Screenshot
Also note, that the field itself should be prefilled with the current value.
It seems, there's more wrong, but that's not bound to my patch. It was wrong before. Unfortunately I have no time for investigating.

So all this patch does is making it work again in setups without Datetime enabled.

Status:Needs review» Needs work

Assigned:tim.plunkett» Unassigned

Title:Remove dependency on datetime from node[Followup] Remove dependency on datetime from node

Next time plz open new issues

Component:datetime.module» node.module

@ParisLiakos:
Sorry, but it seems quite inconsistent, in which cases a new issue is opened and when not.
IMHO it's correct to stay on the main issue if the followup is critical, so not getting it completely fixed until release would warrant a rollback of the main patch. But I'm fine with any other policy.

Regarding the bug, I'll be looking into this tonight, because it's extremely annoying.
Next step would be #2027959: Remove dependency on datetime from comment.

+++ b/core/modules/node/lib/Drupal/node/NodeFormController.phpundefined
@@ -42,7 +42,10 @@ protected function prepareEntity() {
-      $node->date = format_date($node->created, 'custom', 'Y-m-d H:i:s O');
+      $config = \Drupal::config('system.date');
+      $format_type = datetime_default_format_type();
+      $format = $config->get('formats.html_date.pattern.' . $format_type) . ' ' . $config->get('formats.html_time.pattern.' . $format_type);
+      $node->date = format_date($node->created, 'custom', $format);

Isn't $node->date just about getting the node create time stored purely for form related functionality. Based on that, I don't see a reason to load the date format.

Title:[Followup] Remove dependency on datetime from nodeRemove dependency on datetime from node
Priority:Critical» Normal
Status:Needs work» Fixed

Seems like #2000384: php-intl module causes problems with Date and Time fields when editing Basic Page covers the regression as a followup.
Reverting this issue to the pre-#28 state.

Status:Fixed» Needs review
StatusFileSize
new389 bytes
PASSED: [[SimpleTest]]: [MySQL] 57,010 pass(es).
[ View ]

It puts me a bit off that no-one else seems to have noticed, but the new comment for node_update_8014() references a completely unrelated issue, #1836392: In the Views UI, the interaction pattern of “All displays”/ “Override this display” is confusing . Had to use git blame to find this one.
Patch attached for fixing this, unless it is somehow on purpose.

Status:Needs review» Reviewed & tested by the community

Yeah, that makes sense.

Status:Reviewed & tested by the community» Fixed

Committed/pushed the follow-up.

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