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: Dynamic filtering/includes/excludes support (inspired by dynamic-rest) #462

Open
bgervan opened this issue Aug 15, 2024 · 1 comment
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested

Comments

@bgervan
Copy link

bgervan commented Aug 15, 2024

Hey,

thanks for the great project.

https://github.com/AltSchool/dynamic-rest

I am searching for a solution to have features like in this package. Especially the filtering, but the exclude/include could be super useful as well.

Is it something feasible or can be built to the system in a reasonable time?

@hbakri
Copy link
Owner

hbakri commented Sep 1, 2024

Hey @bgervan! Thank you for your interest in the project and for suggesting this feature! I appreciate the idea of implementing functionality similar to the dynamic-rest package.

While I don't believe this feature should be built into the ListView due to security concerns and potential over-complexity, it should be possible to implement it using my package, by creating your extension of the ListView that enables this feature for your specific project.

With this in mind, I attempted to develop this feature using a FilterSchema. Here's what I found:

I was able to create a schema that works independently for dynamic filtering:

class DynamicFilterSchema(ninja.FilterSchema):
    dynamic_filters: dict[str, Any] = pydantic.Field(default_factory=dict)
    dynamic_filter_regex: ClassVar[re.Pattern] = re.compile(r'^filter\{(.+)}$')

    @pydantic.model_validator(mode='wrap')
    @classmethod
    def parse_dynamic_filters(cls, values: Any, handler, info) -> Any:
        if isinstance(values, dict):
            dynamic_filters = {}
            for key, value in list(values.items()):
                if match := cls.dynamic_filter_regex.match(key):
                    dynamic_filters[match.group(1)] = value
                    values.pop(key)

            values['dynamic_filters'] = dynamic_filters

        processed_values = handler(values)
        return processed_values

This schema successfully handles dynamic filters in isolation. Here's a simple test demonstrating its functionality:

data = {"filter{first_name}": "John", "filter{age__gte}": 30}
model = DynamicFilterSchema(**data)
assert model.dynamic_filters == {"first_name": "John", "age__gte": 30}

However, when attempting to integrate this schema with Django Ninja, I encountered significant challenges.

@router.get("/", response=list[UserResponse])
def list_users(request, filters: Query[DynamicFilterSchema]):
    print(filters)
    return User.objects.all()

The main obstacle lies in Django Ninja's internal handling of query parameters, specifically in the _map_data_paths method of the ParamModel class (located in ninja/params/models.py):

class ParamModel(BaseModel, ABC):
    ...

    @classmethod
    def _map_data_paths(cls, data: DictStrAny) -> DictStrAny:
        flatten_map = getattr(cls, "__ninja_flatten_map__", None)
        if not flatten_map:
            return data

        print(data)  # {"filter{first_name}": "John", "filter{age__gte}": 30}
        print(flatten_map)  # {'dynamic_filters': ('filters', 'dynamic_filters'))}
        mapped_data: DictStrAny = NestedDict()
        for k in flatten_map:
            if k in data:
                cls._map_data_path(mapped_data, data[k], flatten_map[k])
            else:
                cls._map_data_path(mapped_data, None, flatten_map[k])

        print(mapped_data)  # {'filters': {}}
        return mapped_data

    @classmethod
    def _map_data_path(cls, data: DictStrAny, value: Any, path: Tuple) -> None:
        if len(path) == 1:
            if value is not None:
                data[path[0]] = value
        else:
            cls._map_data_path(data[path[0]], value, path[1:])

This method strips out our dynamic filters before they reach the Pydantic model validation step, making it challenging to implement the desired functionality within the current Django Ninja framework.

@vitalik, as the creator of Django Ninja, do you have any thoughts on how we might implement this dynamic filter feature using your framework? The solution implemented by dynamic-rest might not be the best fit for Django Ninja, but I'm curious about your perspective on handling dynamic filtering in a way that aligns with Django Ninja's design principles.

@bgervan, if you're still set on implementing this feature with the "filter{...}" format, you might need to create your own custom parameter handler to bypass Django Ninja's default behavior. Alternatively, you could explore different approaches that don't rely on this specific field name format, which might be easier to implement within the current constraints of Django Ninja.

If we don't hear back from @vitalik, I'll raise an issue on the Django Ninja repository to address this or find some help. This issue will remain open for now, feel free to share any alternative ideas or approaches you have in mind! ✨

@hbakri hbakri self-assigned this Sep 1, 2024
@hbakri hbakri added help wanted Extra attention is needed question Further information is requested labels Sep 1, 2024
@hbakri hbakri moved this to In Progress in Django Ninja CRUD Backlog Sep 1, 2024
@hbakri hbakri changed the title Enhancement: Add dynamic filters/includes/excludes etc Feature request: Dynamic filtering/includes/excludes support (inspired by dynamic-rest) Sep 1, 2024
@hbakri hbakri added the enhancement New feature or request label Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
Status: In Progress
Development

No branches or pull requests

2 participants