Title variable isn't set by Syndicate block - so the link text is an unfinished sentence

Can I take a stab at this issue?

Not sure if this is overkill but I used dependency injection to get the site name from the config factory service. I could have also just used the Drupal object like so \Drupal::config('system.site')->get('name').

Working on this.

Here I have added reroll of patch #6.

Here I have addressed comment #7.

Still needs work for tests.

Addressed test case.

Reviewed and looks good to me.

Patch looks good for me, I think we can have a patch for 8.9.x as well.

Patch #12 cleanly apply on drupal version 8.8.

@andrewmacpherson There is no difference with patch #12, But I applied same patch on version 8.8 and its working.

+++ b/core/modules/node/tests/src/Functional/NodeSyndicateBlockTest.php
@@ -37,6 +37,11 @@ public function testSyndicateBlock() {
+    $syndicate_block_title = 'Subscribe to ' . $site_name;
+    $this->assertSession()->pageTextContains($syndicate_block_title);

Thanks for the patch!

This should be

$site_name = $this->config('system.site')->get('name');

We might be OK hardcoding this tho, and just doing

$this->assertSession()->pageTextContains('Subscribe to Drupal');

Rerolled patch for 9.3.x and updated tests as per #21.

Thanks.

Reviewed the code and it looks 👍 from my point of view. There is a nitpicky missing dot at the end of the inline comment in the test, but I'll hope committers will be willing to fix that on commit :)

Ops, I take it back.

1. +++ b/core/modules/node/tests/src/Functional/NodeSyndicateBlockTest.php
   @@ -37,6 +37,10 @@ public function testSyndicateBlock() {
   +    // Verify syndicate block title
   

   Missing dot at the end of line.

2. +++ b/core/modules/node/tests/src/Functional/NodeSyndicateBlockTest.php
   @@ -37,6 +37,10 @@ public function testSyndicateBlock() {
   +    $site_name = $this->config('system.site')->get('name');
   

   Now that we are hardcoding the expected string, this line is no longer needed.

Let me work on this.

Patch from #23 does not apply for me on 9.3.x

Rerolled patch #23 and updated tests as per #25.

👍 Looks good!

Committed/pushed to 9.3.x, thanks!

Since this includes a constructor change, while it's allowable in 9.2.x, think we can probably wait for 9.3 with this one.