Unnecessary check for thruthiness in view_on_site generation?

marcorichetta · October 4, 2020, 10:14pm

From another discussion I found the function used to generate the view_on_site url in the admin.

django/django/blob/292b3be698ef58aff9c215d62a444f66ead578c3/django/contrib/admin/options.py#L293


        form_field.help_text = format_lazy('{} {}', help_text, msg) if help_text else msg
    return form_field

def get_autocomplete_fields(self, request):
    """
    Return a list of ForeignKey and/or ManyToMany fields which should use
    an autocomplete widget.
    """
    return self.autocomplete_fields

def get_view_on_site_url(self, obj=None):
    if obj is None or not self.view_on_site:
        return None

    if callable(self.view_on_site):
        return self.view_on_site(obj)
    elif self.view_on_site and hasattr(obj, 'get_absolute_url'):
        # use the ContentType lookup if view_on_site is True
        return reverse('admin:view_on_site', kwargs={
            'content_type_id': get_content_type_for_model(obj).pk,
            'object_id': obj.pk

Thing is, why self.view_on_site is checked for truthiness in the elif statement?
If it were False it should be captured in the first statement of the function. (L294)

The function was added 7 years ago in it’s entirety so I don’t think this check is there for backwards compatibility.

andrewgodwin · October 4, 2020, 10:40pm

This looks like it might just be an oversight by the original patch author - if you can remove it and the tests pass, then I think it’s sensible to remove it now.

marcorichetta · October 5, 2020, 1:38am

Great!
I’ll try to make my first (tiny) contribution to Django source code.

Thanks Andrew!

marcorichetta · October 6, 2020, 11:06pm

Hello Andrew.

Considering the tiny change in the code, should I make the PR directly from Github? As I understand, tests should ran on djangoCI.

Either case, is django-docker-box still relevant?

andrewgodwin · October 7, 2020, 5:40pm

As long as your contribution ends up as a pull request, how you get there isn’t a massive concern - just make sure it follows all the PR rules and things should be good!

Not sure about django-docker-box - I never used it personally, since I have a waaaay more complex local setup

Topic		Replies	Views
Testing a view Mystery Errors	4	309	March 31, 2023
{% url %} template tag, get_absolute_url(), and views Templates & Frontend	6	4256	June 1, 2023
Loading view depending on a condition Getting Started	6	2570	June 1, 2022
Regarding getabsoluteurl function in model based views Using Django	1	1851	July 7, 2021
square brackets bug Mystery Errors	7	92	March 30, 2024

Unnecessary check for thruthiness in view_on_site generation?

Related Topics