Skip to content

Only return published blog entries. #1757

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 10 commits into from
Aug 3, 2025
Merged

Only return published blog entries. #1757

merged 10 commits into from
Aug 3, 2025

Conversation

@marksweb
Copy link
Contributor

@marksweb marksweb commented Nov 20, 2024

This fixes #1755 to only return published blog entries, even for staff users.

Unpublished entries can still be previewed via the admin.

@marksweb
Copy link
Contributor Author

marksweb commented Nov 20, 2024 •
edited
Loading

On a side note, my test suite is failing me and I'm not sure why.

======================================================================
ERROR: blog.tests (unittest.loader._FailedTest.blog.tests)
----------------------------------------------------------------------
ImportError: Failed to import test module: blog.tests
Traceback (most recent call last):
  File "/Users/mark/.local/share/uv/python/cpython-3.12.7-macos-aarch64-none/lib/python3.12/unittest/loader.py", line 396, in _find_test_path
    module = self._get_module_from_name(name)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/mark/.local/share/uv/python/cpython-3.12.7-macos-aarch64-none/lib/python3.12/unittest/loader.py", line 339, in _get_module_from_name
    __import__(name)
  File "/Users/mark/dev/djangoproject.com/blog/tests.py", line 2, in <module>
    from test.support import captured_stderr
ModuleNotFoundError: No module named 'test'

Discovered the reason for this; #1758

@marksweb marksweb requested a review from a team November 20, 2024 23:02
@marksweb marksweb mentioned this pull request Nov 20, 2024
Closed
jefftriplett
jefftriplett reviewed Nov 20, 2024
View reviewed changes
pauloxnet
pauloxnet reviewed Nov 21, 2024
View reviewed changes
bmispelon
bmispelon requested changes Nov 21, 2024
View reviewed changes
Copy link
Member

@bmispelon bmispelon left a comment

Choose a reason for hiding this comment

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

Thanks for the quick response on this, nice work 👍🏻

If you allow me to change/extend the scope of this PR a little bit, I think it's not enough to test for user.is_staff but we should also make sure that the user has write permissions on the BlogEntry model (since this feature is meant to be used for previewing).

And there should also be some tests added (I'm suprised there weren't already, or that if there were they were still passing after this change):

  • anon users can't see unpublished entries at all (list or detail)
  • logged in users (non-staff) can't see unpublished entries at all
  • staff users without write permission on BlogEntry can't see unpublished entries at all
  • staff users with write permission on BlogEntry can't see unpublished entries in the list, but can view the detail page

@jefftriplett
Copy link
Member

jefftriplett commented Nov 21, 2024

@marksweb this is great. Thank you for jumping in on this.

@marksweb
Copy link
Contributor Author

marksweb commented Nov 21, 2024

And there should also be some tests added (I'm suprised there weren't already

Yes I was surprised & expecting you to suggest this 😄

@marksweb marksweb self-assigned this Nov 21, 2024
@marksweb
Copy link
Contributor Author

marksweb commented Apr 18, 2025

Thanks for the quick response on this, nice work 👍🏻

If you allow me to change/extend the scope of this PR a little bit, I think it's not enough to test for user.is_staff but we should also make sure that the user has write permissions on the BlogEntry model (since this feature is meant to be used for previewing).

And there should also be some tests added (I'm suprised there weren't already, or that if there were they were still passing after this change):

* anon users can't see unpublished entries at all (list or detail)

* logged in users (non-staff) can't see unpublished entries at all

* staff users without write permission on `BlogEntry` can't see unpublished entries at all

* staff users with write permission on `BlogEntry` can't see unpublished entries in the list, but can view the detail page

Ok, it took longer than I expected, but I've added tests and enforced permissions.

adamzap
adamzap suggested changes May 20, 2025
View reviewed changes
adamzap
adamzap approved these changes May 21, 2025
View reviewed changes
Copy link
Member

@adamzap adamzap left a comment

Choose a reason for hiding this comment

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

👍 Looks great!

I guess the last thing to do here is squash commits and rebase the branch, but maybe the merger will do that?

pauloxnet
pauloxnet reviewed May 21, 2025
View reviewed changes
Copy link
Member

@pauloxnet pauloxnet left a comment

Choose a reason for hiding this comment

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

I added 2 minor suggestions.

@adamzap
Copy link
Member

adamzap commented Jul 14, 2025

@marksweb Could you check the CI failure here?

marksweb added 10 commits July 15, 2025 02:52
@marksweb
Only return published blog entries.
9c9efb0
@marksweb
Allow staff to view unpublished entries
91dd36e 
diff --git blog/views.py blog/views.py
index 4629754..49c1fdc 100644
--- blog/views.py
+++ blog/views.py
@@ -21,7 +21,6 @@ def get_queryset(self):
         return Entry.objects.published()

     def get_context_data(self, **kwargs):
-        """ """
         context = super().get_context_data(**kwargs)

         events_queryset = Event.objects.future().published()
@@ -50,3 +49,10 @@ class BlogDayArchiveView(BlogViewMixin, DayArchiveView):

 class BlogDateDetailView(BlogViewMixin, DateDetailView):
     banner_is_title = False
+
+    def get_queryset(self):
+        """Allows staff users to view unpublished entries"""
+        if self.request.user.is_staff:
+            return Entry.objects.all()
+        else:
+            return Entry.objects.published()
@marksweb
WIP testing
f9f937c
@marksweb
Fixed staff blog test
d034ab1
@marksweb
Added test for anonymous users
823dbac
@marksweb
Added test for regular users
b1ea0f9
@marksweb
enforce permissions for blog preview.
75aa79a
@marksweb
Addressed latest review comments.
19cd313
@marksweb
Addressed latest review comments.
7620c65
@marksweb
fix: Recursion issue
c6e2e58
@marksweb
Copy link
Contributor Author

marksweb commented Jul 15, 2025

@marksweb Could you check the CI failure here?

Done - there was an actual issue I'd not spotted 🤦🏼

@adamzap
Copy link
Member

adamzap commented Jul 16, 2025

@bmispelon Do you have any concerns here, or can we merge this?

bmispelon
bmispelon approved these changes Jul 30, 2025
View reviewed changes
Copy link
Member

@bmispelon bmispelon left a comment

Choose a reason for hiding this comment

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

Nice work Mark, thanks for this! 👍🏻

@sabderemane sabderemane merged commit b2a5462 into django:main Aug 3, 2025
4 checks passed
@marksweb marksweb deleted the fix/1755 branch October 16, 2025 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bmispelon bmispelon bmispelon approved these changes

+3 more reviewers

@jefftriplett jefftriplett jefftriplett left review comments

@pauloxnet pauloxnet pauloxnet left review comments

@adamzap adamzap adamzap approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@marksweb marksweb

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unpublished Blog Entries show up for staff in list pages

6 participants

@marksweb @jefftriplett @adamzap @bmispelon @pauloxnet @sabderemane