Fixed #34624 -- Removed change, delete, and view buttons for non-Select widgets in RelatedFieldWidgetWrapper. #18877

JaeHyuckSa · 2024-12-02T13:26:32Z

Trac ticket number

Branch description

I continued working on the PR for @allcaps.

Checklist

This PR targets the main branch.
The commit message is written in past tense, mentions the ticket number, and ends with a period.
I have checked the "Has patch" ticket flag in the Trac system.
I have added or updated relevant tests.
I have added or updated relevant docs, including release notes if applicable.
I have attached screenshots in both light and dark modes for any UI changes.

BSmick6 · 2024-12-02T21:05:17Z

@JaeHyuckSa it looks like you're running into the same issue I was. You should rebase this branch to the main upstream branch and force push again: https://docs.djangoproject.com/en/5.1/internals/contributing/writing-code/working-with-git/#after-upstream-has-changed

JaeHyuckSa · 2024-12-03T04:44:09Z

@JaeHyuckSa it looks like you're running into the same issue I was. You should rebase this branch to the main upstream branch and force push again: https://docs.djangoproject.com/en/5.1/internals/contributing/writing-code/working-with-git/#after-upstream-has-changed

@BSmick6 Thank you for letting me know, but it seems that a different issue is occurring than the one you mentioned.

BSmick6 · 2024-12-03T13:24:51Z

@JaeHyuckSa it looks like you're running into the same issue I was. You should rebase this branch to the main upstream branch and force push again: https://docs.djangoproject.com/en/5.1/internals/contributing/writing-code/working-with-git/#after-upstream-has-changed

@BSmick6 Thank you for letting me know, but it seems that a different issue is occurring than the one you mentioned.

Oh, interesting! I would love to find out why. I don't see why any of your changes would cause any of those errors, but I can try talking a look later this afternoon.

sarahboyce · 2024-12-03T16:28:19Z

Instead of testing the redirect admin, I would refine a new admin in tests/admin_widgets/widgetadmin.py that sets radio_fields similar to the redirect admin

JaeHyuckSa · 2025-08-22T17:22:30Z

@sarahboyce Thank you for the guidance. Sorry for the delayed response — I’ve updated the patch according to your feedback.

sarahboyce

Thank you! I have a few suggestions

Note there is a second part of the ticket which is basically should RedirectAdmin have radio_fields = {"site": admin.VERTICAL} (using the AdminRadioSelect widget) or should it use a select widget instead. Do you have thoughts on this?

sarahboyce · 2025-08-26T07:51:25Z

django/contrib/admin/widgets.py

+        # Widgets that display multiple values are not supported.
+        supported = not getattr(
+            widget, "allow_multiple_selected", False
+        ) and not isinstance(widget, AdminRadioSelect)
+        self.can_change_related = supported and can_change_related


I believe we should make this more generic

Suggested change

# Widgets that display multiple values are not supported.

supported = not getattr(

widget, "allow_multiple_selected", False

) and not isinstance(widget, AdminRadioSelect)

self.can_change_related = supported and can_change_related

# Only single-select Select widgets are supported.

supported = not getattr(

widget, "allow_multiple_selected", False

) and isinstance(widget, Select)

You would need to add the import from django.forms.widgets import Select

I agree with making this more generic. Regarding your review, was the removal of line 293 intentional?

No sorry that was a mistake in the comment 👍

sarahboyce · 2025-08-26T07:53:24Z

tests/admin_widgets/tests.py

@@ -2053,3 +2053,26 @@ def test_clearablefileinput_widget_preserve_clear_checkbox(self):
            self.selenium.find_element(By.ID, self.clear_checkbox_id).is_selected(),
            True,
        )
+
+
+class RadioFieldWidgetWrapperTests(AdminWidgetSeleniumTestCase):


Instead of adding a selenium test (and models/admin), I would add the following test to RelatedFieldWidgetWrapperTests

def test_non_select_widget_cant_change_delete_related(self): main_band = Event._meta.get_field("main_band") widget = widgets.AdminRadioSelect() wrapper = widgets.RelatedFieldWidgetWrapper( widget, main_band, widget_admin_site, can_add_related=True, can_change_related=True, can_delete_related=True, ) self.assertTrue(wrapper.can_add_related) self.assertFalse(wrapper.can_change_related) self.assertFalse(wrapper.can_delete_related)

Could you the criteria for when to write a Selenium test versus a regular test? I think this will be more helpful for decision-making in future work.

Some things need a selenium test (e.g. if it was testing a mixture of python and javascript together or we want to generate screenshots). If it is possible to test with a regular test, this is usually preferable (especially if there are some existing ones) as they tend to be quicker to run

