Split "Display author and date information" into 2 vars/options

(Merging "node system" and "node.module" components for 8.x; disregard.)

i'm working on it

Ok, now that issue is assigned to me

Sending patch

Not working for me.

+++ b/core/modules/node/src/NodeTypeForm.php
@@ -181,11 +181,25 @@ public function form(array $form, FormStateInterface $form_state) {
+    // $form['display']['display_submitted'] = array(
+    //   '#type' => 'checkbox',
+    //   '#title' => t('Display author and date information'),
+    //   '#default_value' => $type->displaySubmitted(),
+    //   '#description' => t('Author username and publish date will be displayed.'),
+    // );

When uploading code that is not needed you should remove completely.

+++ b/core/modules/node/src/Entity/NodeType.php
@@ -102,7 +102,9 @@ class NodeType extends ConfigEntityBundleBase implements NodeTypeInterface {
-  protected $display_submitted = TRUE;
+  protected $display_submitted = FALSE;
+  protected $display_author = FALSE;
+  protected $display_date = TRUE;

Split then up and make for each an docblock.

I hope this is the right fix.

Looks a lot better. Custom content type is working, but we should cover the default Article and Page. Search for node.type.article.yml and node.type.page.yml should be in profiles and make the appropriate changes.

. Bah, ignore, I am tired and been up too long staring at this screen... I am doing this right now in my themes already using trans, it works pretty good, I have it like this, sort of thing:

{{ author_picture }}
{% trans %}Submitted by {{ author_name }} on {{ date }}{% endtrans %}
{{ metadata }}

I change default article and page also made some changes on tests.

Ok i commit local files from /sites and i cut that trash form patch,
so patch is corrupt, i hope this one is not.

An needs review...

So lets go over the test fails.

1. The first is in NodeCreationTest:
looks like we use $node_type->setDisplaySubmitted(TRUE); but we need to do this for author and date separate. So lets try it. Lets make these test green.

Then we will see for the others.

Also when you create interdiffs create them from the previous patch. Its harder to review like this.

So lets go over the test fails.

1. The first is in NodeCreationTest:
looks like we use $node_type->setDisplaySubmitted(TRUE); but we need to do this for author and date separate. So lets try it. Lets make these test green.

Then we will see for the others.

Also when you create interdiffs create them from the previous patch. Its harder to review like this.

I made changes in NoteCreationTest.php

We need to be careful to maintain backward compatibility here. We are leaving display_submitted in config schema and in templates. That's OK. I think we could do few more things with regards to that:
- Have an update hook that checks existing node types and sets both display_date and display_author based on the current setting of display_submitted. Without that we might break any existing nodes that are using templates provided by core which we just changed.
- Since we removed display_submitted from the configuration form it would make sense to set it on save anyway (display_submitted = display_author && display_date or something similar)
- I'd also declare NodeTypeInterface::displaySubmitted() and setDisplaySubmitted() as deprecated and ready for removal in D9.

When this gets in we need to create an issue for D9 to remove BC layer (display_submitted from schema and template, etc.).

+++ b/core/modules/node/templates/node.html.twig
@@ -75,13 +75,28 @@
+  {% if display_author and display_date %}
...
+    </footer>>
+  {% elseif display_author %}
+    <footer>
+      {{ author_picture }}
+      <div{{ author_attributes }}>
+        {% trans %}Submitted by {{ author_name }}{% endtrans %}
+        {{ metadata }}
+      </div>
+    </footer>
+  {% elseif display_date %}
+    <footer>
+      <div{{ author_attributes }}>
+        {% trans %}Submitted on {{ date }}{% endtrans %}
+        {{ metadata }}
+      </div>

Im wondering if we could do this more along the lines of:

{% if display_author or display_date %}
  <footer>
{% endif %}

  {% if display_author %}
    {{ author_picture }}
  {% endif %}
  
  {% if display_author or display_date %}
    <div{{ author_attributes }}>
      {% if display_author and display_date %}
        {% trans %}Submitted by {{ author_name }} on {{ date }}{% endtrans %}
      {% elseif display_author %}
        {% trans %}Submitted by {{ author_name }}{% endtrans %}
      {% elseif display_date %}
        {% trans %}Submitted on {{ date }}{% endtrans %}
      {% endif %}
      {{ metadata }}
    </div>
  {% endif %}
        
{% if display_author or display_date %}
  </footer>
{% endif %}

Apologies for any errors, I did this very quickly :)

I try this patch just for some tests.

More tests

Just one more test.

Trying to fix SearchExactTest.

Trying to fix RowEntityRenderersTest.

Trying to fix all.

Trim trailing white space from patch.

We can't expand the php interfaces in a minor release. We'll either need another approach, or this has to wait until 9.0.0

@tim.plunkett NodeTypeOnSteroidsInterface extends NodeTypeInterface?

+++ b/core/modules/node/node.module
@@ -601,10 +601,13 @@ function template_preprocess_node(&$variables) {
+
   // Used by RDF to add attributes around the author and date submitted.
   $variables['author_attributes'] = new Attribute();
-  $variables['display_submitted'] = $node_type->displaySubmitted();
-  if ($variables['display_submitted']) {
+
+  $variables['display_author'] = $node_type->displayAuthor();
+  $variables['display_date'] = $node_type->displayDate();
+  if ($variables['display_author']) {

I think we need a documented decision if and under which conditions it is allowed to add additional methods to an interface.

Something that I heard is that it's OK to add methods for things where it is highly unlikely that they are replaced completely and there's a base class that people are expected to implement.

I guess it's safe to assume that if someone would really switch out the NodeType class then he will probably extend from NodeType to do so. Except of course if he specifically change displaySubmitted() but I'm not sure if someone would do that.

I'm really unsure how such an additional interface would work for something like this.

The challenge is either way that we need backwards compatibility in all directions. We need to support old config (we could write an update function but that will still break default config in modules). We also need to support templates that use the old property. And if we have a new interface, we need to do all that conditionally (Consider that the class/interface is just a way to access the config information. That is not replaceable, we add support there. Which kind of those that it is pointless to have an optional interface.)

I think the easiest way to achieve that is to:
1. still set display_submitted if both new methods return TRUE
2. In those methods, also check the old property if the new ones don't exist.

That's easy enough. But.. here's the real problem: $variables and hook_template_preprocess_node() is an API too. What happens if someone sets display_submitted to FALSE conditionally and expects that to still work? What if someone sets the new properties in a module but some other site is still using the old variable in a template? I'm not sure.

When talking about a similar thing with @catch a while back, he suggested to have a place where we can check if the variable changed (we'd store an original an if it changed, we'd disable the other two flags as well). That needs to happen after all other preprocess hooks, so that's not something we can do atm.

BC is hard :) And results in ugly/complicated code, no way around that.

+++ b/core/modules/rdf/src/Tests/StandardProfileTest.php
@@ -279,7 +279,8 @@ protected function doPageRdfaTests() {
     // @todo Clean-up standard profile defaults.
     $node_type = NodeType::load('page');
-    $node_type->setDisplaySubmitted(TRUE);
+    $node_type->setDisplayAuthor(TRUE);
+    $node_type->setDisplayDate(TRUE);
     $node_type->save();

I'm wondering if we should explicitly *not* change these tests, exactly to ensure that we are backwards compatible. we can have a follow-up for later to clean it up.

So, we now can extend entity interfaces, so this would be fine to add.

However, an idea that I had recently would be use standard manage display/formatter settings to control this, instead of new node type level settings. All we need to do for that is to allow configuring those two fields and then we have 100% the same as this issue is adding.

One could argue that it's a bit weird with being able to control the weight that then wouldn't work and it depends on the template, but the first is already true for node links ( and I would *love* to do it for node title as well) and the second is true also for node type settings.

Still need t figure out how BC would work exactly, with other option.

Looks like an issue for that actually already existed for a long time: #1399990: Remove "Display author and date information" in Display Settings in favor of listing Author and Date in Manage Displays. Closing as duplicate.