Skip to content

Fix #1275: is_reviewed flag inconsistencies and media access control #1280

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

robsiera
Copy link

Fixes #1275

⚠️ Disclaimer

Note: I am not a Python programmer and this fix was developed with heavy assistance from AI tools. While the solution has been carefully analyzed, it has not been tested! So I recommend thorough code review by experienced Python/Django developers before merging.

Problem

The is_reviewed flag exhibited inconsistent behavior:

  1. Public edit page incorrectly displayed is_reviewed as true when database value was false
  2. Media with is_reviewed = false remained publicly accessible despite review settings

Root Cause Analysis

  1. Form issue: is_reviewed field was completely removed from forms for non-editors using self.fields.pop("is_reviewed"), causing frontend display inconsistencies
  2. Database sync issue: When updating is_reviewed via API, the listable field wasn't included in update_fields, so the recalculated value wasn't persisted to the database

Solution

  1. files/forms.py: Make is_reviewed field readonly/disabled for non-editors instead of removing it entirely
  2. files/views.py: Include listable in update_fields when updating is_reviewed to ensure the recalculated value is saved

Technical Details

  • The listable field is recalculated in the model's save() method based on state, encoding_status, and is_reviewed
  • Using update_fields=["is_reviewed"] only persists the is_reviewed change, ignoring the recalculated listable value
  • This created a desync where is_reviewed=False but listable=True, allowing unintended public access

Testing

  • Edit page now shows correct review status for all user types
  • Non-reviewed media are properly filtered from public access
  • Review status changes via API now properly update access control
  • Existing functionality for editors/managers remains unchanged

Request for Review

Given my limited Python/Django experience, I especially welcome feedback on:

  • Django best practices for form field handling
  • Proper use of update_fields parameter
  • Any potential edge cases or security implications

Robrecht Siera added 2 commits May 28, 2025 23:09
- Make is_reviewed field readonly/disabled for non-editors instead of removing it
- This preserves the field value and allows frontend to display review status correctly
- Fixes inconsistent display between public edit page and Django admin

Fixes mediacms-io#1275
- Include 'listable' in update_fields when saving is_reviewed changes via API
- Ensures listable status is recalculated and persisted to database
- Prevents unreviewed media from remaining publicly accessible

Fixes mediacms-io#1275
@mgogoulos
Copy link
Contributor

hi, can you sync with main branch? there have been a few changes. I think the change in files/forms.py is not needed anymore, so probably keep the change in views.py only.
Thanks

# Conflicts:
#	files/forms.py
@robsiera
Copy link
Author

beter like this?

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.

The 'is_reviewed' flag for media items exhibits inconsistent behavior across different UI views
2 participants