Skip to content
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

fix: Refactor CMSMenu class for easier subclassing #7832

Open
wants to merge 6 commits into
base: develop-4
Choose a base branch
from

Conversation

fsbraun
Copy link
Sponsor Member

@fsbraun fsbraun commented Mar 6, 2024

Description

The CMSMenu class has been updated in #7807 for efficient and versioning-compatible menus.
This PR factors out two major changes to allow for future subclassing (such as asked for in django-cms/djangocms-versioning#387):

  • CMSMenu.get_pagecontent_queryset: returns the queryset to build the menu. The default implementation includes the admin-visible latest_content, e.g. not yet published pages, in edit and preview mode
  • CMSMenu.preview_endpoint: returns a callable to get an endpoint for the menu entry. The default implementation returns the preview endpoint in edit or preview mode.

The PR does not change the logic of how CMSMenu works.

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.

@fsbraun fsbraun requested review from jrief and a team March 6, 2024 09:39
@fsbraun fsbraun added the 4.1 label Mar 7, 2024
@fsbraun
Copy link
Sponsor Member Author

fsbraun commented Apr 7, 2024

@jrief Do you think this refactor will help you customize the menu system?

@jrief
Copy link
Contributor

jrief commented Apr 8, 2024

@jrief Do you think this refactor will help you customize the menu system?

The bigger problem I have is that the menu renderer can't be replaced by a customized implementation, but that's a different story. I will merge this PR into my private branch and test it. Make take some days, though.

@fsbraun
Copy link
Sponsor Member Author

fsbraun commented May 22, 2024

@jrief Is this still an issue? Or shall I close this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants