Proposal: Resolving the read_only widget dilemma and establishing a deprecation path (Tickets #30577 & #35959)

Django Internals
1

Hi everyone,

I recently started looking into Ticket #30577 (customizing read-only fields). While diving in, I noticed it has some concerned references to the recent fix for Ticket #35959 (specifically this part of commit d755a98).

It appears that the fix for #35959 had an unintended broader impact. By removing the getattr(widget, "read_only", False) workaround, it inadvertently affected third-party and custom code that relied on this undocumented behavior as a feature, since developers currently have no other way to render custom read-only widgets in the admin (see this comment and this post for more info) - reminds me of Hyrum’s Law.

So it looks like there’s a dilemma:

  • Removing the check entirely (current state): Fixes #35959, but affects widespread community reliance on read_only=True without providing an alternative.

  • Reverting the check as-is: Restores the ecosystem but brings back the crash reported in #35959.

I wonder if we should revisit #35959 and handle this through a standard deprecation cycle:

  1. Restore the getattr check, but wrap it in a try...except (NotImplementedError, AttributeError): block. This defensively prevents the #35959 crashes while keeping custom read-only widgets functional for the time being.

  2. Deprecation Warning: Within that same block, issue a RemovedInDjango70Warning for any widget utilizing this behavior.

  3. Official API: Prioritize the design and implementation of an official, documented solution for read-only fields (Ticket #30577) so the community has a target to migrate toward - I can give it a shot, although I honestly haven’t dug much into #30577 yet - I got distracted figuring out #35959 :stuck_out_tongue:

  4. Final Removal: Once the deprecation cycle concludes (and hopefully #30577 is concluded), we can remove the workaround once again.

To help make this second pass on #35959 concrete, I have put together a Draft PR demonstrating a possible mitigation and the warning here: Draft PR #21072

Cheers!

3 Likes
3

Hi @rodbv, is there any update on this topic?

1 Like
4

Since my last post here I concluded the most useful thing was to implement what #30577 actually asks for. That PR is now up: Fixed #30577 -- Added ModelAdmin.readonly_formfield_overrides. by rodbv · Pull Request #21686 · django/django · GitHub

It adds ModelAdmin.readonly_formfield_overrides, a mapping of field names to widgets used when a field renders read-only, either because it is in readonly_fields or because the user only has view permission:

class PrettyJSONWidget(forms.Widget):
    def render(self, name, value, attrs=None, renderer=None):
        return format_html("<pre>{}</pre>", json.dumps(value, indent=2))


class PersonAdmin(admin.ModelAdmin):
    readonly_fields = ("metadata",)
    readonly_formfield_overrides = {"metadata": PrettyJSONWidget}

The lookup is resolved from the ModelAdmin rather than the form, since the admin builds the form with read-only fields excluded. That is also why the old read_only widget hack only worked for fields declared on a custom form.

UserAdmin is the first consumer, replacing the declared-field workaround for the password hash. For those affected by the 6.0 removal (comments 13 and 16 on the issue), this covers the same use cases with less code.

5

The snippet in my post above no longer works: I went back through the ticket history and changed the API. The dict is keyed by the model field class now, matching formfield_overrides, and there is a get_readonly_widget() method for a single field.

class PersonAdmin(admin.ModelAdmin):
    readonly_fields = ("metadata",)
    readonly_formfield_overrides = {
        models.JSONField: {"widget": PrettyJSONWidget},
    }

@nemesifier, one caveat for the OpenWISP case: widget Media is not collected for read-only fields, because AdminReadonlyField has no media property. A plain-HTML widget like the JSON one above works, but a leaflet widget renders without its CSS and JavaScript. PR #21674 (ticket #14643) adds that property, and I have noted on both tickets that the two need to fit together.

Details: #30577 (Custom rendering for readonly fields in admin) – Django

1 Like