Clean up Help & Statistics blocks

Wait, don't we still have the problem of empty blocks not showing to be empty?

1. +++ b/core/modules/help/src/Plugin/Block/HelpBlock.php
   @@ -93,19 +86,6 @@ public static function create(ContainerInterface $container, array $configuratio
   -  protected function blockAccess(AccountInterface $account) {
   -    $this->help = $this->getActiveHelp($this->request);
   -    if ($this->help) {
   -      return AccessResult::allowed();
   -    }
   -    else {
   -      return AccessResult::forbidden();
   -    }
   -  }
   

   Do we perhaps need a comment on the method in the interface that this method should NOT be used to vary visibility based on stateful information, eg, data that would be shared with build()? Seems like a wise thing to do, since we want to discourage module authors from doing... what we're making this block not do anymore. :-)

2. +++ b/core/modules/statistics/src/Plugin/Block/StatisticsPopularBlock.php
   @@ -123,18 +86,25 @@ public function blockSubmit($form, FormStateInterface $form_state) {
   -    if ($this->day_list) {
   -      $content['top_day'] = $this->day_list;
   -      $content['top_day']['#suffix'] = '<br />';
   +    if ($this->configuration['top_day_num'] > 0) {
   +      $result = statistics_title_list('daycount', $this->configuration['top_day_num']);
   +      if ($result) {
   +        $content['top_day'] = node_title_list($result, $this->t("Today's:"));
   +        $content['top_day']['#suffix'] = '<br />';
   +      }
        }
   

   These changes are going to conflict with #1446932: Improve statistics performance by adding a swappable backend, which is largely rewriting this method.

As much as I hate conflicts I agree the statistics block should be an easy-to-resolve conflict, and as much as I would love #1446932: Improve statistics performance by adding a swappable backend into 8.0.x I'm not sure I can argue the case, so guess I need to accept conflicts.

Sorry Wim, but I disagree. Whether or not we punt on documenting something seems to vary widely, depending primarily on whether or not jhodgdon is in the issue thread. :-) I'm going to channel her in this case and say that if this issue is fixing "these blocks are doing something unnecessarily stupid", then it should also be responsible for the documentation to ensure no one else does anything unnecessarily stupid.

@wim - My patch pretty much just rewrites statistics_title_list function to work with the service also added in the patch, so should be pretty straightforward to merge.

If this was just the statistics patch I would RTBC, but I guess the block docs needs a fight.

I talked with Wim in IRC earlier today about this issue, and offered to just write the requisite docs myself. However, after staring at it for a while I realized I didn't know how to do it justice without referencing StatefulBlockPluginInterface as the correct way to handle edge cases... which of course doesn't exist yet and won't until #2543340: Convert BlockViewBuilder to use #lazy_builder (but don't yet let context-aware blocks be placeholdered). Sooo... I'm going to RTBC this and instead be annoying about documentation in that issue instead, as then we can do it properly (and it's partially done already by StatefulBlockPluginInterface itself).

+1 as subsystem maintainer.

I cannot possibly fathom how this is a quick fix. :D

Needs a reroll.

Simple reroll.
Conflicted with the cache context change of #2543332: Auto-placeholdering for #lazy_builder without bubbling

Committing this as it improves cacheability. Committed d904faa and pushed to 8.0.x. Thanks!

Thanks you for adding the beta evaluation to the issue summary.