Updated: Comment #17

Problem/Motivation

The ' Display author and date information.' and other node-type settings for content types are ignored when turned off
Steps to reproduce

  • Create a content type
  • Uncheck ' Display author and date information.'
  • Create a node of that type
  • View node in bartik theme
  • Result: Screenshot 2013-10-13 17.24.53.png

Proposed resolution

Ensure that node options and submitted variables are written as booleans to the node-type config entity.
Update node-type schema to flag these as booleans.
Update NodeType::getExportedProperties to correctly type these properties (ConfigEntityBase ignore config schemas).
Ensure config() isn't used to access node settings, use the config entity instead.

Remaining tasks

Reviews

User interface changes

None

API changes

None

None

Original report by @Mark Carver

"Display author and date information." checkbox on content types does not work. Introduced by #111715: Convert node/content types into configuration.

Steps to reproduce:

  1. Create new page and submit.
  2. Toggle "Display author and date information." on content type and watch it not change.

Need to test for this in the future.

Comments

Title:Submitted by always shows on node types"Submitted by" always shows on node types
Status:Active» Needs review
StatusFileSize
new837 bytes
FAILED: [[SimpleTest]]: [MySQL] 57,714 pass(es), 21 fail(s), and 0 exception(s).
[ View ]

Here's the fix, we need tests.
Also, the comment above this line says to not load the config entity, but that would have prevented this bug...

Status:Needs review» Needs work

The last submitted patch, node-2053461-1.patch, failed testing.

Status:Needs work» Needs review
Issue tags:-Needs tests
StatusFileSize
new4.71 KB
FAILED: [[SimpleTest]]: [MySQL] 57,714 pass(es), 7 fail(s), and 0 exception(s).
[ View ]
new1.43 KB
FAILED: [[SimpleTest]]: [MySQL] 57,907 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

This is a bigger problem than I thought.
The default value is only applied on the form level, not the entity level.

Also, tests can create nodes of node types that don't exist.
Finally, NodeTestBase creates a page node type that doesn't match the expectations of the standard profile config.

Issue tags:+Needs tests

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeCreationTest.php
@@ -56,6 +56,8 @@ function testNodeCreation() {
+    $this->assertNoText(t('Submitted by !username on !datetime', array('!username' => $this->loggedInUser->getUsername(), '!datetime' => format_date($node->getCreatedTime()))));

Can't really see with just the context lines, but are we also switching it on and testing that it shows up too?

Issue tags:-Needs tests

wtf... tagging is screwed up. This wasn't even in the field.

Status:Needs review» Needs work

The last submitted patch, node-2053461-3-PASS.patch, failed testing.

Assigned:Unassigned» tim.plunkett

Yep. Also need to fix those other tests.

Status:Needs work» Needs review
StatusFileSize
new2.83 KB
new6.72 KB
FAILED: [[SimpleTest]]: [MySQL] 57,835 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

Fixed, expanded tests. Should be good now.

Status:Needs review» Needs work

The last submitted patch, node-2053461-8.patch, failed testing.

Assigned:tim.plunkett» Unassigned
Status:Needs work» Needs review
StatusFileSize
new803 bytes
new6.78 KB
PASSED: [[SimpleTest]]: [MySQL] 58,347 pass(es).
[ View ]

Last one.

Status:Needs review» Needs work

\Drupal:: in stead of Drupal::

Status:Needs work» Needs review
StatusFileSize
new5.86 KB
FAILED: [[SimpleTest]]: [MySQL] 58,908 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

rerolled, lot of things have moved in since august.

Status:Needs review» Needs work

The last submitted patch, node-2053461-13.patch, failed testing.

I vote we close this tackle as part of bigger issue in #2111091: Node type settings such as published state, promoted state, author information cannot be turned off, the issue also effects the status, promote and revision options. Thoughts?

This addresses those, just not in the issue title... But as long as the approaches are merged, I don't care. The two issues have completely different code ATM.
Let's chat on IRC later.

Sure.
Fwiw I tested this manually before my 'vote', it fixes submission details, but not promoted our published so that's why I thought it had a narrower scope.

Status:Needs work» Needs review
StatusFileSize
new12.39 KB
FAILED: [[SimpleTest]]: [MySQL] 59,057 pass(es), 33 fail(s), and 0 exception(s).
[ View ]
new7.62 KB

Patch combines #13 and #2111091: Node type settings such as published state, promoted state, author information cannot be turned off which was closed as duplicate of this.
Issue summary updated to reflect wider scope.
Lets see how many fails left.

Title:"Submitted by" always shows on node typesNode type settings such as published state, promoted state, create revision and author information cannot be turned off

New title

Status:Needs review» Needs work

The last submitted patch, node-post-settings-2053461.18.patch, failed testing.

Priority:Normal» Major
Status:Needs work» Needs review
StatusFileSize
new11.83 KB
FAILED: [[SimpleTest]]: [MySQL] 58,800 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
new876 bytes

This is surely major with the wider scope.

Status:Needs review» Needs work

The last submitted patch, node-post-settings-2053461.20.patch, failed testing.

StatusFileSize
new2.44 KB

Missed a call to setDefaultRevision in EditFieldForm::init
I've made some new git alias toys so here's hoping they worked

From my .gitconfig

  interdiff = !git diff > ~/patches/interdiff.txt
  interdiff-stg = !git diff --staged > ~/patches/interdiff.txt
  genpatch = "!sh -c "br=`git symbolic-ref HEAD|sed s#refs/heads/##`; git diff origin/8.x > ~/patches/\\${br}.$1.patch""
  rebase8x = !git fetch origin && git rebase origin/8.x
  merge8x = !git fetch origin && git merge origin/8.x

Use like so - assuming branches are named {description}-{issuenumber}

# generate an interdiff based on current unstaged changes
git interdiff
# generate an interdiff based on current staged changes
git interdiff-stg
# rebase 8.x
git rebase8x
# merge 8.x
git merge8x
# generate a patch against 8.x with format {description}-{branch}.{comment}.patch
# here comment is 22
git genpatch 22

Status:Needs work» Needs review
StatusFileSize
new13.6 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

and how about a patch this time?

Status:Needs review» Needs work

+++ b/core/modules/system/lib/Drupal/system/Tests/Common/UrlTest.php
@@ -172,7 +172,7 @@ function testLinkRenderArrayText() {
+   * @papam $class

Typo

Status:Needs work» Needs review
StatusFileSize
new13.1 KB
PASSED: [[SimpleTest]]: [MySQL] 59,141 pass(es).
[ View ]
new516 bytes

Fixes #25

  1. +++ b/core/modules/edit/lib/Drupal/edit/Tests/EditLoadingTest.php
    @@ -246,7 +246,10 @@ public function testUserWithPermission() {
    -      $this->container->get('config.factory')->get('node.type.article')->set('settings.node.options', array('status', 'revision'))->save();
    +      $this->container->get('config.factory')->get('node.type.article')->set('settings.node.options', array(
    +        'status' => TRUE,
    +        'revision' => TRUE,
    +      ))->save();

    Can we use entity_load('node_type', 'article') and set this properly?

  2. +++ b/core/modules/edit/lib/Drupal/edit/Tests/EditLoadingTest.php
    @@ -246,7 +246,10 @@ public function testUserWithPermission() {
    +      $this->container->get('config.factory')->get('node.type.article')->set('settings.node.options', array(
    +++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php
    @@ -288,6 +288,10 @@ protected function drupalCreateNode(array $settings = array()) {
    +    if (!$this->container->get('entity.query')->get('node_type')->condition('type', $settings['type'])->execute()) {

    We're apparently to use \Drupal over $this->container in tests.

  3. +++ b/core/modules/node/lib/Drupal/node/Entity/NodeType.php
    @@ -211,4 +211,38 @@ public static function postDelete(EntityStorageControllerInterface $storage_cont
    +  public function getExportProperties() {
    +    $properties = parent::getExportProperties();
    +    // Ensure that submitted and options are booleans.
    +    $properties['settings']['node']['submitted'] = (bool) $properties['settings']['node']['submitted'];
    +    foreach ($properties['settings']['node']['options'] as $key => $value) {
    +       $properties['settings']['node']['options'][$key] = (bool) $value;
    +    }
    +    return $properties;
    +  }

    It really sucks that we have to do this.

  4. +++ b/core/modules/node/lib/Drupal/node/NodeFormController.php
    @@ -43,7 +43,7 @@ protected function prepareEntity() {
    -          $node->$key = (int) in_array($key, $this->settings['options']);
    +          $node->$key = (int) (isset($this->settings['options'][$key]) ? $this->settings['options'][$key] : 0);

    This cast is suspicious. Can we remove the getExportProperties if we remove this? And make the default FALSE not 0

  5. +++ b/core/modules/node/lib/Drupal/node/NodeTypeFormController.php
    @@ -118,7 +112,7 @@ public function form(array $form, array &$form_state) {
    +      '#default_value' => MapArray::copyValuesToKeys(array_keys(array_filter($node_settings['options']))),

    O_o
    I guess this is needed? Not sure why.

  6. +++ b/core/modules/node/node.module
    @@ -702,8 +702,7 @@ function template_preprocess_node(&$variables) {
       // Avoid loading the entire node type config entity here.
    -  $submitted = \Drupal::config('node.type.' . $node->bundle())->get('settings.node.submitted') ?: TRUE;
    -  if ($submitted) {
    +  if (\Drupal::config('node.type.' . $node->bundle())->get('settings.node.submitted')) {

    In a follow-up, we should profile using the full entity here, because breaking our own rule of not using config() for entities sucks.

Issue summary:View changes

Updated issue summary

Hello. I see you are investigating? Could you make a small patch to make it workable? I see this problem lives too long. Let's use work around.

Thank you.

Issue summary:View changes
Status:Needs review» Needs work
Issue tags:+Needs reroll

Patch has test coverage so it needs re-roll and address #27

@likin You could try to re-roll it.

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new8.43 KB
new12.53 KB
FAILED: [[SimpleTest]]: [MySQL] 59,828 pass(es), 14 fail(s), and 3 exception(s).
[ View ]

Merge with following changes:
- Addressed #27 review
- converted standard profile config for node types
- added @todo to clean-up rfd config for page node type, somehow comment was removed, probably in node type cmi conversion

Status:Needs review» Needs work

The last submitted patch, 31: 2053461-node-options-31.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.08 KB
new14.61 KB
PASSED: [[SimpleTest]]: [MySQL] 60,093 pass(es).
[ View ]

Patch adds fixes broken tests, that affected that "submitted" needs node-type entity config to display itself
Also #27.6 seems pointed to that, so the right fix would be to decide do we really need this this setting depend on node-type setting or use entity display somehow...

+++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php
@@ -288,6 +288,10 @@ protected function drupalCreateNode(array $settings = array()) {
+    // If the specified node type does not exist, create it.
+    if (!$this->container->get('entity.query')->get('node_type')->condition('type', $settings['type'])->execute()) {
+      $this->drupalCreateContentType(array('type' => $settings['type'], 'name' => $this->randomString()));
+    }

Removal of this shows that node needs node type entity to properly display submitted variable. And only that!

PS: Can't reproduce failures in SimpleTestTest and ShortcutLinksTest locally

StatusFileSize
new5.32 KB
new12.52 KB
FAILED: [[SimpleTest]]: [MySQL] 59,986 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Clean-up patch

StatusFileSize
new5.33 KB
new12.53 KB
PASSED: [[SimpleTest]]: [MySQL] 60,067 pass(es).
[ View ]

right patch with interdiff

The last submitted patch, 34: 2053461-node-options-34.patch, failed testing.

Status:Needs review» Reviewed & tested by the community

In my case it works well.

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

Component:node.module» node system

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

Status:Fixed» Closed (fixed)

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