fix uniweb-issue-1353: attempt to remove missing item from list

[jrief]

Description

Fix #430: sometimes fields are missing in list

Related resources

• Internal server error when clicking on "Settings"-button. #430

Checklist

• I have opened this pull request against master
• 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 #workgroup-pr-review on
  Slack to find a “pr review buddy” who is going to review my pull request.

Summary by Sourcery

Bug Fixes:

• Fix an issue where sometimes "slug" and "overwrite_url" fields were missing in the list of read-only fields, resulting in an error.

[sourcery-ai]

Reviewer's Guide by Sourcery

This pull request fixes issue #430 by changing the way readonly fields are determined. Instead of always attempting to remove "slug" and "overwrite_url" from the list of fields, the code now checks if these fields exist in the list before attempting to remove them. This prevents an error when trying to remove a non-existent item.

Sequence diagram for readonly fields determination

sequenceDiagram
    participant Admin as Admin System
    participant Fields as Fields Handler

Admin->>Fields: get_readonly_fields(request, obj)
Note over Fields: Check if form has fieldsets
alt Has fieldsets
    Fields->>Fields: flatten_fieldsets()
end
Fields->>Fields: Convert to list
Fields->>Fields: Find intersection with {slug, overwrite_url}
Fields->>Fields: Remove matching fields
Fields-->>Admin: Return filtered fields

Loading

State diagram for field removal logic change

stateDiagram-v2
    [*] --> CheckFields
    CheckFields --> IntersectFields: Fields exist
    IntersectFields --> RemoveFields: Matching fields found
    RemoveFields --> [*]: Fields removed
    IntersectFields --> [*]: No matching fields

    note right of IntersectFields
        New: Only attempt removal
        of existing fields using
        set intersection
    end note

Loading

File-Level Changes

Change	Details	Files
Handle cases where "slug" or "overwrite_url" are missing from the list of fields.	Changed the way readonly fields are determined to avoid potential errors. Use set intersection to remove fields only if they exist in the list of fields	djangocms_versioning/cms_config.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#430	Fix the internal server error when clicking on 'Settings'-button in page version management	✅
#430	Prevent ValueError when attempting to remove a field that doesn't exist in the list	✅

Possibly linked issues

• Internal server error when clicking on "Settings"-button. #430: The PR fixes the bug described in the issue.

---

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.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a 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. You can also use
  this command to specify where the summary should be inserted.

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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey @jrief - I've reviewed your changes - here's some feedback:

Overall Comments:

• Please add tests to verify this fix - specifically a test case that would have caught the original KeyError and verifies the new behavior works correctly.

Here's what I looked at during the review

• 🟢 General issues: all looks good
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 91.21%. Comparing base (c9bf9f4) to head (e4c1f1f).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
djangocms_versioning/cms_config.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #439   +/-   ##
=======================================
  Coverage   91.21%   91.21%           
=======================================
  Files          72       72           
  Lines        2663     2663           
  Branches      307      307           
=======================================
  Hits         2429     2429           
  Misses        163      163           
  Partials       71       71           

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

[fsbraun]

@jrief Thanks for the fix!!