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

RFC: Custom filters and orderings #1868

Open
anehx opened this issue Sep 6, 2022 · 3 comments
Open

RFC: Custom filters and orderings #1868

anehx opened this issue Sep 6, 2022 · 3 comments

Comments

@anehx
Copy link
Contributor

anehx commented Sep 6, 2022

Description of the problem

Caluma defines a fix set of filters and orderings for each graph in our schema. If an implementing app requires an extension of said fix sets, most of the time it can be implemented in the core. However, there are use-cases in which it doesn't make sense to put it into the core because it would be too complex to implement it dynamically enough to be in the core of Caluma.

For instance: let's say we want to filter all cases that have an answer (date) "decision-date" on a document on a work item of the task "decision" but only if that work item is completed.

To implement this in the core, a massively complex extension of the hasAnswer or searchAnswer filter would be necessary, for a filter that could be solved with a very basic custom implementation:

class CustomFilter(DateFilter):
    def filter(self, queryset, value):
        return queryset.filter(
            Exists(
                Answer.objects.filter(
                    question_id="decision-date",
                    date=value,
                    document__work_item__status=WorkItem.STATUS_COMPLETED,
                    document__work_item__case=OuterRef("pk"),
                )
            )
        )

If such custom filters would be allowed by the Caluma core, we'd have a method to not unnecessarily make the core too complex and still be able to implement the needed features. This RFC proposes an extension of the API to allow custom filters and orderings.

Possible solutions

Option A

Caluma allows two new extension setting CUSTOM_FILTER_CLASSES and CUSTOM_ORDERING_CLASSES. Those should be defined as a dict which keys are the existing filterset / orderset classes that should be extended and a string which holds the location of the custom filterset / orderset class:

CUSTOM_FILTER_CLASSES=QuestionFilterSet=myapp.caluma.extensions.custom_filters.CustomQuestionFilterSet;CaseFilterSet=myapp.caluma.extensions.custom_filters.CustomCaseFilterSet

Which would result in a value of

settings.CUSTOM_FILTER_CLASSES = {
    "QuestionFilterSet": "myapp.caluma.extensions.custom_filters.CustomQuestionFilterSet",
    "CaseFilterSet": "myapp.caluma.extensions.custom_filters.CustomCaseFilterSet",
}

Now, this would then generate a new parameter customFilter for all graphs using the QuestionFilterSet or CaseFilterSet:

query {
  allCases(customFilter: [{ foo: "Test" }]) {
    # ...
  }
}

There is already a proof of concept of this implementation here: #1867

Option B

Configuration would be the same as in option a but the custom filters would be merged into the existing filter parameter. This option would result in a slimmer schema but it brings more technical problems such as colliding filters etc. Also, I'm not 100% sure that this would even be possible since the filter / order parameter currently needs one backing filterset / orderset class.

The API usage would look like this:

query {
  allCases(filter: [{ foo: "Test" }]) {
    # ...
  }
}

Pros / cons

  • (+) Consuming apps can define custom filters and orderings tailored for their use case
  • (+) No more bloated core filters to make specific filtering / ordering use cases possible
  • (-) Custom filters / orderings may have an impact on performance and possible future performance optimizations
  • (-) The Caluma schema can vary in each consuming application
@czosel
Copy link
Contributor

czosel commented Sep 7, 2022

I like the proposal, and I think we could handle the impact of varying schemas 👍

@winged
Copy link
Contributor

winged commented Sep 14, 2022

I do like the idea, but I see two primary issues that we need to solve:

  • I suggest extending by adding stand-alone filters (as functions or part of a class can be discusseD), but not whole filtersets. This would allow a simpler API for implementation / extension. Also, I'd use the type (either model or graphene object type) as an "anchor point" instead of the filterset class.
  • We do need to take care of the type system - How far should we go in allowing custom input types?

Regarding the Python API

The current FilterSet classes can dynamically add properties to add the new filters. The corresponding setting could look like this:

settings.CUSTOM_FILTERS = {
    "Question": {
        "customFilter": "myapp.caluma.custom_filters.my_custom_question_filter"
    },
    "Case": {
        "anotherFilter": "myapp.caluma.custom_filters.my_custom_case_filter",
        "oneMore": "myapp.caluma.custom_filters.one_more_filter"
    }
}

Regarding the type system

I suggest adding a few "simple" base filtes to use that extend the existing django-filters types, so users can easily implement their own choice filters for example.

For more complex filters, I suggest providing a "generic" input type where we don't enforce an exact syntax: Just pass in a piece of JSON, similar to the meta fields. This would avoid forcing implementors to define their input types, which could lead to some added complexity. This shouldn't be a problem, because the people who implement the custom filters most likely are also the same who will implement the client code (or at least will be on the same project team).

This won't keep implementors from adding their own input types if they really need, but we should strive to make the normal case as simple as possible

@czosel
Copy link
Contributor

czosel commented Oct 10, 2023

As discussed with @winged @anehx: The suggested PoC looks good, and can be implemented. The preferred API is Option B (if not too hard to implement). Potential naming collisions of custom filters with core filters can throw an exception.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants