django-nested-admin icon indicating copy to clipboard operation
django-nested-admin copied to clipboard

Some nested views incompatible with Django 4.0 admin changes

Open gaige opened this issue 2 years ago • 4 comments

We've been planning a move to Django 4.0 and haven't run into any showstoppers yet, however, on our admin pages, we've seen some unnecessary text showing up since the update. (Thanks for updating the build, etc. for 4.0!)

In my nested forms (which have a single FK and a single PK, which is a string that is useful to show to the admin user), I'm seeing the tabular.html producing a dictionary output on top of the printed string. For example:

{'name': 'code', 'label': 'code', 'help_text': '', 'field': 'code', 'is_hidden': True} appearing in the HTML after the text of the same field in <p>...</p> and the hidden <input> comprising the original field. The result is that the aforementioned text (a dictionary coming from the AdminReadonlyField's field member, build in the constructor. It appears there were some notable changes affecting how this is handled in this commit made in September of this year to align the stacked and inline views. However, this makes the current template in django-nested-admin place unnecessary text in the output.

The proximate cause appears to be that hidden is now being set to something other than False, which was it's previous value for anything that came from readonly.

I'd submit a PR, but I'm looking for guidance on:

  1. Appropriate way to adjust the behavior depending upon 4.0 vs pre-4.0 (maybe a separate template? not clear, but that's an architectural decision)
  2. Thoughts on ramifications of aligning the 4.0+ version of the html to the tabular view

Thanks, -Gaige

gaige avatar Dec 22 '21 18:12 gaige

There are some django version templatetag filters defined in nested_admin.templatetags. You could implement conditional logic in the template with something like:

{% load django_version_gte from nested_admin %}

{% if "4.0"|django_version_gte %}
  {# Django 4.0 variant #}
{% else %}
  {# Django 2.2/3.2 variant #}
{% endif %}

The templates are tricky: I have a need to add class names and data attributes on elements so that the inlines can be more easily manipulated in javascript, and to add sorting to the admin (when it's not available from grappelli) but those modifications aren't documented or enumerated anywhere, and they differ for stacked and tabular inlines (and for grappelli). Though it's generally pretty obvious which things are django-nested-admin specific.

The goal for the templates is that they should have full feature parity with the django admin, and the visual differences should be extremely minimal—I try to keep on top of the diffs across versions but I definitely fall short.

That said, there are some automated integration tests that checks that the admin is 100% visually identical for a non-nested non-sortable inline, whether it extends ModelAdmin or NestedModelAdmin (see here) that I've used to ensure I don't introduce visual regressions when I'm fixing version compatibility issues. If you could add a failing test case for this is_hidden issue it would be much appreciated!

fdintino avatar Dec 22 '21 20:12 fdintino

So, after a bit more experimentation I've determined that although it might make sense to deploy these changes in order to reduce the difference between the 4.x InlineTabular template and the NestedInlineTabular template, it was not the root cause of my problem.

I don't modify my Admin pages much, and it appears I considerably over-thought them when I built them. Aligning them more closely with best practices made my problems go away. In particular, I'd been using readonly_fields as a way to prevent changes across the entire page, when I should have just been providing has_change_permission. In this case, switching this up removes the problem.

I need to do a bit more digging to see if it's something that I can reproduce with any more sane configuration. I might be able to do this with a changeable layout with a primary key that shouldn't be edited (true in my case), but that I do want displayed. It may be a few days before I can look at this again.

gaige avatar Dec 23 '21 00:12 gaige

Okay, no rush...I wouldn't be able to respond until the new year anyway. Thanks!

fdintino avatar Dec 23 '21 00:12 fdintino

Ran into this today, and it's rather (visually) annoying. Hope to see a fix soon :)

image

Django 4.0.4 nested-admin v3.4.0

MinchinWeb avatar May 17 '22 16:05 MinchinWeb