Show subscription status behind newsletter in subscriber overview

1. +++ b/src/Plugin/Field/FieldFormatter/SimplenewsSubscriberStatusFormatter.php
   @@ -0,0 +1,56 @@
   + * @file
   + * Contains \Drupal\simplenews\Plugin\Field\FieldFormatter\CollectUriTailFormatter.
   + */
   

   This hasn't been updated.

2. +++ b/src/Plugin/Field/FieldFormatter/SimplenewsSubscriberStatusFormatter.php
   @@ -0,0 +1,56 @@
   + * @FieldFormatter(
   + *   id = "simplenews_status",
   

   simplenes_subscription_status is a better name for this.

3. +++ b/src/Plugin/Field/FieldFormatter/SimplenewsSubscriberStatusFormatter.php
   @@ -0,0 +1,56 @@
   + *   label = @Translation("Subscriber Status Formatter"),
   

   This should also mention subscription.

4. +++ b/src/Plugin/Field/FieldFormatter/SimplenewsSubscriberStatusFormatter.php
   @@ -0,0 +1,56 @@
   +      $elements[]['status'] = array(
   +        '#markup' => 'Unsubscribed',
   ...
   +      foreach ($items as $delta => $item) {
   +        if ($item->status == SIMPLENEWS_SUBSCRIPTION_STATUS_SUBSCRIBED) {
   +          $output = 'Subscribed';
   +        }
   +        if ($item->status == SIMPLENEWS_SUBSCRIPTION_STATUS_UNCONFIRMED) {
   +          $output = 'Unconfirmed';
   +        }
   +        if ($item->status == SIMPLENEWS_SUBSCRIPTION_STATUS_UNSUBSCRIBED) {
   +          $output = 'Unsubscribed';
   +        }
   +        $elements[$delta]['status'] = array(
   +          '#markup' => $output,
   +        );
   

   Visible strings must always use t() or $this->t() if available so that they can be translated.

   Also not sure this here makes sense. I'd just leave it empty in this case. You don't really need the if/else then, you can always loop over items.

5. +++ b/src/Plugin/Field/FieldFormatter/SimplenewsSubscriberStatusFormatter.php
   @@ -0,0 +1,56 @@
   +        if ($item->status == SIMPLENEWS_SUBSCRIPTION_STATUS_SUBSCRIBED) {
   +          $output = 'Subscribed';
   +        }
   +        if ($item->status == SIMPLENEWS_SUBSCRIPTION_STATUS_UNCONFIRMED) {
   +          $output = 'Unconfirmed';
   +        }
   +        if ($item->status == SIMPLENEWS_SUBSCRIPTION_STATUS_UNSUBSCRIBED) {
   +          $output = 'Unsubscribed';
   +        }
   +        $elements[$delta]['status'] = array(
   +          '#markup' => $output,
   

   This still needs to show the actual subscription label like the parent implementation does.

+++ b/src/Plugin/Field/FieldFormatter/SimplenewsSubscriptionStatusFormatter.php
@@ -0,0 +1,57 @@
+      // Add the label.
+      if ($output) {
+        $elements[$delta]['status'] = [
+          '#markup' => $output,
+        ];
+      }

Hm. While this is nice to make it show up after it... there is a problem with it.

This isn't going to work for left-to right languages, as the output won't be in the right order for them.

I think we need to change this to use t('@label (Unsubscribed)') and same for the other variation, so it can be translated properly. And then build the full output ourself and don't rely on the parent.

No, you must have two separate t() calls for the two cases with additional label to have it properly translatable. for the confirmed case, you can just use the entity label as is.

Hm.

Is it really helpful to have the newsletter there linked? it doesn't seem like a common operation to got to the newsletter form from there, and having all those links on a page with lots of newsletters seems quite distracting.

Also, when you edit the view and disable the link option then this doesn't work anymore because the parent structure doesn't match anymore.

Let's change this to extend from EntityReferenceFormatterBase, no longer call the parent at all and just set #markup => $output.

1. +++ b/src/Plugin/Field/FieldFormatter/SimplenewsSubscriptionStatusFormatter.php
   @@ -0,0 +1,56 @@
   +/**
   + * Implementation of the 'status' formatter for Simplenews Subscription.
   + *
   

   Formatter that displays a newsletter subscription with the status.

2. +++ b/src/Plugin/Field/FieldFormatter/SimplenewsSubscriptionStatusFormatter.php
   @@ -0,0 +1,56 @@
   +      // Add label for subscribed.
   +      $output = $this->t('@label', array('@label' => $label));
   

   You can just directly assign $label.

   And the comment should be something like "// Do not explicitly display the status for confirmed subscriptions."

3. +++ b/src/Plugin/Field/FieldFormatter/SimplenewsSubscriptionStatusFormatter.php
   @@ -0,0 +1,56 @@
   +      // Add the label.
   +      $elements[$delta]['#title'] = $output;
   

   This will then become #markup.

We have a pretty good demo module now to test this with some data. We should make sure to have at least one unconfirmed and unsubscribed subscription set up in simplenews_demo_install().

+++ b/modules/simplenews_demo/simplenews_demo.install
@@ -85,6 +85,8 @@ function simplenews_demo_install() {
   $subscription_manager->unsubscribe('c@example.com', $newsletters['press_releases']->id(), FALSE);
+  // Unconfirmed subscriber.
+  $subscription_manager->subscribe('d@example.com', $newsletters['special_offers']->id(), SIMPLENEWS_SUBSCRIPTION_STATUS_UNCONFIRMED, FALSE);

That's how this API works. There are arguments that except the constant, it just expects NULL, TRUE or FALSE. And what you want is TRUE.

Test fails can be ignored, that's just missing dependencies. You might want to run the demo test locally though to make sure that is passing.

+++ b/modules/simplenews_demo/simplenews_demo.install
@@ -85,6 +85,8 @@ function simplenews_demo_install() {
   $subscription_manager->unsubscribe('c@example.com', $newsletters['press_releases']->id(), FALSE);
+  // Unconfirmed subscriber.
+  $subscription_manager->subscribe('d@example.com', $newsletters['special_offers']->id(), SIMPLENEWS_SUBSCRIPTION_STATUS_UNCONFIRMED, TRUE);

I'm sorry but this is still not correct.

There is no argument that would accept that constant. It works, but that's only because the value is cast to a boolean.

Now it looks fine, committed.