Add a ContextProvider for Feeds

I'm repurposing this issue. I believe that 📌 Convert the feed block into a view Postponed is the way forward for this module's blocks and I want to eliminate the block plugin. So I don't want to use some of the work that was done here. But the great thing is that we'll still need the ContextProvider that was written for Views blocks! So I rerolled that part of the patch and added a test.

I thought about going ahead and changing the block plugin. But I don't want to worry about upgrading blocks given that they could be placed with Layout Builder or some other means. And I don't want to worry about backward compatibility for the proposed change either. Let's leave the plugin-generated blocks alone for now and only worry about BC when we add the view.

I added the test trait with the eventual goal of moving more test helper functions into it. The functional tests have a lot of that in the base class, but there's nothing for kernel tests. I would have already started work on it, but the functional base class is heavily dependent on the frontpage view as a convenient RSS feed to test against. I haven't figured out a plan for replacing that yet.

I think we should postpone this on the views block
The concern I have here is we're adding every feed as a context which on a site with a lot of feeds is a major performance issue.

If we contrast this with how node works, there's a 'current node' context only, i.e. the current node being viewed.

I'm not convinced using context in this way is the correct approach here.

Consider e.g. Drupal.org's planet - it would have 100s of feeds loaded on every page load.

@larowlan My expectation is that getAvailableContexts() which is the function that loads all feeds would only be called at configuration time. Then getRuntimeContexts() only loads the configured feeds. Am I mistaken about my assumptions and getAvailableContexts() gets called before getRuntimeContexts()?

The intended use of this would be for the sake of replacing the existing Aggregator blocks which display single feeds. We need a way to people to be able to select which feed they want to display when they configure the block. For something like Drupal Planet or any situation where you're displaying Items from multiple feed sources, then using context wouldn't even be the most efficient way to build a block. A person would want to build a custom view and use standard Views filters on the fid value.

Am I wrong about this?

I think that is how it is supposed to work, but I have a vague recollection of it not working that way in practice.

At one point there were some places in core that called the wrong method. If that's since fixed then we might be ok.

+++ b/src/ContextProvider/AggregatorFeedContext.php
@@ -0,0 +1,59 @@
+    $this->feedStorage = $entity_type_manager->getStorage('aggregator_feed');

We shouldn't do this, accessing the storage handler in a constructor can cause data loss if done too early.

We should inject the entity type manager as is, and get the storage handler in the actual methods.

#3099968: Layout builder incorrectly resolves global contexts values when viewing layouts → might be what I was thinking of

Thanks for letting me know. I'm going to do some digging on my own to try and find out what happened.

\Drupal\layout_builder\Context\LayoutBuilderContextTrait::getPopulatedContexts is one point where all available contexts are requested at run time.

This called when showing a list of blocks to display to a content editor in layout builder.

@larowlan What do you think about doing a simple database query for all the Feed uuids and titles in getAvailableContexts() instead of loading the entities?

@dcam we shouldn't use DB queries for entities, we can use entity query though

\Drupal::entityTypeManager()->getStorage('aggregator_feed')->getAggregateQuery()->groupBy('uuid')->groupBy('title')->execute();

Should be able to use array_combine and array_column on that and get what you need

> we shouldn't use DB queries for entities

Yeah, I know. I think I once heard Crell say that if you're writing a DB query in D8+ then you're probably doing something wrong. And true to that I could probably count on one or two hands the number of queries I've written since 2015 aside from those in migration source plugins.

I knew nothing about aggregate entity queries though, let alone that you can use them to return entity values. Thank you for showing me that. This updated context class still passes the test and works to allow me to select the feed when placing a Views block.

We should inject the entity type manager as is, and get the storage handler in the actual methods.

Sorry, I forgot to do that. Here's a new patch.

I moved the latest patch into GitLab since Drupal CI testing has been shut off.