lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

Better alternative for @whereConditions/@orderBy

Open LastDragon-ru opened this issue 4 years ago • 22 comments

I'm tried to use the existing @whereConditions, unfortunately, I very fast found that it is a bit strange and doesn't allow you to control available relations and their properties + it uses Mixed type that makes queries unsafe (eg "string < 10" probably is an error). Initially, I wanted to create a PR with fixes, but I carried away and created two new directives instead 😁

Will be glad for any feedback :)

@searchBy

scalar Date @scalar(class: "Nuwave\\Lighthouse\\Schema\\Types\\Scalars\\Date")

type Query {
  users(where: UsersQuery @searchBy): ID! @all
  comments(where: CommentsQuery @searchBy): ID! @all
}

input UsersQuery {
  id: ID!
  name: String!
}

input CommentsQuery {
  text: String!
  user: UsersQuery
  date: Date
}

That's all, just search 😃

# Write your query or mutation here
query {
  # WHERE name = "LastDragon"
  users(where: {
    name: {eq: "LastDragon"}
  }) {
    id
  }

  # WHERE name != "LastDragon"
  users(where: {
    name: {eq: "LastDragon", not: yes}
  }) {
    id
  }

  # WHERE name = "LastDragon" or name = "Aleksei"
  users(where: {
    anyOf: [
      {name: {eq: "LastDragon"}}
      {name: {eq: "Aleksei"}}
    ]
  }) {
    id
  }

  # WHERE NOT (name = "LastDragon" or name = "Aleksei")
  users(where: {
    anyOf: [
      {name: {eq: "LastDragon"}}
      {name: {eq: "Aleksei"}}
    ]
    not: yes
  }) {
    id
  }

  # WHERE date IS NULL
  users(where: {
    date: {isNull: yes}
  }) {
    id
  }

  # Relation: WHERE EXIST (related table)
  comments(where: {
    user: {
      where: {
        date: {between: {min: "2021-01-01", max: "2021-04-01"}}
      }
    }
  }) {
    id
  }

  # Relation: WHERE COUNT (related table) = 2
  comments(where: {
    user: {
      where: {
        date: {between: {min: "2021-01-01", max: "2021-04-01"}}
      }
      eq: 2
    }
  }) {
    id
  }
}

@sortBy

Very close to @orderBy but in addition allows using HasOne/BelongTo relations for ordering 😊

type Query {
  users(order: UsersSort @sortBy): ID! @all
  comments(order: CommentsSort @sortBy): ID! @all
}

input UsersSort {
  id: ID!
  name: String!
}

input CommentsSort {
  text: String
  user: UsersSort
}
query {
  # ORDER BY user.name ASC, text DESC
  comments(order: [
    {user: {name: asc}}
    {text: desc}
  ])
}

LastDragon-ru avatar Apr 03 '21 12:04 LastDragon-ru

I like the idea of nesting the operator and value within an input object equal to the field name, that does indeed allow for better type safety. I can see us adding something like that to Lighthouse, but in the form you currently implemented it does have a problem: there is a name collision for fields named allOf, anyOf or not. How can we avoid this?

I am curious if you managed to do negation of nested condtitionals, e.g. not(a=1 and b=2). Is that possible? Small tidbit: not being a single value enum feels overly clever, I would just use not: Boolean = false.

spawnia avatar Apr 04 '21 11:04 spawnia

Thank you for your feedback.

there is a name collision for fields named allOf, anyOf or not. How can we avoid this?

Yes, this is a known limitation and, unfortunately, I don't know how to avoid this conflict. Maybe a directive like @rename, but it should rename field after operator matching, not sure that this is possible (?).

I am curious if you managed to do negation of nested condtitionals, e.g. not(a=1 and b=2). Is that possible?

Yep, you just need and not and Builder will wrap it into not (...) or and not (...).

I would just use not: Boolean = false

I'm actually not sure about the enum. I added it because { not: false } does nothing, moreover

isNull: false
not: true

looks very confusing.

LastDragon-ru avatar Apr 04 '21 18:04 LastDragon-ru

I think the potential for a naming conflict is very low, we could just accept that for ergonomic reasons. The operators Lighthouse already has (AND, OR - and once we fix the implementation NOT) are reserved in SQL anyways, no sane person would use them as column names.

Concerning NOT: I would love to see support for it added to Lighthouse. It is unclear to me if a single attribute or nesting provides a better API:

input WhereConditions {
  NOT: WhereConditions
# vs.
  NOT: Boolean = false
}

spawnia avatar Apr 05 '21 09:04 spawnia

About not I think would be better to change it (and probably I will do it on this weekend):

  1. Comparison operators: {eq: 1, not: yes} => {notEqual: 1}

  2. Logical: {anyOf: [...], not: yes} => not { anyOf: [...]}

This will make it more consistent with allOf/anyOf and compatible with Tagged Type/oneOf input. And also will resolve ambiguity for Relation:

# this means "where count(...) != 2" now
relation: {
  where: {....}
  eq: 2
  not: yes
}

After the change, not will be always "doesntHave" => code above will throw an error.

Still not sure about isNull/isNotNull, probably they still will use yes:

isNull: yes
isNotNull: yes

because from my point of view:

isNull: false
isNotNull: false

a bit hard to understand.

LastDragon-ru avatar Apr 05 '21 15:04 LastDragon-ru

Still not sure about isNull/isNotNull, probably they still will use yes.

How about:

enum Is {
  NULL
  NOT_NULL
}

spawnia avatar Apr 09 '21 19:04 spawnia

How about:

Mmm... Not sure, because this will break the "one operator = one property = one action" rule)

isNull: yes
isNotNull: yes

Anyway, your variant also will be possible with a custom Operator :)

and probably I will do it on this weekend

Updated, but it is more complicated than I expected and still needs some work/refactor for Composite operator (relation), will continue on this weekend.

Current structure (not yet pushed anywhere):

"""
Conditions for the related objects (`has()`) for input Nested.

See also:
* https://laravel.com/docs/8.x/eloquent-relationships#querying-relationship-existence
* https://laravel.com/docs/8.x/eloquent-relationships#querying-relationship-absence
"""
input SearchByComplexRelationNested {
  # Used as marker for ComplexOperator
  relation: SearchByFlag! = yes

  # Conditions for related objects
  where: SearchByConditionNested

  # Conditions for count
  count: SearchByScalarInt

  """
  Shortcut for `doesntHave()`, same as:
  
  \```
  where: [...]
  count: {
    lt: 1
  }
  \```
  """
  not: Boolean! = false
}

"""
Available conditions for input Nested (only one property allowed at a time).
"""
input SearchByConditionNested {
  """All of the conditions must be true."""
  allOf: [SearchByConditionNested!]

  """Any of the conditions must be true."""
  anyOf: [SearchByConditionNested!]

  """Not."""
  not: SearchByConditionNested

  """Property condition."""
  value: SearchByScalarStringOrNull
}

LastDragon-ru avatar Apr 12 '21 15:04 LastDragon-ru

Still in WIP state... The good news:

  • Fully rework how simple (=comparison/logical) operators can add their own types (it much much easier/better now)
  • Added support for custom complex operators other than built-in Relation, they can be added by directive and allow transform input type into any condition (eg to support #1243).

What is left:

  • While reworking of the directive the way how it determines existing operators has changed, they will be stored inside directive arguments instead of config (the main reason - custom complex directives). It is a bit complicated and not yet fully done.

Also about @sortBy (already available in the master branch)

  • Fixed where conditions, they will be used in joins
  • Added MorphOne support

LastDragon-ru avatar Apr 25 '21 11:04 LastDragon-ru

Finally, it released 🎉

composer require lastdragon-ru/lara-asp-graphql

Documentations: https://github.com/LastDragon-ru/lara-asp/blob/master/packages/graphql/README.md

LastDragon-ru avatar May 16 '21 15:05 LastDragon-ru

Meanwhile @sortBy support the following relations:

  • HasOne (https://laravel.com/docs/8.x/eloquent-relationships#one-to-one)
  • BelongsTo (https://laravel.com/docs/8.x/eloquent-relationships#one-to-many-inverse)
  • MorphOne (https://laravel.com/docs/8.x/eloquent-relationships#one-to-one-polymorphic-relations)
  • HasOneThrough (https://laravel.com/docs/8.x/eloquent-relationships#has-one-through)

🤗

LastDragon-ru avatar Jun 12 '21 18:06 LastDragon-ru

Breaking News: @sortBy support Laravel Scout and TypesRegistry 🎉

Also there few breaking changes in @searchBy:

  • short-named operators (lt, lte, etc) renamed into full form (lessThan, etc).
  • Relation will use notExists instead of not + added exists

Full changelog: https://github.com/LastDragon-ru/lara-asp/releases/tag/0.7.0

LastDragon-ru avatar Aug 15 '21 09:08 LastDragon-ru

Latest news 🥳

  • @searchBy
    • New operators for String: contains, startsWith, endsWith
    • TypeRegistry support
  • @sortBy
    • will use dependent subqueries instead of joins (they are much faster with classic pagination)
    • types auto-generation - no need to create input by hand for sorting 😀
# Schema
type Query {
  "input will be generated automatically 😛"
  comments(order: _ @sortBy): [Comment!]! @all
}

type Comment {
  text: String
}

# Result
type Query {
  """input will be generated automatically 😛"""
  comments(order: [SortByClauseComment!]): [Comment!]!
}

"""Sort clause for type Comment (only one property allowed at a time)."""
input SortByClauseComment {
  """Property clause."""
  text: SortByDirection
}

Full changelog: https://github.com/LastDragon-ru/lara-asp/releases/tag/0.9.0

LastDragon-ru avatar Oct 24 '21 12:10 LastDragon-ru

@LastDragon-ru I like this allot ;) is it possible to integrate this into Lighthouse, or are there additional requirements to meet @spawnia objections/ vision?

JustinElst avatar Dec 12 '21 10:12 JustinElst

@MrGekko thanks :) Would be good to integrate it, but lighthouse requires "php": ">= 7.2" when my package(s) works only with 8.0 - I have no time (and to be honest I do not want) to downgrade the code. Also, both directives will be heavily refactored soon (~month or so) to support raw SQL expressions (and to use Arguments instead of args array)

LastDragon-ru avatar Dec 12 '21 14:12 LastDragon-ru

The next major version of Lighthouse will probably be PHP 7.4+, so we won't be able to do a copy-paste integration anytime soon. I am happy to let @LastDragon-ru experiment with an improved API though, perhaps it can make its way into Lighthouse at some point.

spawnia avatar Dec 12 '21 16:12 spawnia

As Laravel 9 will require PHP 8.0, is there any chance to see this PR merged in Lighthouse? It seems to be the only available way to sort by a related field property.

esistgut avatar Feb 07 '22 16:02 esistgut

Since 0.14.0 the @sortBy directive also supports BelongsToMany, MorphToMany, HasMany, HasManyThrough, and MorphMany relations 🎁

LastDragon-ru avatar Apr 02 '22 09:04 LastDragon-ru

Finaly, v1.0.0 is released. It is more about huge refactoring to have operator directives instead of list of classes and for future changes (as I've mentioned in https://github.com/nuwave/lighthouse/issues/1782#issuecomment-991911658), but also added support of bitwise operators support (&, |, ^, >>, <<) for @searchBy 🎉

LastDragon-ru avatar Aug 06 '22 13:08 LastDragon-ru

v2.0.0 is here 🥳 The major changes are:

  1. PHP 8.2 (v1 should also work, but with few deprecation messages)
  2. Deep refactoring to make creation of custom operators easy (eg it is possible to implement https://github.com/LastDragon-ru/lara-asp/issues/11 now)
  3. List of operators will be determinate by the actual Builder (so when you use @search you will not see operators which are not supported by Scout Builder)
  4. Scout support for @searchBy
  5. Placeholder/Types auto-generation/_ support for @searchBy (so you no need to create separate input anymore)
  6. Order by random for @sortBy

Please see full release note and documentation for more details. Please also note that minimal version of Lighthouse set to "^5.68.0".

LastDragon-ru avatar Dec 25 '22 05:12 LastDragon-ru

is there any hope to see this inside mainstream lighthouse?

esistgut avatar Mar 19 '23 00:03 esistgut

The new version with Lighthouse v6 support is released! 🎉

About merging inside Lighthouse: looks like PHP and Laravel versions are the same now, but the package depends from few other packages -> it is not possible just copy it into Lighthouse -> a lot of work required. I have no time to work on it, unfortunately. There is also some chance that API may be changed a bit (eg it is definitely required for complexity calculation, and maybe for json support).

LastDragon-ru avatar Apr 01 '23 09:04 LastDragon-ru

The first version was released almost 3 years ago, and today the v6.2.0 with Laravel v11 support is released 🎉 The v6 finally solved collision between fields names and operators names - fields moved into field: { name: ... }[^1]. IMHO, I should have done this in the first release 🤷‍♂️

# Old
users(
  where: { 
    name: { equal: "..." } 
  }
) { ... }

# New
users(
  where: { 
    field: { 
      name: { equal: "..." } 
    }
  }
) { ... }

[^1]: it is possible to restore old syntax if needed

LastDragon-ru avatar Mar 28 '24 12:03 LastDragon-ru

Any updates on when/if this will make it into Lighthouse?

williamjallen avatar May 13 '24 17:05 williamjallen

Any updates on when/if this will make it into Lighthouse?

To be honest, I'm not sure that it will happen anytime soon. Since the first versions there were a lot of changes, code base growth significantly, more external dependencies added (including my other packages), etc. This is a huge work. I have no time for that 🤷‍♂️

Why not just use my package? 🤗

LastDragon-ru avatar May 14 '24 05:05 LastDragon-ru