Problem/Motivation

From @David_Rothstein in #630446: Allow SimpleTest to test the non-interactive installer:

Note that if I switched this test to using the standard install profile and additionally searched for "Welcome to [site:name]" on the front page, it would probably reveal a bug since right now in Drupal 8 it seems to be hardcoded to say "Welcome to Drupal" - not sure if there's an issue for that.

Proposed resolution

Use the [site:name] token for the frontpage title.

CommentFileSizeAuthor
#19 drupal-1963268-19.patch6.27 KBdawehner
#19 interdiff.txt3.32 KBdawehner
#14 drupal-1963268-14.patch5.17 KBdawehner
#14 interdiff.txt1.65 KBdawehner
#10 views-1963268-10.patch4.81 KBxjm
#8 views-1963268-8.patch807 bytesxjm
#8 interdiff.txt807 bytesxjm
#5 drupal-1963268-5.patch4.81 KBdawehner
#5 drupal-1963268-5-test.patch3.62 KBdawehner
#1 site_name_token.png29.5 KBxjm
frontpage-title-token.patch505 bytesxjm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
FileSize
site_name_token.png29.5 KB

With patch applied:
site_name_token.png

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

BTW, I confirmed that D7 apparently did say "Welcome to mysite" rather than "Welcome to Drupal".

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Issue tags: +Needs tests

That means we're missing tests for that old behavior.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Assigned: xjm » dawehner

@dawehner and I just discussed this and it looks like we didn't actually add any tests for this view at all. @dawehner is looking into it.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Assigned: dawehner » Unassigned
FileSize
drupal-1963268-5-test.patch3.62 KB
drupal-1963268-5.patch4.81 KB

Spotted another bug: we ordered first by date and then by sticky ascending, which is pretty useless, if not really wrong.

Status: Needs review » Needs work

The last submitted patch, drupal-1963268-5-test.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review

And back.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
FileSize
interdiff.txt807 bytes
views-1963268-8.patch807 bytes

Tests look great, and the new sorting is much better as well.

+++ b/core/modules/node/lib/Drupal/node/Tests/Views/FrontpageTest.phpundefined
@@ -0,0 +1,117 @@
+    // Create some nodes on the frontpage view. Add more then 10 nodes in order
+    // to enable paging.

Fixed in attached: s/then/than

Status: Needs review » Needs work

The last submitted patch, views-1963268-8.patch, failed testing.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
views-1963268-10.patch4.81 KB

Erm. Real patch this time.

Status: Needs review » Needs work
Issue tags: -Quick fix, -VDC

The last submitted patch, views-1963268-10.patch, failed testing.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs work » Needs review
Issue tags: +Quick fix, +VDC

#10: views-1963268-10.patch queued for re-testing.

olli’s picture

olli CreditAttribution: olli commented

Great work.

+++ b/core/modules/node/lib/Drupal/node/Tests/Views/FrontpageTest.php
@@ -0,0 +1,117 @@
+use Drupal\views\Tests\ViewUnitTestBase;

Not used.

+++ b/core/modules/node/lib/Drupal/node/Tests/Views/FrontpageTest.php
@@ -0,0 +1,117 @@
+    // Create some nodes which aren't on the frontpage.
+    for ($i = 0; $i < 5; $i++) {
+      $values = array();
+      $values['type'] = 'article';
+      $values['title'] = $this->randomName();
+      $values['status'] = TRUE;
+      $values['promote'] = FALSE;
+      $this->nodeStorageController->create($values)->save();
+    }

What about making one of these nodes sticky, promoted and unpublished?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
interdiff.txt1.65 KB
drupal-1963268-14.patch5.17 KB

Not used.

Is there any magical trick you apply to catch these things? Awesome!

What about making one of these nodes sticky, promoted and unpublished?

I like this idea. I went with one published, but not promoted,
one promoted but not published and one promoted sticky but not published. This seems to really cover the cases.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented 
+++ b/core/modules/node/lib/Drupal/node/Tests/Views/FrontpageTest.phpundefined
@@ -0,0 +1,129 @@
+    }
+
+
+    // Create some nodes which aren't on the frontpage, either because they
...
+    $this->nodeStorageController->create($values)->save();
+
+
+
+
+    $column_map = array('nid' => 'nid');

Some crazy extra whitespace here.

Looks great though.

Edit: we should probably save the names of the four "do not want" nodes to an array, and assert that none of them are listed?

olli’s picture

olli CreditAttribution: olli commented

Should we also check that after creating some content the title is gone? (Or actually.. why are we using no results title and not the normal title on front page?)

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

(Or actually.. why are we using no results title and not the normal title on front page?)

This is feature parity with D7. When you install Drupal and have no content, it tells you what to do. As soon as you create content, there's no more need for the "welcome" title. At that point the D7 behavior is to simply have a list of teasers. Now the user can add an additional title if he or she wants (now that #1956912: Title area handler sets the title even when it should not (results in "Welcome to Drupal" never going away) is fixed), but by default there is no title.

olli’s picture

olli CreditAttribution: olli commented

I see, thanks. I was thinking the other way around: Now in D8 the user can easily remove the default title if he or she wants =)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
interdiff.txt3.32 KB
drupal-1963268-19.patch6.27 KB

Fixed that.

olli’s picture

olli CreditAttribution: olli commented
Status: Needs review » Reviewed & tested by the community

Looks good if not perfect to me.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

Looks good if not perfect to me.

That's not a confidence-inspiring RTBC for maintainers; what are your concerns?

olli’s picture

olli CreditAttribution: olli commented

Oh, sorry. I was trying to say that this looks really great to me: fixes the original problem and another bug and adds test coverage even beyond that.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

I, for one, am happy to be welcomed to "OHAI I HAZ A SITE NAME." :)

Committed and pushed to 8.x. Thanks!

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