Skip to content

fix: Sitemap: Return a QuerySet in CMSSitemap.items() #8031

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

Merged
merged 4 commits into from
Oct 16, 2024

Conversation

MacLake
Copy link
Contributor

@MacLake MacLake commented Oct 14, 2024

Description

According to the Django docs also a sequence like a list is allowed as the return value type of CMSSitemap.items(),
but djangocms_page_sitemap applies exclude, which only works with QuerySet.

This PR changes the type of the return value back to QuerySet.

Related resources

Checklist

  • I have opened this pull request against develop-4
  • I have added or modified the tests when changing logic
  • I have followed the conventional commits guidelines to add meaningful information into the changelog
  • I have read the contribution guidelines and I have joined the channel #pr-reviews on our Discord Server to find a “pr review buddy” who is going to review my pull request.

@MacLake MacLake changed the title Fix (Sitemap): Return a QuerySet in CMSSitemap.items() Fix: Sitemap: Return a QuerySet in CMSSitemap.items() Oct 14, 2024
According to the Django docs a sequence like a list is allowed
for the return value of CMSSitemap.items() but djangocms-page-sitemap
applies exclude, which only works with querysets.

In django CMS 3.11 it was a QuerySet. In django CMS 4 the return value
changed to a list, though, which breaks djangocms-page-sitemap.
So the next commit becomes more readable.
@MacLake MacLake changed the title Fix: Sitemap: Return a QuerySet in CMSSitemap.items() fix: Sitemap: Return a QuerySet in CMSSitemap.items() Oct 14, 2024
@fsbraun
Copy link
Member

fsbraun commented Oct 14, 2024

Fixes #8029

@fsbraun fsbraun added the needs to be backported Commits need to be backported label Oct 16, 2024
@fsbraun fsbraun merged commit accc8da into django-cms:develop-4 Oct 16, 2024
45 of 51 checks passed
fsbraun added a commit that referenced this pull request Oct 16, 2024
* fix: Sitemap: Don’t convert QuerySet to list in CMSSitemap.items()

According to the Django docs a sequence like a list is allowed
for the return value of CMSSitemap.items() but djangocms-page-sitemap
applies exclude, which only works with querysets.

In django CMS 3.11 it was a QuerySet. In django CMS 4 the return value
changed to a list, though, which breaks djangocms-page-sitemap.

* style: Format cms/tests/test_sitemap.py with ruff

So the next commit becomes more readable.

* test: Add test for type of CMSSitemap.items()

---------

Co-authored-by: Fabian Braun <fsbraun@gmx.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs to be backported Commits need to be backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants