Skip to content

fix: show_placeholder did not respect edit/preview mode and failed loudly #8274

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 1 commit into from
Jun 30, 2025
Merged

fix: show_placeholder did not respect edit/preview mode and failed loudly #8274

merged 1 commit into from
Jun 30, 2025

Conversation

fsbraun
Copy link
Member

@fsbraun fsbraun commented Jun 30, 2025 •
edited by sourcery-ai bot
Loading

Description

Backport of 8272

Related resources

Checklist

  • I have opened this pull request against main
  • 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.

Summary by Sourcery

Backport the fix for show_placeholder to respect edit and preview modes and gracefully handle missing page content, update related model and templatetag logic, add corresponding tests, and clean up documentation and admin utilities.

Bug Fixes:

  • Make _show_placeholder_by_id respect toolbar edit/preview mode and return empty string instead of failing in production

Enhancements:

  • Extend Page.get_placeholders to accept an admin_manager flag for retrieving draft content when editing or previewing
  • Catch PageContent.DoesNotExist alongside Placeholder.DoesNotExist in _show_placeholder_by_id and handle accordingly

Documentation:

  • Bump pip-compile Python version, update django-cms and urllib3 versions in docs requirements
  • Remove djangocms_admin_style from example django_settings

Tests:

  • Add test to verify show_placeholder handles nonexistent page content in both DEBUG and non-DEBUG settings

Chores:

  • Remove unused content change-permission disabling logic from admin utils

@fsbraun
fix: show_placeholder did not respect edit/preview mode and failed lo…
3cd8c38 
…udly (#8272)

* fix: show_placeholder tag

* Add test

* Use current_content

* fix: test names switched

* Better naming inside test

* fix: docs

* Update docs/requirements.txt

* Fix django_settings for docs
@sourcery-ai Sourcery AI
Copy link
Contributor

sourcery-ai bot commented Jun 30, 2025 •
edited
Loading

Reviewer's Guide

Backported improved placeholder rendering to respect toolbar edit/preview state by introducing an admin_manager flag in get_placeholders and _show_placeholder_by_id, added error handling for missing PageContent, cleaned up dead admin utils code, and updated documentation dependency pins.

Sequence diagram for placeholder rendering with toolbar edit/preview mode

sequenceDiagram
    participant User as actor User
    participant Request as Request
    participant Toolbar as Toolbar
    participant Page as Page
    participant PageContent as PageContent
    participant Placeholder as Placeholder

    User->>Request: Request page with placeholder
    Request->>Toolbar: get_toolbar_from_request(request)
    Toolbar-->>Request: toolbar (with edit/preview state)
    Request->>Page: get_placeholders(lang, admin_manager)
    alt admin_manager True (edit/preview mode)
        Page->>PageContent: admin_manager.current_content().get(language, page)
    else admin_manager False (live mode)
        Page->>PageContent: objects.get(language, page)
    end
    Page->>Placeholder: get_for_obj(page_content)
    Placeholder-->>Request: placeholder
    Request-->>User: Rendered placeholder or error
Loading

Class diagram for updated Page.get_placeholders method

classDiagram
    class Page {
        +get_placeholders(language: str, admin_manager: bool = False) QuerySet
    }
    class PageContent {
        +admin_manager
        +objects
    }
    class Placeholder {
        +get_for_obj(obj)
    }
    Page --> PageContent : uses
    Page --> Placeholder : uses
    PageContent <|-- admin_manager
    PageContent <|-- objects
Loading

File-Level Changes

Change Details Files
Placeholder rendering now respects edit/preview modes and handles missing content gracefully
  • Hook into toolbar to set admin_manager in _show_placeholder_by_id
  • Extend get_placeholders to accept admin_manager and use admin_manager.current_content() when true
  • Catch PageContent.DoesNotExist with PlaceholderModel.DoesNotExist and return empty string in production
  • Add tests for DEBUG vs non-DEBUG missing content behavior
 cms/templatetags/cms_tags.py
cms/models/pagemodel.py
cms/tests/test_templatetags.py
Remove unreachable content permission checks in admin utils
  • Drop dead code block enforcing can_change_content on additional fields
 cms/admin/utils.py
Update documentation dependency pins and settings
  • Bump Python version and update django-cms and urllib3 pins
  • Sync requirements.in updates
  • Remove djangocms_admin_style from example settings
 docs/requirements.txt
docs/requirements.in
docs/django_settings.py

Possibly linked issues

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@github-actions GitHub Actions
Copy link

👋 Hi there!

Please remember to MERGE COMMIT pull requests from main!

Do not SQUASH commits to preserve history for the changelog.

@fsbraun fsbraun merged commit e7be7bc into release/5.0.x Jun 30, 2025
69 checks passed
@fsbraun fsbraun deleted the chore/backport-8272-2 branch June 30, 2025 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@fsbraun