🚜 Improve the news links in the header menu and on the homepage - #3092
🚜 Improve the news links in the header menu and on the homepage#3092jefftriplett wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aligns the homepage “Latest News” More link destination with the site’s aggregated news feed by pointing it to the local /blogs/ page, and increases the number of entries shown on that /blogs/ page.
Changes:
- Update the homepage “More” link to use the internal
blogURL instead ofBLOG_URL. - Increase
/blogs/page entry list size from 5 to 10 (by fetchingENTRY_LIST_LIMIT + 1to support a featured header entry).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| templates/components/blog-posts.html | Updates “More” to reverse the internal blog URL. |
| apps/blogs/views.py | Adds ENTRY_LIST_LIMIT = 10 and expands the BlogHome queryset slice accordingly. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (3)
apps/blogs/views.py:21
- There is an extra space after the slice colon (
[: ENTRY_LIST_LIMIT + 1]). This is inconsistent with typical formatting and may trigger linting (e.g., flake8 E203) depending on project config.
entries = BlogEntry.objects.order_by("-pub_date")[: ENTRY_LIST_LIMIT + 1]
apps/blogs/views.py:22
- The view behavior change increases the number of blog entries surfaced on the blog homepage (from 6 total to
ENTRY_LIST_LIMIT + 1). The existing view test asserts the latest entry but doesn’t assert how many entries are exposed inresp.context["entries"], so this change isn’t guarded against regressions.
def get_context_data(self, **kwargs):
"""Return the latest blog entries for the blog homepage."""
context = super().get_context_data(**kwargs)
entries = BlogEntry.objects.order_by("-pub_date")[: ENTRY_LIST_LIMIT + 1]
latest_entry = None
templates/components/blog-posts.html:8
- The homepage "Latest News" component is still fetching only 5 entries (
limit=5), but the PR description says this section should show 10. This currently doesn’t implement the intended behavior.
{% get_latest_blog_entries limit=5 as entries %}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
fixtures/boxes.json:467
- This fixture still links to Python Insider over plain HTTP (
http://blog.python.org). Since this PR is explicitly moving external links to HTTPS, this should also be updated to avoid mixed/insecure links.
"content": "<h2 class=\"widget-title\">Copyright</h2>\r\n<p>Python Insider by the Python core team is licensed under a <a href=\"https://creativecommons.org/licenses/by-nc-sa/3.0/us/\">Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported License</a>. Based on a work at <a href=\"http://blog.python.org\">blog.python.org</a>.</p>",
"content_markup_type": "html",
"_content_rendered": "<h2 class=\"widget-title\">Copyright</h2>\r\n<p>Python Insider by the Python core team is licensed under a <a href=\"https://creativecommons.org/licenses/by-nc-sa/3.0/us/\">Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported License</a>. Based on a work at <a href=\"http://blog.python.org\">blog.python.org</a>.</p>"
0b37d16 to
8b28a1f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
fixtures/boxes.json:467
- The updated blogs copyright box still links to
http://blog.python.org, even though the project’s configured blog URL is HTTPS and this PR is moving external links to HTTPS. Update the link tohttps://blog.python.orgin bothcontentand_content_rendered.
"content": "<h2 class=\"widget-title\">Copyright</h2>\r\n<p>Python Insider by the Python core team is licensed under a <a href=\"https://creativecommons.org/licenses/by-nc-sa/3.0/us/\">Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported License</a>. Based on a work at <a href=\"http://blog.python.org\">blog.python.org</a>.</p>",
"content_markup_type": "html",
"_content_rendered": "<h2 class=\"widget-title\">Copyright</h2>\r\n<p>Python Insider by the Python core team is licensed under a <a href=\"https://creativecommons.org/licenses/by-nc-sa/3.0/us/\">Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported License</a>. Based on a work at <a href=\"http://blog.python.org\">blog.python.org</a>.</p>"
apps/blogs/views.py:9
ENTRY_LIST_LIMIT = 10currently results in only 9 items in the “Latest News” list on /blogs/ because the newest entry is moved into the page header (other_entries = entries[1:]). If the intent is to show 10 list items (as before: query size = list size + 1), fetch one extra entry and slice using a dedicated query-limit constant.
# Number of entries the page shows. The newest one goes in the page
# header, the rest go in the "Latest News" list.
ENTRY_LIST_LIMIT = 10
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
templates/components/blog-posts.html:8
- PR description says the homepage widget should show 10 news entries instead of 5, but this template tag still limits the list to 5.
{% get_latest_blog_entries limit=5 as entries %}
apps/blogs/views.py:21
ENTRY_LIST_LIMIT = 10currently results in 9 items in the "Latest News" list because the template treats the first entry as the separate header item (other_entries = entries[1:]). If the intent is 10 list items (i.e., matching the prior behavior where[:6]produced 5 list items), the query needs to fetch one extra entry; otherwise, consider updating the constant/docstring/PR description to make it explicit that 10 is the total (1 header + 9 list).
entries = BlogEntry.objects.order_by("-pub_date")[:ENTRY_LIST_LIMIT]
5a7ce27 to
4a79a1e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
apps/blogs/views.py:9
ENTRY_LIST_LIMITis described/used as the total number of entries shown on the page (header entry + list items), but the name reads like it only limits the list portion. Renaming to something likeENTRY_PAGE_LIMIT/BLOG_HOME_ENTRY_COUNTwould make the intent clearer and avoid off-by-one confusion when usingENTRY_LIST_LIMIT - 1for the list size in tests.
# Number of entries the page shows. The newest one goes in the page
# header, the rest go in the "Latest News" list.
ENTRY_LIST_LIMIT = 10
hugovk
left a comment
There was a problem hiding this comment.
Thanks, nice improvement.
- Clarifies the Python News menu wording so it is clearer that it points to the aggregated
/blogs/page rather than directly to Python Insider.
Where's this one? I didn't spot it.
There was a problem hiding this comment.
🟡 Changes recommended
Existing deployed menu and box data are not updated by fixtures, and the test does not enforce the required nine list entries.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
apps/blogs/tests/test_views.py:64
- This assertion derives the expected count from
ENTRY_LIST_LIMIT, so it would still pass if the limit were accidentally reverted to 6 (it would then expect 5 list entries). The PR's required total of 10 entries is therefore not protected; assert the contract directly with 9 list entries (or separately assert that the limit is 10).
for index in range(ENTRY_LIST_LIMIT + 5):
fixtures/boxes.json:465
- This box is also seed data rather than an update to the existing row:
BoxFactoryusesdjango_get_or_createkeyed bylabel, so changingboxes.jsonwill not modify an existingblogs-copyrightbox. The deployed/blogs/page can therefore continue showing the old wording and HTTP attribution link unless this is applied with a data migration or deployment update.
"content": "<h2 class=\"widget-title\">Copyright</h2>\r\n<p>Python Insider by the Python core team is licensed under a <a href=\"https://creativecommons.org/licenses/by-nc-sa/3.0/us/\">Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported License</a>. Based on a work at <a href=\"https://blog.python.org\">blog.python.org</a>.</p>",
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
Two moderate issues remain, along with a test coverage nit.
Review details
Suppressed comments (3)
apps/blogs/templates/blogs/index.html:45
- This page now advertises PSF, PyCon, and Planet entries, but the existing hard-coded “Welcome to the Python Insider” banner still labels whichever
latest_entryis newest as Python Insider. When a non-Insider feed supplies the newest post, its title is shown under the wrong source branding; use generic aggregate branding or derive the source fromlatest_entry.feed.
<h2 class="widget-title">News Sources</h2>
<ul class="menu">
<li><a href="https://blog.python.org/">Python Insider</a></li>
apps/blogs/tests/test_views.py:76
- Because both the number of records created and this assertion are derived from
ENTRY_LIST_LIMIT, the test would still pass if the implementation regressed to the old limit of 6 (it would create 11 records and assert 5 list entries). The PR's key contract is exactly 10 total entries—one header plus 9 list entries—so assert the expected list count independently of the production constant.
self.assertEqual(len(resp.context["entries"]), ENTRY_LIST_LIMIT - 1)
fixtures/boxes.json:465
- This file is only seed input to
apps/boxes/factories.initial_data, andBoxFactoryusesdjango_get_or_create = ("label",), so an existingblogs-copyrightrow is returned without applying this new content. On deployed databases the/blogs/page will therefore keep rendering the old HTTP link/text unless the row is updated separately; add an explicit data update/migration or document and apply the production update.
"content": "<h2 class=\"widget-title\">Copyright</h2>\r\n<p>Python Insider by the Python core team is licensed under a <a href=\"https://creativecommons.org/licenses/by-nc-sa/3.0/us/\">Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported License</a>. Based on a work at <a href=\"https://blog.python.org\">blog.python.org</a>.</p>",
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
Two moderate issues remain regarding deployment data updates and test coverage of the concrete entry limit.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
fixtures/boxes.json:467
- Updating this fixture does not update an existing
blogs-copyrightbox in deployed databases:initial_data()callsBoxFactory, whosedjango_get_or_create = ("label",)keeps the existing row and ignores the new content. As a result, the HTTPS URL and wording change will only affect fresh/empty data unless this is accompanied by an explicit data migration or production update.
apps/blogs/tests/test_views.py:76
- This test derives the expected count from
ENTRY_LIST_LIMIT, so it does not lock in the requested 15-entry behavior: reverting both the constant and view to the old limit of 6 would still make it pass. Assert the concrete 14 list entries (or separately assert that the configured limit is 15) to catch that regression.
self.assertEqual(len(resp.context["entries"]), ENTRY_LIST_LIMIT - 1)
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Following up on the latest Copilot review notes:
|
There was a problem hiding this comment.
🔵 Needs a closer look
Address the responsive widget placement and ensure the existing copyright box is updated during deployment.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
fixtures/boxes.json:467
- This fixture change will not update the existing
blogs-copyrightrow:apps/boxes/factories.py:21,27-41creates boxes withget_or_create(label=...)and ignores changed fields for an existing label, while the page renders that stored row via{% box 'blogs-copyright' %}. As a result, normal deployments will continue showing the old “Core Developers” wording and HTTP URL unless this box is updated separately (for example with a data migration or an explicit deployment update).
apps/blogs/templates/blogs/index.html:50
- Between the 50em and 75em breakpoints,
.small-widgetis 48.9% wide and.lastonly removes its right margin; the right-float rule is added only at 75em. Because this is now the third child after the 74.5%-wide posts block, the source widget floats beside the wrapped subscriptions widget while the copyright widget moves to another row, so it is not consistently below subscriptions in the right column. Add breakpoint-specific clearing/grouping or adjust the widget layout to preserve that placement.
<div class="small-widget news-sources-widget last">
<h2 class="widget-title">News Sources</h2>
<ul class="menu">
<li><a href="https://blog.python.org/">Python Insider</a></li>
<li><a href="https://pyfound.blogspot.com/">PSF Blog</a></li>
<li><a href="https://pycon.blogspot.com/">PyCon Blog</a></li>
<li><a href="https://planetpython.org/">Planet Python</a></li>
</ul>
</div>
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
Two moderate issues remain unresolved: responsive widget stacking and updating existing box data.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
fixtures/boxes.json:465
- This
boxes.jsonedit only changes seed data:BoxFactoryusesdjango_get_or_create = ("label",)andinitial_data()passes the existing row through without updating its content (apps/boxes/factories.py:17-40). Therefore an existing deployment will keep the oldblogs-copyrighttext andhttp://blog.python.orglink, so this HTTPS/wording change will not appear there. Please update the existingBoxin a data migration or deployment step as well.
apps/blogs/templates/blogs/index.html:42
- At the 800–940px breakpoint,
.small-widgetis 48.9% wide and.lastonly removes the right margin, so this new widget can sit besideblogs-subscriptionsinstead of below it; on wider screens the following copyright widget can also float beside it. Add a clear/stacking rule or wrapper so the right-column widgets remain vertical at these responsive widths.
<div class="small-widget news-sources-widget last">
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
Sorry, that was a little confusing. I made some changes in the fixtures then realized some of it I could already fix (like https) in the live website. |
Add a Python Insider link to the News menu, point the homepage More link at /blogs/ instead of the external blog, and show 10 entries there instead of 6. Also switch the external nav links to https and clarify the Python News wording, since that item goes to /blogs/ and not to Python Insider.
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
8b46495 to
9f15f2f
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The deployed blogs-copyright Box still retains the old copyright text and HTTP link because fixture changes do not update existing rows.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
| "content": "<h2 class=\"widget-title\">Copyright</h2>\r\n<p>Python Insider by the Python core team is licensed under a <a href=\"https://creativecommons.org/licenses/by-nc-sa/3.0/us/\">Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported License</a>. Based on a work at <a href=\"https://blog.python.org\">blog.python.org</a>.</p>", | ||
| "content_markup_type": "html", | ||
| "_content_rendered": "<h2 class=\"widget-title\">Copyright</h2>\r\n<p>Python Insider by the Python Core Developers is licensed under a <a href=\"https://creativecommons.org/licenses/by-nc-sa/3.0/us/\">Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported License</a>. Based on a work at <a href=\"http://blog.python.org\">blog.python.org</a>.</p>" | ||
| "_content_rendered": "<h2 class=\"widget-title\">Copyright</h2>\r\n<p>Python Insider by the Python core team is licensed under a <a href=\"https://creativecommons.org/licenses/by-nc-sa/3.0/us/\">Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported License</a>. Based on a work at <a href=\"https://blog.python.org\">blog.python.org</a>.</p>" |

The Latest News section on python.org aggregates posts from several different sources, but its “More” link currently sends people to the newer
blog.python.orgsite. That is confusing because not all of the news shown on python.org appears there. In particular, PSF announcements, election information, and other posts published through the PSF blog can be easy to miss unless someone already knows to visitpyfound.blogspot.com.This PR makes the News section and menu a little clearer:
/blogs/, the existing python.org page that aggregates these sources./blogs/from 6 entries to 15. The newest entry is used as the page header, with the other 14 shown in the list./blogs/list to match the homepage Latest News widget: the date sits in its own column next to the title, instead of leaving a large empty gap with the title and date pushed into a second column./blogs/itself. That page already aggregates every source, so there was no single “more” to point at. A News Sources widget in the right column, below Python Insider Subscriptions, now links to each site instead: Python Insider, the PSF Blog, the PyCon Blog, and Planet Python./blogs/page rather than directly to Python Insider.brochure.getpython.infowas returning an error, and the item is already hidden in production.The homepage Latest News widget itself still shows 5 entries; only its link destination changes.
Note: the fixtures are seed data. The live menu is stored in the database; those edits have already been made in production.