Opened 3 years ago

Closed 22 months ago

#18205 closed Bug (invalid)

Admin validate() method suggests erroneous invocation?

Reported by: Keryn Knight <django@…> Owned by: nobody
Component: contrib.admin Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

According to the docstring for validate() in django.contrib.admin.validation (currently):

Calls custom validation classmethod in the end if it is provided in cls.
The signature of the custom validation classmethod should be: def validate(cls, model).

I take this to mean that a ModelAdmin subclass may provide a validate method of it's own, to ensure additional checks are run. However, doing something like:

class MyModelAdmin(admin.ModelAdmin):
[...]
    @classmethod
    def validate(cls, model):
        from pdb import set_trace; set_trace()

Never yields an interactive shell, nor can I see anywhere in the validation code where it might even be attempting to call it.

As it's undocumented, as far as I know (thus an internal API) this could be considered a case of:

  • the docstring not being clear enough [at least, for me to understand]
  • the docstring never being correct [because it has appeared that way since 1.0]
  • the functionality having been removed at some point [perhaps wrongly]

Apologies if this is a dupe, but the search results were rather mired in form validation and the admin.

Change History (6)

comment:1 Changed 3 years ago by jezdez

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

This is a dupe, the validate function is called by default from the admin site class, and is not a public API.

https://github.com/django/django/blob/ecc8208f881340c7b97500e47af6315055182503/django/contrib/admin/sites.py#L91-92

comment:2 Changed 3 years ago by Keryn Knight <django@…>

Public API or not, I cannot see how the docstring is correct, and I would submit that it ought to be clarified or changed to better reflect the functionality.

The docstring suggests that a custom validation method may be set on cls - it is ambiguous as to what cls may be, is it the AdminSite instance, or the ModelAdmin instance, or something else altogether? (According to pdb, it's the ModelAdmin class). This could stand to be clarified.

The docstring also indicates, that before the AdminSite is loaded (assuming the conditions of being in DEBUG mode are met), this custom validation method will run, but nothing in the code seems to do so, and seems borne out by attempting to load pdb, or print to stdout inside said custom method. This would appear to negate the entire docstring section.

May I ask that, if it is a dupe, the ticket number or discussion link is provided, for future reference?

comment:3 Changed 3 years ago by kmtracey

  • Resolution invalid deleted
  • Status changed from closed to reopened

Possibly #12674 is the dupe that was thought of? Given it is 2 years old and doesn't seem to have anyone actively interested in working on it, I'm reopening this one to correct the existing docstring, which near as I can tell is simply wrong, and has been wrong since its introduction into the code. That docstring shouldn't state it calls custom validation if it does not do so.

comment:4 Changed 3 years ago by julien

  • Triage Stage changed from Unreviewed to Accepted

comment:5 Changed 2 years ago by aaugustin

  • Status changed from reopened to new

comment:6 Changed 22 months ago by timo

  • Resolution set to invalid
  • Status changed from new to closed

Since #12674 has been fixed, this is no longer valid.

Note: See TracTickets for help on using tickets.
Back to Top