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

Allow plugins to extend autotype decorator function #16091

Open
amyasnikov opened this issue May 11, 2024 · 2 comments
Open

Allow plugins to extend autotype decorator function #16091

amyasnikov opened this issue May 11, 2024 · 2 comments
Labels
complexity: medium Requires a substantial but not unusual amount of effort to implement status: under review Further discussion is needed to determine this issue's scope and/or implementation type: feature Introduction of new functionality to the application

Comments

@amyasnikov
Copy link
Contributor

amyasnikov commented May 11, 2024

NetBox version

v4.0.1

Feature type

Change to existing functionality

Proposed functionality

Problem
NetBox 4.0 Plugin migration guide suggests using autotype_decorator to convert filtersets into strawberry filters. However, this decorator does not support a lot of existing filters (e.g. ChoiceFilter):

import django_filters
from netbox.graphql.filter_mixins import autotype_decorator, BaseFilterMixin
from dcim import models, choices
import strawberry_django

class MyFilterSet(django_filters.FilterSet):
     type = django_filters.ChoiceFilter(choices=choices.ConsolePortTypeChoices)
     class Meta:
         model = models.ConsolePort
         fields = ['type']

@strawberry_django.filter(models.ConsolePort, lookups=True)
@autotype_decorator(MyFilterSet)
class MyFilter(BaseFilterMixin):
    pass

# NotImplementedError: GraphQL Filter field unknown: type: <django_filters.filters.ChoiceFilter object at 0x7fa9d06bb370>

This error also ruins all the work made by autotype_decorator for any other filters inside FilterSet. Hence, it would be very painful to handle it at the plugin side.

Possible Solution
Just use Dependency Injection to inject map_strawberry_type inside autotype_decorator:

def autotype_decorator(filterset, map_strawberry_type_fn=map_strawberry_type):
   ...
   should_create_function, attr_type = map_strawberry_type_fn(field)
   ...

This would allow plugin developers to supply their own map_strawberry_type_fn for their own filters when using autotype_decorator.

Use case

Plugin developers will be allowed to handle arbitrary filter types by themselves:

def map_strawberry_extra(field):
    create_fn, attr_type = map_strawberry_type(field)
    if attr_type is None and isinstance(field, ChoiceFilter):
        return True, str | None
    return create_fn, attr_type

@strawberry_django.filter(models.ConsolePort, lookups=True)
@autotype_decorator(MyFilterSet, map_strawberry_type_fn=map_strawberry_extra)
class MyFilter(BaseFilterMixin):
    pass

# No error

I can make a PR if you're okay with this change

Database changes

No response

External dependencies

No response

@amyasnikov amyasnikov added status: needs triage This issue is awaiting triage by a maintainer type: feature Introduction of new functionality to the application labels May 11, 2024
@jeffgdotorg
Copy link
Collaborator

Thank you for your interest in helping improve NetBox. Making life easier for plugin developers is important to us too.

My role at this stage is just to triage; others on the team with deeper understanding of the factors in play will need to take a closer look to determine whether your proposal is viable with fully gamed out. I'm setting the issue's status to under review and will raise it with the team.

Thanks for volunteering to work it through to a PR, but we ask that you please wait until you see the status move to accepted and the issue assigned to you before doing so.

@jeffgdotorg jeffgdotorg added status: under review Further discussion is needed to determine this issue's scope and/or implementation and removed status: needs triage This issue is awaiting triage by a maintainer labels May 13, 2024
@jeffgdotorg jeffgdotorg removed their assignment May 13, 2024
@jeffgdotorg jeffgdotorg changed the title New autotype_decorator is very plugin-unfriendly Rethink autotype_decorator approach to be friendlier to plugin authors May 13, 2024
@jeffgdotorg jeffgdotorg changed the title Rethink autotype_decorator approach to be friendlier to plugin authors Allow plugins to extend autotype decorator function May 13, 2024
@amyasnikov
Copy link
Contributor Author

@jeffgdotorg Hi, any news on this issue?

@jeremystretch jeremystretch added the complexity: medium Requires a substantial but not unusual amount of effort to implement label May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: medium Requires a substantial but not unusual amount of effort to implement status: under review Further discussion is needed to determine this issue's scope and/or implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

No branches or pull requests

3 participants