Thank you for the guidance.

JaeHyuckSa · 2025-08-26T14:48:35Z

Thank you! I have a few suggestions

Note there is a second part of the ticket which is basically should RedirectAdmin have radio_fields = {"site": admin.VERTICAL} (using the AdminRadioSelect widget) or should it use a select widget instead. Do you have thoughts on this?

radio

select widget

@sarahboyce Thank you for the review, Sarah. I had overlooked that there were additional considerations here. My view is that switching to a select (ideally autocomplete_fields) aligns better with the admin’s default patterns and the current JS behavior, while radios don’t scale well when there are many Sites, so I think moving to a select would be preferable.

If the above is appropriate and changes are needed, would it be more suitable to continue in this PR, or should I open a separate PR for that work?

sarahboyce · 2025-08-27T07:03:18Z

Happy for it to be a select, a standard one is also fine imo.
I would have it as a seperate commit in this PR that is Refs #34624 --

…ct widgets in RelatedFieldWidgetWrapper. Signed-off-by: SaJH <wogur981208@gmail.com>

…ite field. Signed-off-by: SaJH <wogur981208@gmail.com>

sarahboyce

Thank you! This looks good to me

JaeHyuckSa force-pushed the issue-34624 branch from 2adf8a7 to 5785346 Compare December 2, 2024 15:27

JaeHyuckSa force-pushed the issue-34624 branch from 5785346 to 7770665 Compare December 3, 2024 03:39

JaeHyuckSa force-pushed the issue-34624 branch from 7770665 to 242d790 Compare August 22, 2025 17:14

JaeHyuckSa force-pushed the issue-34624 branch from 3c4668a to 4cdab7b Compare August 22, 2025 17:28

sarahboyce added selenium Apply to have Selenium tests run on a PR screenshots 🖼️ labels Aug 26, 2025

sarahboyce reviewed Aug 26, 2025

View reviewed changes

JaeHyuckSa force-pushed the issue-34624 branch from 4cdab7b to be0fecb Compare August 27, 2025 14:39

sarahboyce changed the title ~~Fixed #34624 - Removed edit and view options in RelatedFieldWidgetWrapper for radioselects.~~ Fixed #34624 -- Removed edit and view options in RelatedFieldWidgetWrapper for radioselects. Aug 28, 2025

JaeHyuckSa added 2 commits August 28, 2025 09:30

Fixed #34624 -- Removed change, delete, and view buttons for non-Sele…

0d2fcc5

…ct widgets in RelatedFieldWidgetWrapper. Signed-off-by: SaJH <wogur981208@gmail.com>

Refs #34624 -- Changed RedirectAdmin to use a Select widget for the s…

12cd2b7

…ite field. Signed-off-by: SaJH <wogur981208@gmail.com>

sarahboyce force-pushed the issue-34624 branch from be0fecb to 12cd2b7 Compare August 28, 2025 07:52

sarahboyce changed the title ~~Fixed #34624 -- Removed edit and view options in RelatedFieldWidgetWrapper for radioselects.~~ Fixed #34624 -- Removed change, delete, and view buttons for non-Select widgets in RelatedFieldWidgetWrapper. Aug 28, 2025

sarahboyce approved these changes Aug 28, 2025

View reviewed changes

sarahboyce merged commit eaaf01c into django:main Aug 29, 2025
34 checks passed

Uh oh!

Fixed #34624 -- Removed change, delete, and view buttons for non-Select widgets in RelatedFieldWidgetWrapper. #18877

Fixed #34624 -- Removed change, delete, and view buttons for non-Select widgets in RelatedFieldWidgetWrapper. #18877

Uh oh!

Conversation

JaeHyuckSa commented Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Trac ticket number

Branch description

Checklist

Uh oh!

BSmick6 commented Dec 2, 2024

Uh oh!

JaeHyuckSa commented Dec 3, 2024

Uh oh!

BSmick6 commented Dec 3, 2024

Uh oh!

sarahboyce commented Dec 3, 2024

Uh oh!

JaeHyuckSa commented Aug 22, 2025

Uh oh!

sarahboyce left a comment

Choose a reason for hiding this comment

Uh oh!

sarahboyce Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

JaeHyuckSa Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

sarahboyce Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

sarahboyce Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

JaeHyuckSa Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

sarahboyce Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

JaeHyuckSa Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

JaeHyuckSa commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

radio

select widget

Uh oh!

sarahboyce commented Aug 27, 2025

Uh oh!

sarahboyce left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JaeHyuckSa commented Dec 2, 2024 •

edited

Loading

JaeHyuckSa commented Aug 26, 2025 •

edited

Loading