fix: filter expired and future posts from public views - #85

Reviewer's Guide

Ensures public post listings only show currently published posts by filtering on publication date range, and stabilizes test data by using deterministic publication dates and adding coverage for the new filtering behavior.

Sequence diagram for public post listing with publication date filtering

sequenceDiagram
  actor PublicUser
  participant Browser
  participant PostListView
  participant PostContent_manager
  participant Database

  PublicUser->>Browser: Request_stories_listing
  Browser->>PostListView: HTTP_GET /stories/
  PostListView->>PostListView: get_queryset()
  PostListView->>PostContent_manager: model.objects.all()
  PostContent_manager->>Database: SELECT * FROM postcontent
  Database-->>PostContent_manager: All_PostContent_rows
  PostListView->>PostContent_manager: filter(date_published_lte_now, date_published_end_null_or_gt_now)
  PostContent_manager->>Database: SELECT ... WHERE post.date_published <= now AND (post.date_published_end IS NULL OR post.date_published_end > now)
  Database-->>PostContent_manager: Currently_published_PostContent
  PostListView->>PostContent_manager: filter(language, namespace)
  PostContent_manager->>Database: SELECT ... WHERE language = request_language AND app_config.namespace = view_namespace
  Database-->>PostContent_manager: Localized_namespace_posts
  PostListView-->>Browser: Rendered_list_template
  Browser-->>PublicUser: Page_with_only_currently_published_posts

Updated class diagram for post content listing and publication dates

classDiagram
  class Post {
    DateTimeField date_published
    DateTimeField date_published_end
  }

  class PostAppConfig {
    CharField namespace
  }

  class PostContent {
    CharField language
    ForeignKey post
    ForeignKey app_config
  }

  class PostContentManager {
    QuerySet all()
  }

  class PostContentAdminManager {
    QuerySet latest_content()
  }

  class PostListView {
    get_queryset()
  }

  PostContent "*" --> "1" Post
  Post "1" --> "1" PostAppConfig
  PostContent "*" --> "1" PostAppConfig

  PostContent --> PostContentManager : objects
  PostContent --> PostContentAdminManager : admin_manager

  PostListView --> PostContent : model

  PostListView ..> Post : filters_by_date_published
  PostListView ..> Post : filters_by_date_published_end
  PostListView ..> PostContent : filters_by_language
  PostListView ..> PostAppConfig : filters_by_namespace

File-Level Changes

Possibly linked issues

• Public views do not filter posts by date_published_end (Expired posts are visible) #67: PR updates BaseConfigListViewMixin queryset to respect date_published/date_published_end and adds tests, fixing the issue.

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.41%. Comparing base (f628aa0) to head (6046e20).

@@            Coverage Diff             @@
##             main      #85      +/-   ##
==========================================
- Coverage   90.94%   89.41%   -1.53%     
==========================================
  Files          24       24              
  Lines        2309     2325      +16     
  Branches      276      281       +5     
==========================================
- Hits         2100     2079      -21     
- Misses        126      160      +34     
- Partials       83       86       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@dnlzrgz Nice PR! Thank you!

I appreciate the use of django ORM features to fix the visibility and sorting issues.

There's a package called djangocms-timed-publishing which is an extension to djangocms-verisoning and adds precisely the feature of making published content only visible after and/or before a certain date.

We might need to think on how to align those features. Would you mind having a look at djangocms-timed-publishing ?

Thanks for the comment @fsbraun!

I took a look at the djangocms-timed-publishing and you're right. It will definitely conflict with my current solution and make the UI "weirder" than it should.

I feel like djangocms-stories should have a way to handle this without the need of another dependency. But I can also see how having a plugin providing this functionality can make sense.

My proposal would be then to ensure that djangocms-stories works as intended with its own dates so it isn't "broken" by default and things work as the user expects and then try to update the get_queryset to check if the plugin is installed and try to avoid the conflict there.

Let me know if this approach sounds good to you!

OK, let's avoid the "being broken by default". For now, can we check for "djangocms_timed_publishing" being in INSTALLED_APPS. If so, omit the publication dates from the forms - so the user does not have two options?

To not make things messier these are the changes I will make if everything looks good to you:

1. In the BaseConfigListViewMixin I will update the get_queryset as follows:

#...
        if hasattr(self.request, "toolbar") and (
            self.request.toolbar.edit_mode_active or self.request.toolbar.preview_mode_active
        ):
            queryset = self.model.admin_manager.latest_content()
        else:
            queryset = self.model.objects.all()
            if not apps.is_installed("djangocms_timed_publishing"):
                queryset = queryset.filter(
                    Q(post__date_published__lte=current_time) | Q(post__date_published__isnull=True),
                    Q(post__date_published_end__isnull=True) | Q(post__date_published_end__gt=current_time),
                )
#...

2. Update the get_fieldsets of the PostAdmin class in a similar way.

This PR feels a bit "spaghetti" to me but I haven't been able to come up with a better solution.

There is a failling test in test_admin.py (I don't know if it was me who broke something but it shouldn't be the case) and I also still need to update the documentation to point to the potential conflict between plugins.

I got a bit lost in some parts but I think everything should be good now.

Edit: I just merged the 'main' branch so maybe the failing test I mentioned is fixed now.

@dnlzrgz Can you take another look at the failing tests? It seems to be influenced by the django CMS version.

Hey @fsbraun, thanks for the patience. I have been trying to understand what is failing and why but so far I haven't been able to replicate the CI results on my machine (even after purging the environment, setting a new one, and doing a fresh install). All tests pass on my end.

I checked if maybe djangocms_timed_publishing was accidentally being installed, if the test util publish_if_necessary was modifying things in a way that it shouldn't, reviewing the factories, re-reviewing the queryset filtering logic, checking that the django CMS version was correct and so on...

Any guidance on what to look at next would be greatly appreciated!