Opened 14 years ago

Closed 9 years ago

#12974 closed Bug (duplicate)

Admindocs app introspection omits several model methods

Reported by: Jared Forsyth Owned by: Jared Forsyth
Component: contrib.admindocs Version: dev
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 14 years ago.
patch
admindocs_methods_trunk.diff (612 bytes ) - added by Jared Forsyth 14 years ago.
diff'd against the root

Download all attachments as: .zip

Change History (17)

by Jared Forsyth, 14 years ago

Attachment: admindocs_methods.diff added

patch

comment:1 by Jared Forsyth, 14 years ago

Keywords: admindocs added

comment:2 by Gert Van Gool, 14 years ago

Needs tests: set

comment:3 by Russell Keith-Magee, 14 years ago

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.

by Jared Forsyth, 14 years ago

diff'd against the root

comment:4 by Jared Forsyth, 14 years ago

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 by Jared Forsyth, 14 years ago

Needs tests: unset
Patch needs improvement: unset

comment:6 by Russell Keith-Magee, 14 years ago

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 by Jared Forsyth, 14 years ago

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 by Ramiro Morales, 13 years ago

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

comment:9 by Luke Plant, 13 years ago

Type: Bug

comment:10 by Luke Plant, 13 years ago

Severity: Normal

comment:11 by danols@…, 13 years ago

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 by Daniel Sokolowski <danols@…>, 13 years ago

Cc: danols@… added

(adding myself to cc)

comment:13 by Aymeric Augustin, 11 years ago

Status: reopenednew

comment:14 by Tim Graham, 10 years ago

Component: contrib.admincontrib.admindocs

comment:15 by Žan Anderle, 9 years ago

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