News feeds as page controller

[bezin]

This implements news feeds as page controller and superseds #4459 eventually. Also I decided to split these into a news-bundle PR and an Event-bundle PR, because I find it easier to handle.

Still a couple things to do:

• Add feeds to layout (DCA is updated, but the correct routes need to be added to the page. Requires ResponseContext)
• Check and improve extensibility (e.g. provide better ways to extend RSS feeds core-bundle#1243, overwrite image size of every image element)
• Get rid of some legacy code. if possible
• Update permisssion handling (see below)
• Page Icon: fix offset in page tree and add required variants (not visible, not editable, ...)
• Update News Feed Insert tag
• Remove obsolete code
• Write migration (migrate feeds to pages; update permissions, if necessary)
• Add tests

There are also some permissions for news feeds, i.e. which news feed a editor has access to and if he/she is allowed to create or delete them). Since this feed is now a page controller, access in general is handled by the page type limitation. The question is, if we want to keep this distinction? I for myself are fine with the general limitation (can access news feed page types yes/no).

New Features

• JSON Feeds
• Only list featured/unfeatured news articles in the feed (equivalent to the newslist module)

[aschempp]

Looks very promising! Added feedback and minor nitpicks, but overall its 🔥

[aschempp]

I also noticed FeedIO currently requires Guzzle (see alexdebril/feed-io#398) 🙈

[bezin]

There are also some permissions for news feeds, i.e. which news feed a editor has access to and if he/she is allowed to create or delete them). Since this feed is now a page controller, access in general is handled by the page type limitation. The question is, if we want to keep this distinction?

What is your take on this? IMO we should drop these granular permissions and handle feeds like every other page, too.

[aschempp]

There are also some permissions for news feeds, i.e. which news feed a editor has access to and if he/she is allowed to create or delete them). Since this feed is now a page controller, access in general is handled by the page type limitation. The question is, if we want to keep this distinction?

What is your take on this? IMO we should drop these granular permissions and handle feeds like every other page, too.

I think so too. Did we consider how this can be migrated on an update? Because I guess we cannot migrate the permissions?

[bezin]

I think so too. Did we consider how this can be migrated on an update? Because I guess we cannot migrate the permissions?

As there is no direct equivalent to each option after this gets merged I think we cannot migrate these. The baseline would be, that if an editor has the permission to at least one news feed, we should allow access to the new News Feed page type in general. However, this would mean the editor may now have permission to more feeds than previously.

[bezin]

As you see I have added a TransformFeedItem to make the transformation from a news model to feed item exensible. However, I am not exactly sure about this and have some more cases, where I fail to see how to implement this the right way:

• I use feeds (custom implementation) with CleverReach to import Contao News into their editor. They do not wait long enough when requesting images for it work with our deferred image resizing. Hence I need a way to tell Contao to resize images right away in this case.

• For the same use case, I want to be able to overwrite image sizes in Image content elements to resize them to a size that is suitable for mailings.

From contao/core-bundle#1243:

• Add a namespace to the Feed object, e.g. for Facebook Instant Articles. The library we use supports this, but it must be extensible by a third party.

Any comment much appreciated. Maybe the image resizing must be solved completely different anyways.

[bezin]

I experimented with a provider service rather than an event (see bezin@19aad3b)

I implemented a FeedProvider that a) creates the feed instance, b) fetches the news articles depending on page settings and c) transforms articles to items. This way, you may decorate the provider and alter the feed instance and/or every item.

Add additional namespaces to the feed instance:

// src/App/NewsFeedProvider

class NewsFeedProvider implements FeedProviderInterface
{
  // ...
  public function getFeed(): FeedInterface
  {
    $feed = $this->inner->getFeed();
    $feed->addNS('content', 'http://purl.org/rss/1.0/modules/content/');
    return $feed;
  }
  // ...
}

Add a custom format options to tl_page.feedSource and alter item transformation all together in your decorator:

// src/App/NewsFeedProvider

class NewsFeedProvider implements FeedProviderInterface
{
  // ...
  public function getItemFromModel(NewsModel $model): ItemInterface
  {
    if ('my_custom_source_option' !== $this->pageModel->feedSource) {
      return $this->inner->getItemFromModel($model);
    }
    
    // Handle item creation in your own way
  }
  // ...
}

What are your thoughts on this?

[fritzmg]

Apparently you are not supposed to extend from the original service when decorating 😬 🤷

I'd prefer an event still I think, since that is what we use for /sitemap.xml and /robots.txt as well.

[m-vo]

Apparently you are not supposed to extend from the original service when decorating

Yes, that's a bad idea in general, because you cannot guarantee that the original class will be in the correct state if you only pass on some calls. Implementing the interface, injecting the original service and using its public API is usually safer to do. 🙂

But tbh this is also true for every extended base class - so it primarily depends on if a class is designed for this use case or not (for non abstract classes: mostly not).

[bezin]

Apparently you are not supposed to extend from the original service when decorating 😬 🤷

Yes, you are right. This is actually wrong in the example – it is not required.

[bezin]

Couldn’t you create those images on the fly when running the tests? like we do here:

Did so in 931f26b Thanks for the hint!

[leofeyer]

Thank you @bezin.