Opened 7 years ago

Closed 12 months ago

#12974 closed Bug (duplicate)

Admindocs app introspection omits several model methods

Reported by: Jared Forsyth Owned by: Jared Forsyth
Component: contrib.admindocs Version: master
Severity: Normal Keywords: admin, docs, method, admindocs
Cc: danols@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

The problem is that some methods are preprocessed (especially those with a @decorator) are ignored by admin docs, because they take no arguments as opposed to a single (self) argument. This patch changes a single character (from '==' to '<=').

Attachments (2)

admindocs_methods.diff (537 bytes) - added by Jared Forsyth 7 years ago.
patch
admindocs_methods_trunk.diff (612 bytes) - added by Jared Forsyth 7 years ago.
diff'd against the root

Download all attachments as: .zip

Change History (17)

Changed 7 years ago by Jared Forsyth

Attachment: admindocs_methods.diff added

patch

comment:1 Changed 7 years ago by Jared Forsyth

Keywords: admindocs added
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:2 Changed 7 years ago by Gert Van Gool

Needs tests: set

comment:3 Changed 7 years ago by Russell Keith-Magee

Patch needs improvement: set
Resolution: worksforme
Status: newclosed

Firstly - please generate patches against the root of the source tree.

Secondly, due to the absence of a concrete test case (either in code, or in the problem description), I can't work out what problem it is that this patch is trying to solve.

Changed 7 years ago by Jared Forsyth

diff'd against the root

comment:4 Changed 7 years ago by Jared Forsyth

Resolution: worksforme
Status: closedreopened

When admindocs displays methods for a model, it only displays ones w/ no arguments, or rather with only the (self) argument. The reason for this is no doubt to display only those methods which are valid for use in a template.

This patch is trying to solve the following problem:
Occasionally a model can have a method with no arguments, due to preprocessing @decorators or the use of the django.utils.curry function. These methods are then overlooked by admindocs, which checks for exactly one argument.
With this patch, it will check for fewer than or equal to one required argument.

The 'test' case that brought this problem to my attention was using that django-basic-apps blog app, whose Post model defines a get_absolute_url method. This method has a @permalink decorator before it, which results in Post.get_absolute_url having zero arguments. Given that the current code checks for exactly one argument, this method (which is totally valid for use in a template) is not displayed on the docs page.

Looking deeper into the code, I found that any get_absolute_url (even without a @permalink) gets curryd by the base model, and would therefore never be displayed by admindocs.

comment:5 Changed 7 years ago by Jared Forsyth

Needs tests: unset
Patch needs improvement: unset

comment:6 Changed 7 years ago by Russell Keith-Magee

Component: Documentationdjango.contrib.admin
Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Ok - problem now confirmed. For future reference: While I was eventually able to piece together an example that broke, it took some work. The best test case isn't a longer, more verbose verbal explanation -- it's a working sample of code that demonstrates the breakage, along with a short set of instructions for how to cause the breakage. For example:

Sample model:

class Thing(models.Model):
    name = models.CharField(max_length=100)
    
    @models.permalink
    def get_absolute_url(self):
        "Get the URL"
        return ('blog_detail', None, {})

    def button(self):
        "Push the button"
        pass

If you inspect /admin/doc/models/myapp.thing/ (the admin docs for the Thing model), button will be displayed, but get_absolute_url will not.

The best part is that this programatic test case can form the start of the test for your patch. Your patch doesn't have a test case, so the 'needs test case' flag still applies. The fact that this particularly feature doesn't have any tests at the moment doesn't give you a pass to not include tests -- it's an opportunity to add tests to make sure the feature will continue to work.

I've also turned on patch needs improvement -- At this point, the check for function arguments is redundant, since you can't have a function with a negative argument count.

comment:7 Changed 7 years ago by Jared Forsyth

Could you explain the 'redundant' comment? The function must not have any required arguments (at least that is my impression of the desired behavior), and the check is therefore still necessary. Are you referring to some line other than the one that I modified?

comment:8 Changed 6 years ago by Ramiro Morales

Summary: Admin docs omits several methods for a ModelAdmindocs app introspection omits several model methods

comment:9 Changed 5 years ago by Luke Plant

Type: Bug

comment:10 Changed 5 years ago by Luke Plant

Severity: Normal

comment:11 Changed 5 years ago by danols@…

Easy pickings: unset
UI/UX: unset

Please note that even if you remove @models.permlink from above example get_absolute_url still fails to be documented in the admin documentation. Further more functions that take no arguments that are template friendly also fail to be documented (see model definition below).

However this is not the reason I am adding to this ticket. I believe the assumption that admindoc should only document template friendly functions is flawed. For the admindoc app to be useful as a documentation portal for both the template designer and django programmer all functions should be included; template friendly functions can simply be differentiated visually.

Documenting all functions simply means removing len(inspect.getargspec(func)[0]) == 1 from the if statement on line 242. If needed I can submit a patch.

Thank you

class Example(models.Model):
    ### django native method
    def get_absolute_url(self):
        """ I'm not in the admin """
        pass
        
    def button(self):
        "I'm in the admin"
        pass
    
    ### extra model functions
    def get_next_order():
        """ I'm not in the admin """
        pass
    
    name = models.CharField(help_text='Name', max_length=20)

comment:12 Changed 5 years ago by Daniel Sokolowski <danols@…>

Cc: danols@… added

(adding myself to cc)

comment:13 Changed 4 years ago by Aymeric Augustin

Status: reopenednew

comment:14 Changed 2 years ago by Tim Graham

Component: contrib.admincontrib.admindocs

comment:15 Changed 12 months ago by Žan Anderle

Resolution: duplicate
Status: newclosed

I believe this can be closed, as it was fixed by #24917

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