Skip to content

Fix#6855 browsable api extra action#9934

Merged
auvipy merged 6 commits into
encode:mainfrom
dlfoerster:fix/6855-browsable-api-extra-action
May 13, 2026
Merged

Fix#6855 browsable api extra action#9934
auvipy merged 6 commits into
encode:mainfrom
dlfoerster:fix/6855-browsable-api-extra-action

Conversation

@dlfoerster
Copy link
Copy Markdown
Contributor

@dlfoerster dlfoerster commented Mar 31, 2026

refs #6855

Summary

  • Avoid object-level permission checks when rendering synthetic OPTIONS form metadata in the browsable API.
  • Keep DELETE/edit permission behavior unchanged.
  • Add regression coverage for extra actions returning serializer-backed data with incompatible serializer instances, plus a delete-permission guardrail test.

Test plan

  • .venv/bin/pytest tests/test_renderers.py::BrowsableAPIRendererTests

@auvipy auvipy requested review from auvipy and Copilot April 1, 2026 09:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a browsable API crash for ViewSet extra actions by preventing object-level permission checks when generating the synthetic OPTIONS form metadata (issue #6855).

Changes:

  • Special-case OPTIONS handling in BrowsableAPIRenderer.get_rendered_html_form() to avoid object-level permission checks against serializer.instance.
  • Add regression coverage reproducing the crash scenario for extra actions returning serializer.data.
  • Add a guardrail test ensuring DELETE form rendering still respects object-level permission checks.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
rest_framework/renderers.py Adjusts browsable API form generation to skip object-permission checks for synthetic OPTIONS placeholders.
tests/test_renderers.py Adds regression tests for issue #6855 and verifies DELETE still enforces object permissions.
Comments suppressed due to low confidence (1)

rest_framework/renderers.py:515

  • The new OPTIONS special-casing duplicates show_form_for_method() logic (allowed_methods + permission check) and makes the later if method in ('DELETE', 'OPTIONS') branch effectively unreachable for OPTIONS. Consider reusing show_form_for_method(view, method, request, obj=None) here (to skip object permissions) and then simplifying the later condition to only handle DELETE, to keep the control flow in one place and avoid future divergence.
            if method == 'OPTIONS':
                # The browsable API only needs a placeholder for OPTIONS, so
                # avoid object-level permission checks against serializer.instance.
                if method not in view.allowed_methods:
                    return
                try:
                    view.check_permissions(request)
                except exceptions.APIException:
                    return
                return True

            if not self.show_form_for_method(view, method, request, instance):
                return

            if method in ('DELETE', 'OPTIONS'):
                return True  # Don't actually need to return a form


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

@PsymoNiko PsymoNiko left a comment

Choose a reason for hiding this comment

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

Thanks

@auvipy auvipy requested a review from browniebroke May 6, 2026 07:34
@auvipy auvipy changed the title Fix/6855 browsable api extra action Fix#6855 browsable api extra action May 13, 2026
@auvipy auvipy merged commit 3e40eb4 into encode:main May 13, 2026
7 checks passed
@auvipy auvipy added this to the 3.18 milestone May 13, 2026
@auvipy auvipy added the Bug label May 13, 2026
@browniebroke
Copy link
Copy Markdown
Member

Can we close the referenced issue or is there anything else to do?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants