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

Multiple OR operators #109

Open
Grimmoth opened this issue Nov 29, 2023 · 6 comments
Open

Multiple OR operators #109

Grimmoth opened this issue Nov 29, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@Grimmoth
Copy link

Hi there,

I have the following scenario, using version 2.0.0 alpha 3, referring to the just implemented #82.

In many scenarios I need to filter out data by two columns which could be an integer or null. I could do so with an @or operator (just beginning, so maybe I just don't know the right approach for that), but if I try to add multiple OR operators to the filters it only adds one of them in the request url.
I verified that postgrest allows this by using a custom curl request.

I don't want you to spend time on something that could just be my fault, my question is simply: has it been tested already (in which case I'll investigate on my side) or should I post what I've tried and experienced so far?

My current workaround is fairly stupid, I add the two filters to the request url in the dataprovider.

If it helps, I'm happy to test that and try around - just to get rid of any unnecessary workarounds. ;)

@scheiblr
Copy link
Collaborator

scheiblr commented Dec 6, 2023

Hey @Grimmoth, thank you very much for pointing that out! I think any information you want to share might be helpful - especially the link to the custom curl of postgREST.

@Grimmoth
Copy link
Author

Grimmoth commented Dec 7, 2023

Ok, short story long 🙂

My scenario

Every row contains two columns: cluster and cluster_configuration which can both be of an integer value or null.
In my settings I can select which cluster and cluster_configuration I want to see, those values are then being used in the list filters.
So for example I selected cluster: 1 and also cluster_configuration: 1. This selection should end in a list which shows entries matching the following values:

|cluster|cluster_configuration|...other columns..|
|-------|---------------------|
|   1   |         1           |
|  null |        null         |
|  null |         1           |
|   1   |        null         |

So if an entry has cluster: 2 or cluster_configuration: 2 etc. it should not be in the list.

Testing/Playing around

To keep testing simple, I just modified the filter parameter in the dataprovider before calling the postgrestRestProvider.
So the default request url contains just (I leave out unnecessary paging, sorting stuff..):
?filter=%7B%7D , so no filters at all, showing all rows as expected.

a) Adding one OR filter to the parameters:
filter:{@or: { "cluster_configuration_id": 1, "cluster_configuration_id@is": null }}
-> resulting in ?or=%28cluster_configuration_id.eq.1%2Ccluster_configuration_id.is.null%29 working as expected

b) adding another OR filter

filter: { 
@or: {cluster_configuration_id: 1, cluster_configuration_id@is: null},
cl@or: {cluster_id: 1, cluster_id@is: null}
}

-> resulting in: ?or=%28cluster_id.eq.1%2Ccluster_id.is.null%29
so it seems the @or filter containting cluster_configuration has just been replaced

c) trying to combine with AND

filter:{
  @and: {
    c@or: { "cluster_configuration_id": 1, "cluster_configuration_id@is": null },
    @or: { "cluster_id": 1, "cluster_id@is": null }
  }
}

-> throwing error Error: Error in data provider: HttpError2: "failed to parse logic tree ((or.(cluster_id.eq.1,cluster_id.is.null)))" (line 1, column 7)
and Cannot set properties of undefined (setting '@and')
-> resulting in ?and=%28or.%28cluster_id.eq.1%2Ccluster_id.is.null%29%29 so, again, seems that it cut out the second @or filter there

My workaround for all of that atm is just modifying the the request url in the httpClients in the data provider used by the crud calls:
filter:{}
-> after modifying the url it results in ?or=%28cluster_id.eq.2%2Ccluster_id.is.null%29&or=%28cluster_id.eq.2%2Ccluster_configuration_id.is.null%29
(I didn't try that with an AND operator yet, but this simple approach is working as expected)

CURL

curl command for a fetching list results:
curl -H "Accept-Profile: ui" -H "Prefer: count=exact" -H "Authorization: Bearer <myToken I copied from Browser DevTools>" 'http://localhost:3001/sources?or=%28cluster_id.eq.1%2Ccluster_id.is.null%29&or=%28cluster_configuration_id.eq.1%2Ccluster_configuration_id.is.null%29&hidden=eq.false&order=id.asc'
-> returned:

[{"id":1,"cluster_id":null,"cluster_configuration_id":null,...blablablaData}, 
 {"id":2,"cluster_id":1,"cluster_configuration_id":1,...blablablaData}, 
 {"id":11,"cluster_id":null,"cluster_configuration_id":null,...blablablaData}]⏎  

Sorry that I didn't check everything in the urlBuilder.ts myself, but if necessary I can investigate further as soon as I got enough time for that.

Best regards

@scheiblr
Copy link
Collaborator

scheiblr commented Dec 8, 2023

Thanks a lot, that is a really great explanation, with which I can create a test case and procede from there.

@scheiblr
Copy link
Collaborator

scheiblr commented Dec 29, 2023

Hey @Grimmoth, I was just looking into that issue in more detail.

I did not really end up with THE solution, but had one idea, which I would like to share with you.

Instead of using two @or of which one is artificially generated, I would suggest to use an array inside of the or, which would be mapped to two ors in the url in the end:

filter: {
    '@or': [{ "cluster_configuration_id": 1, "cluster_configuration_id@is": null }, { "cluster_id": 1, "cluster_id@is": null} ],
}

The same approach would work with @and. This requires changes in the URL generation, i.e. we need a custom and more complicated function for qs.stringify(query) which is used multiple times in the data provider and we need to change the function parseFilters of the urlBuilder.

The nesting which you tried is another issue and would make things complicated, i.e. requiring some recursion. Thus, I would first consider the proposed change and from there go further.

What do you think? Maybe you have a more sophisticated idea.

EDIT: You can look at the test cases here

@scheiblr scheiblr added the enhancement New feature or request label Dec 29, 2023
@Grimmoth
Copy link
Author

Grimmoth commented Jan 2, 2024

Happy new year and thank you for the quick reply.

I don't have a better idea, but for my example it would be absolutely enough to have an array - the hopeful idea of my @and was also only to get the two conditions in the url - I don't really care if it's done by nesting or not.
I also don't want to overcomplicate things, imho it's better to have it as simple and robust as possible.

Best regards

@scheiblr
Copy link
Collaborator

Hey @Grimmoth unfortunately, I did not yet find some time to tackle the request. If you have more ressources, I could lead and consult you in doing the change.

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

When branches are created from issues, their pull requests are automatically linked.

2 participants