-
Notifications
You must be signed in to change notification settings - Fork 4
Improve the blog feed #426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,5 @@ | ||||||||||||
| from datetime import datetime, time | ||||||||||||
| from typing import TYPE_CHECKING, Any, Optional | ||||||||||||
|
|
||||||||||||
| from django.contrib.syndication.views import Feed | ||||||||||||
|
|
||||||||||||
|
|
@@ -7,41 +8,74 @@ | |||||||||||
| from .models import BlogPage | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| if TYPE_CHECKING: | ||||||||||||
| from django.http import HttpRequest, HttpResponse | ||||||||||||
|
|
||||||||||||
| from wagtail.query import PageQuerySet | ||||||||||||
|
|
||||||||||||
| # Main blog feed | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| class BlogFeed(Feed): | ||||||||||||
| title = "The Torchbox Blog" | ||||||||||||
| link = "/blog/" | ||||||||||||
| link = "/news/" | ||||||||||||
| description = "The latest news and views from Torchbox on the work we do, the web and the wider world" | ||||||||||||
|
|
||||||||||||
| def items(self): | ||||||||||||
| return BlogPage.objects.live().public().order_by("-date")[:10] | ||||||||||||
|
|
||||||||||||
| def item_title(self, item): | ||||||||||||
| request: Optional["HttpRequest"] = None | ||||||||||||
|
|
||||||||||||
| def __call__( | ||||||||||||
| self, request: "HttpRequest", *args: Any, **kwargs: Any | ||||||||||||
| ) -> "HttpResponse": | ||||||||||||
| self.request = request | ||||||||||||
|
|
||||||||||||
| return super().__call__(request, *args, **kwargs) | ||||||||||||
|
|
||||||||||||
| def items(self) -> "PageQuerySet[BlogPage]": | ||||||||||||
| return ( | ||||||||||||
| BlogPage.objects.live() | ||||||||||||
| .public() | ||||||||||||
| .defer_streamfields() | ||||||||||||
| .select_related("feed_image") | ||||||||||||
| .prefetch_related("authors__author") | ||||||||||||
| .order_by("-date")[:10] | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| def item_title(self, item: BlogPage) -> str: | ||||||||||||
| return item.title | ||||||||||||
|
|
||||||||||||
| def item_description(self, item): | ||||||||||||
| return item.listing_summary | ||||||||||||
| def item_description(self, item: BlogPage) -> str: | ||||||||||||
| return item.listing_summary or item.search_description | ||||||||||||
|
|
||||||||||||
| def item_link(self, item): | ||||||||||||
| return item.get_full_url() | ||||||||||||
| def item_link(self, item: BlogPage) -> str: | ||||||||||||
| return item.get_full_url(request=self.request) | ||||||||||||
|
|
||||||||||||
| def item_author_name(self, item): | ||||||||||||
| pass | ||||||||||||
| def item_author_name(self, item: BlogPage) -> str | None: | ||||||||||||
| return ( | ||||||||||||
| ", ".join([author_item.author.name for author_item in item.authors.all()]) | ||||||||||||
| or None | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| def item_pubdate(self, item): | ||||||||||||
| def item_pubdate(self, item: BlogPage) -> datetime: | ||||||||||||
| return datetime.combine(item.date, time()) | ||||||||||||
|
|
||||||||||||
| def item_enclosure_url(self, item): | ||||||||||||
| def item_enclosure_url(self, item: BlogPage) -> str | None: | ||||||||||||
| if item.feed_image: | ||||||||||||
| return item.feed_image.file.url | ||||||||||||
|
|
||||||||||||
| def item_enclosure_mime_type(self, item): | ||||||||||||
| def item_enclosure_mime_type(self, item: BlogPage) -> str | None: | ||||||||||||
| if item.feed_image: | ||||||||||||
| image_format = filetype.guess_extension(item.feed_image.file) | ||||||||||||
| return f"image/{image_format}" | ||||||||||||
| try: | ||||||||||||
| if image_format := filetype.guess_extension(item.feed_image.file): | ||||||||||||
| return f"image/{image_format}" | ||||||||||||
| except (AttributeError, OSError, TypeError): | ||||||||||||
| pass | ||||||||||||
|
|
||||||||||||
| return None | ||||||||||||
|
|
||||||||||||
| def item_enclosure_length(self, item): | ||||||||||||
| def item_enclosure_length(self, item: BlogPage) -> str | None: | ||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another code review issue that claude raised - will leave you to judge if it is valid (I don't know if it's correct about what file.size returns but seems plausible that it is an int): Issue: The return type is str | None, but file.size returns an int. This creates a type mismatch.
Suggested change
Impact:
|
||||||||||||
| if item.feed_image: | ||||||||||||
| return item.feed_image.file.size | ||||||||||||
| try: | ||||||||||||
| return str(item.feed_image.file.size) | ||||||||||||
| except (AttributeError, OSError, TypeError): | ||||||||||||
| pass | ||||||||||||
|
|
||||||||||||
| return None | ||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain this change a bit more? I get the exact same result for the urls before and after the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.wagtail.org/en/stable/advanced_topics/performance.html#page-urls