Skip to content

Fix: Page Based Statistics - Pages still visible after converting to Draft status - #4602

Merged
dasgoutam merged 4 commits into
developfrom
fix/statistics-reflect-pages-draft-status-4139
Nov 10, 2026
Merged

dasgoutam merged 4 commits into
developfrom
fix/statistics-reflect-pages-draft-status-4139

Conversation

@dasgoutam

Copy link
Copy Markdown
Contributor

Short description

When page/s are saved as draft (bulk action or in the editor), some of them still show up in the Page Based Statistics until the Redis cache is flushed.

Proposed changes

  • When Redis is enabled, use cacheops invalidated_update() instead of update(), which clears the cached queries. Without Redis the update() stays. This follows the existing pattern in other parts of the code.
  • Applied in change_publication_status (bulk action "Save as draft") and in PageFormView

Side effects

  • Hopefully None

Faithfulness to issue description and design

There are no intended deviations from the issue and design.

How to test

Follow steps in Issue Description

  • Make sure that there are 1+ published pages
  • Go to 'Statistics' verify the pages are visible in Page Based Statistics. Go to 'Pages' and select all pages
  • Select bulk action 'Save as draft' and execute bulk action
  • Go to 'Statistics'. The Pages should no longer be visible after choosing a language
  • Check other conditions by adding and removing Draft status from a single page, while keeping the remaining pages published

Resolved issues

Fixes: #4139


Pull Request Review Guidelines

@MizukiTemma MizukiTemma left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 😸

💡 We can add a release note 📜

@dasgoutam
dasgoutam force-pushed the fix/statistics-reflect-pages-draft-status-4139 branch from 7dac970 to a9b0493 Compare November 10, 2026 09:31
@deliverino

deliverino Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

LLM PR Review (verdigado-code)

Analyzed diff, but not the whole repository. Important context might be missing.

  • The PR body says the change is used “when Redis is enabled”, but the diff gates on hasattr(..., "invalidated_update") in integreat_cms/cms/views/utils/publication_status.py:60 and integreat_cms/cms/views/pages/page_form_view.py:547. That checks whether cacheops has monkey-patched QuerySet, not whether Redis is configured or active. Either the description or the condition should be aligned.

  • The PageFormView change only modifies set_dependent_translations_to_draft(). If the editor’s primary page/translation draft-save path performs another queryset update() outside this diff, that path is not covered by the change.

  • The fix only invalidates cacheops-managed queryset caches. If the page-based statistics endpoint uses a separately cached aggregate or cache key outside cacheops, this may not clear it; that cannot be verified from the diff alone.

  • invalidated_update is not a standard Django QuerySet method. The diff shows no type stub, protocol, or # type: ignore[...]; confirm that strict mypy accepts these calls.

  • Using hasattr to detect cacheops is a runtime implementation detail. If cacheops is enabled with a non-Redis backend, the code will still call invalidated_update, contrary to the statement that “Without Redis the update() stays”.

  • No tests are included. The stale-statistics behavior should be covered by a test that saves pages as draft and asserts they no longer appear in page-based statistics, or at least by a test for the cacheops-aware fallback behavior.

  • No new views, permission checks, API response fields, model/migration changes, or new gettext strings are visible in the diff.

  • Release note integreat_cms/release_notes/current/unreleased/4139.yml is present with en and de. Nitpick: “page based” could be “page-based” if that is the term used in the UI.

  • Commit messages add release note and change release note message are vague. The first commit says “when redis is available”, but the current code checks for cacheops instead.

@dasgoutam
dasgoutam merged commit 874312e into develop Nov 10, 2026
5 checks passed
@dasgoutam
dasgoutam deleted the fix/statistics-reflect-pages-draft-status-4139 branch November 10, 2026 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Page Based Statistics: Some pages still visible if there are ONLY draft pages.

4 participants