ncrypthic / doctrine-graphql

Doctrine ORM to GraphQL bridge
MIT License
2 stars 1 forks source link

filter Type 'xxxSearch' doesn't allow nested filtering #2

Open nuncanada opened 5 years ago

nuncanada commented 5 years ago

An example from my Model:

type StmModelSIPOC_WEBTbMigracaoPlanoAcaoPage { total: Int page: Int limit: Int filter: StmModelSIPOC_WEBTbMigracaoPlanoAcaoSearch match: StmModelSIPOC_WEBTbMigracaoPlanoAcaoSearch sort: StmModelSIPOC_WEBTbMigracaoPlanoAcaoSort items: [StmModelSIPOC_WEBTbMigracaoPlanoAcao] }

input StmModelSIPOC_WEBTbMigracaoPlanoAcaoSearchInput { id: [SearchFilterInput] dataHora: [SearchFilterInput] usuario: [SearchFilterInput] justificativa: [SearchFilterInput] }

type StmModelSIPOC_WEBTbMigracaoPlanoAcao { id: Int! dataHora: DateTime! usuario: String! justificativa: String planoAcaoOrigem: StmModelSIPOC_WEBTbPlanoAcao planoAcaoDestino: StmModelSIPOC_WEBTbPlanoAcao }

Notice how I am unable to filter by planoAcaoOrigem properties, that is a must for me!

I think there should be a major refactoring, instead of input StmModelSIPOC_WEBTbMigracaoPlanoAcaoSearchInput { id: [SearchFilterInput] dataHora: [SearchFilterInput] usuario: [SearchFilterInput] justificativa: [SearchFilterInput] } Something like:

input SearchFilterInput { path: String operator: SearchOperator value: String }

And instead of creating a SearchInput for each Type, let them all use this new one.

I am willing to do the work for this, but does it interest you?

ss-limafriyadi commented 5 years ago

hi @nuncanada , thank you for reporting. You're right, nested search was not implemented yet. You're contribution is most welcome. :pray:

ncrypthic commented 5 years ago

Anyway, I think we need to keep the property filter input as List of SearchFilterInput. Because we might need to filter the same property that fulfill more than one condition. (e.g date > last week and date < yesterday).

Or maybe you have better idea?

StmModelSIPOC_WEBTbMigracaoPlanoAcaoSearchInput {
id: [SearchFilterInput]
dataHora: [SearchFilterInput]
usuario: [SearchFilterInput]
justificativa: [SearchFilterInput]
}
nuncanada commented 5 years ago

Yes, and I think my proposal do add a Path isn't very GraphQL in spirit. I think the correct option would be to have something like Generics (2nd-order types):

StmModelSIPOC_WEBTbMigracaoPlanoAcaoSearchInput { id: [SearchFilterInput<Int!>] dataHora: [SearchFilterInput<DateTime!>] usuario: [SearchFilterInput<String!>] justificativa: [SearchFilterInput] planoAcaoOrigem: [SearchFilterInput] planoAcaoDestino: [SearchFilterInput] }

This example is following Java's syntax for Generics...

Reading about Generic's issue for GraphQL itself: https://github.com/graphql/graphql-spec/issues/190

Do you have another suggestion on how to create the syntax for nested filtering that doesn't make the schema still a lot bigger than it currently already is?

nuncanada commented 5 years ago

The same problem in another library Strapi in another language Python: https://github.com/strapi/strapi/issues/2297

Here is how they solved it: https://github.com/strapi/strapi/pull/1948

image

nuncanada commented 5 years ago

The advantage of using dotNotation instead of the graphql object is that it guarantees only 1 path per condition, it doesn't make sense to have more than that because you only have 1 value to compare to.

Using for example: path: "planoAcaoOrigem.idOinfo.name" operator: like value: "%test%"

https://github.com/graphql/graphql-spec/issues/174

ncrypthic commented 5 years ago

By using dotNotation, at least in doctrine queryBuilder, means we have to manage logical relationship aliases between entities. I had done this before, and it's not trivial.

As the generics & flatten path issues was not resolve yet and also since the schema was generated dynamically from doctrine metadata, I think we should generate the relational fields on the xxxSearchInput type and implement the mechanics to apply that filter.

input StmModelSIPOC_WEBTbMigracaoPlanoAcaoSearchInput {
    id: [SearchFilterInput]
    dataHora: [SearchFilterInput]
    usuario: [SearchFilterInput]
    justificativa: [SearchFilterInput]
    planoAcaoOrigem: StmModelSIPOC_WEBTbPlanoAcaoSearchInput
    planoAcaoDestino: StmModelSIPOC_WEBTbPlanoAcaoSearchInput
}

What do you think?

nuncanada commented 5 years ago

Yes, that matches exactly the style of the rest of the schema. Overnight I thought about it and was going to propose exactly this approach. It should be really easy to make these appear in the schema, not sure how easy it is to make the queries work in the backend.

About handling the aliases, I do have working code to handle the aliases! I have already built working GraphQL-like engine using Doctrine Metadata, but with very few features. If you think that could be useful somehere else, I can commit it here.

ncrypthic commented 5 years ago
About handling the aliases, I do have working code to handle the aliases! I have already built working GraphQL-like engine using Doctrine Metadata, but with very few features. If you think that could be useful somehere else, I can commit it here.

If you could share some code, that'll be really helpful.

Anyway, currently trying to implement on the backend side. Hopefully. I can finish this on 1 or 2 days.

ncrypthic commented 5 years ago

Sorry, I accidentally pushed the close button

ncrypthic commented 5 years ago

@nuncanada can you try the nested filter feature on using version ~1.1?

ncrypthic commented 5 years ago

@nuncanada , please let me know the result when you have try this.