Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some nested views incompatible with Django 4.0 admin changes #211

Open
gaige opened this issue Dec 22, 2021 · 4 comments
Open

Some nested views incompatible with Django 4.0 admin changes #211

gaige opened this issue Dec 22, 2021 · 4 comments

Comments

@gaige
Copy link

gaige commented Dec 22, 2021

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

@fdintino
Copy link
Member

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!

@gaige
Copy link
Author

gaige commented Dec 23, 2021

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.

@fdintino
Copy link
Member

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

@MinchinWeb
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants