Wrong text if no content types are available

It was just the wrong render element used in the build function. Here's the patch!

This is a duplicate

Duplicate of which issue? It's similar to https://www.drupal.org/node/2454309, but that deals with content, not content types.

@larowlan, if you believe this is a duplicated please provide the link for the duplicated issue.

@drubb, great work it seem to be working fine.

I have tested this patch and seem to be working fine. I have attached an image of the results of this patch.

Let's add an assertion to a test for this.

@alexpott,

Its unclear to me what is the next step. If you can provide me a link for what type of testing it needs I might be able to help testing this patch.

How about something like this for a test? I created a new test class for node module, b/c I didn't want to inherit the setup from NodeTestBase. If I did I would have had to delete the Page and Article types which are defined there. Let me know if I'm off base. Otherwise, this seems like a good enough test.

epophoto and I tested this at DrupalConLA.

I tested this at DrupalConLA

Add related issue

This is exactly the kind of thing that we're looking for, except that by making it its own test extending from WebTestBase it's going to incur a Drupal installation/teardown, which is quite expensive. Is it possible to look for an existing test that's already hitting the admin/structure/types page where we could simply add the assertion there?

Slim picking for tests to piggyback off of, nearly all of them create content types as part of setup.

Few not-so-great candidates I found

NodeBodyFieldStorageTest
NodeCacheTagsTest
NodeFieldAccessTest
NodeFieldOverrridesTest
NodeListBuilderTest

None of these really have anything to do with content types though and none of them seem suitable generic.

@all as suggested by @tim.plunkett can we please combine this issue with #2490290: Subclasses of EntityListBuilder incorrectly override the #empty message and fix all of the subclasses of EntityListBuilder at once?

@smccabe could NodeTypeTest or NodeAdminTest be candidate? we simply need to delete the Page and Article node types before test the empty message

@willzyx I agree one of those makes a lot more sense logically, I just wasn't sure about undoing the setup, possibly screwing up additional tests that get added. What would be the best option, to remove the content types, run the test and then re-add them so additional tests still have the correct setup?

Changed #9 patch according to @webchick said. I have added the test in NodeTypeTest and also changed NodeTypeListBuilder as per #2490290: Subclasses of EntityListBuilder incorrectly override the #empty message.

Oops!!! Forgot to upload the patch.

1. +++ b/core/modules/node/src/Tests/NodeTypeTest.php
   @@ -22,7 +23,7 @@ class NodeTypeTest extends NodeTestBase {
   -  public static $modules = array('field_ui');
   +  public static $modules = array('node', 'field_ui');
   

   Should be short array syntax here.

2. +++ b/core/modules/node/src/Tests/NodeTypeTest.php
   @@ -202,6 +203,20 @@ public function testNodeTypeFieldUiPermissions() {
   +    $this->assertRaw(t('No content types available. <a href="@link">Add content type</a>.', array(
   +        '@link' => Url::fromRoute('node.type_add')->toString()
   +      )), 'Empty text when there are no content types in the system is correct.');
   

   same here (short array syntax)

As NodeTypeTest class extends the NodeTestBase, and you can see that in setup, Article and Page content types are created. So in your test case, You should first delete those two bundles.

Also I think that the new test case method name be testNoNodeContentType (just thought).

updated test to remove the content types and then re-add after the tests are done.

@joshi.rohit100 im not sure why you suggest short array syntax for those sections, when I looked at any other tests they don't use short array syntax either. Is this something new that is being changed for D8?

@smccabe

I think the working policy is to use short array syntax if the array in question is in the issue scope.

Ah I see, try to update if you're doing new stuff but not doing the mass convert of everything. I will update the patch when I get a chance.

https://www.drupal.org/node/2135291

Fixed the short array syntax.

I have tested this patch and seem to be working fine, I have attached an image of the results of this patch:

And the result of running phpunit:

./vendor/bin/phpunit --group node

Remove embedding images and status update

I am just curious why we re-added the node module in NodeTypeTest class as it is already added in parent class ?

@smccabe : I just suggested the short array syntax as it is new patch policy.

removed unneeded node module included

+++ b/core/modules/node/src/Tests/NodeTypeTest.php
@@ -202,6 +203,34 @@ public function testNodeTypeFieldUiPermissions() {
+    //Add content types back in so other tests have the expected environment
+    $this->drupalCreateContentType([
+      'type' => 'page',
+      'name' => 'Basic page',
+      'display_submitted' => FALSE,
+    ]);
+    $this->drupalCreateContentType(['type' => 'article', 'name' => 'Article']);

Not needed.

@jmolivas running PHPUnit doesn't run the test added by this patch - unfortunately. It's a web test.

Thanks @joshi.rohit100 RTBC when turns green.

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 34d3c8c and pushed to 8.0.x. Thanks!