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

Feature Request: Simplify Public API #12

Open
isik-kaplan opened this issue Oct 6, 2024 · 1 comment
Open

Feature Request: Simplify Public API #12

isik-kaplan opened this issue Oct 6, 2024 · 1 comment
Assignees
Labels
feature request New feature or request

Comments

@isik-kaplan
Copy link

Hi, there is no option for a feature request or a question so I had to use the bug template but cleared everything inside, jfyi.

I was looking for a basic django admin autocomplete solution that supported django 4+ and stumbled upon this one and from the readme docs it looks fine but I was curious if you were planning on making the public api simpler. I haven't started using this yet, just browsing but wanted to ask regardless.

@admin.register(Post)
class PostAdmin(DALFModelAdmin):
    list_filter = (
        ('author', DALFRelatedOnlyField),    # if author has a post!
        ('category', DALFRelatedFieldAjax),  # enable ajax completion for category field (FK)
        ('tags', DALFRelatedFieldAjax),      # enable ajax completion for tags field (M2M)
    )

This is fine but I'd prefer

@admin.register(Post)
class PostAdmin(DALFModelAdmin):
    # Perhaps an optional argument deciding if all relations should automatically be converted to autocomplete filters?
    # list_filters_are_autocomplete_list_filters = True  # should be default imo but I don't mind, I already have a base admin class
    list_filter = (
        'author',
        'category',
        'tags', 
    )

I've checked the admin code and I think this is as simple as registering a Lookup to the ORM. Same concept apparently.

# some init file or some file that's imported during usage of this library

from django.db.models import OneToOneField, ForeignKey, ManyToManyField
from django.contrib.admin.filters import FieldListFilter

# functools.partial doesn't work because isinstance is a CPython function that doesn't take keyword arguments, *sigh*
type_checker = lambda klass: lambda item: isinstance(item, klass)

FieldListFilter.register(type_checker(OneToOneField), DALFRelatedOnlyField)
FieldListFilter.register(type_checker(ForeignKey), DALFRelatedFieldAjax)
FieldListFilter.register(type_checker(ManyToManyField), DALFRelatedFieldAjax)

# If you want to implement list_filters_are_autocomplete_list_filters the flag from the above example,
# I believe you should be able to wrap the second argument of FieldListFilter.register instead of just putting DALFRelatedOnlyField 

# For the below function to work you need list_filters_are_autocomplete_list_filters=default_boolean set in the DALFModelAdmin

def decide_filter_class(filter_class):
    # Django doesn't provide any better way to hook into this place for us to decide on a filter class easier if we want to 
    # tie it to a condition on the model_admin class
    def get_filter_class(field, request, lookup_params, model, model_admin, field_path):
        if model_admin.list_filters_are_autocomplete_list_filters:
            return filter_class
        for test, list_filter_class in FieldListFilter._field_list_filters:
            # If it equals and list_filters_are_autocomplete_list_filters we already returned above
            if test(field) and list_filter_class != filter_class: 
                return list_filter_class(field, request, params, model, model_admin, field_path=field_path)
    return get_filter_class
    
FieldListFilter.register(type_checker(OneToOneField), decide_filter_class(DALFRelatedOnlyField))
FieldListFilter.register(type_checker(ForeignKey), decide_filter_class(DALFRelatedFieldAjax))
FieldListFilter.register(type_checker(ManyToManyField), decide_filter_class(DALFRelatedFieldAjax))

I'm not sure if I got the whole idea behind DALFRelatedFieldAjax vs DALFRelatedOnlyField at a first glance but ^ method should allow you to set the distinction I believe.

I haven't tested out the code at all but from the looks of django's source code and your code this seems like it should work.

@isik-kaplan isik-kaplan added the bug Something isn't working label Oct 6, 2024
@vigo
Copy link
Owner

vigo commented Oct 6, 2024

Well, that would be lovely if you implemented it and made a PR. I was following Django's conventions and don't want to override the existing logic. As Django suggests, I like to keep things clean and simple. Future changes on Django's side shouldn't affect it heavily.

@vigo vigo added feature request New feature or request and removed bug Something isn't working labels Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants