Skip to content

fix: views.details revealed existence of unpublished language #7853

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Mar 25, 2024
Merged

fix: views.details revealed existence of unpublished language #7853

merged 4 commits into from
Mar 25, 2024

Conversation

fsbraun
Copy link
Member

@fsbraun fsbraun commented Mar 22, 2024 •
edited
Loading

Description

page.get_languages() contains a list of languages, for which translations exist. It does not imply those languages should be visible to the public. djangocms-versioning, e.g., hides languages upon unpublishing.

This PR removes the call to page.get_languages() from django CMS' central view in views.details. To avoid additional db accesses, the (public) page content objects are fetched into the page's content cache and the available languages are determined through the cache.

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 #workgroup-pr-review on Slack to find a “pr review buddy” who is going to review my pull request.

@jrief
Copy link
Contributor

jrief commented Mar 22, 2024

If I now try to open the English version of a page, I get this exception:

 File "django-cms/cms/toolbar/toolbar.py", line 510, in _call_toolbar
    getattr(toolbar, func_name)()
  File "djangocms-versioning/djangocms_versioning/cms_toolbars.py", line 340, in populate
    self.page_content = self.get_page_content() if self.page else None
  File "djangocms-versioning/djangocms_versioning/cms_toolbars.py", line 336, in get_page_content
    return get_latest_admin_viewable_content(self.page, language=language, include_unpublished_archived=True)
  File "djangocms-versioning/djangocms_versioning/helpers.py", line 329, in get_latest_admin_viewable_content
    return qs.filter(**extra_grouping_fields).latest_content().first()
AttributeError: 'QuerySet' object has no attribute 'latest_content'

btw.:

qs.filter(**extra_grouping_fields).first()

delivers an object, so presumably the wrong manager is used.

@fsbraun
Copy link
Member Author

fsbraun commented Mar 22, 2024 •
edited
Loading

@jrief Django's prefetch cache ignores the queryset which versioning swaps out. That leads to the error you mentioned.

I've now exchanged the prefetch by a separate call to fill the page content cache. Turns out this is neutral on db hits since it was filled anyways. Can you give it another test?

@fsbraun fsbraun requested review from jrief and a team March 22, 2024 18:12
@jrief
Copy link
Contributor

jrief commented Mar 25, 2024

@fsbraun Thanks for the quick fix. This now works as expected.

@fsbraun fsbraun merged commit fa7b89c into django-cms:develop-4 Mar 25, 2024
@fsbraun fsbraun deleted the fix/spurious-languages branch April 20, 2024 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrief jrief jrief approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fsbraun @jrief