Lighthouse: Add @like directive

[olivernybroe]

What problem does this feature proposal attempt to solve?
Make it easier for adding a new directive for making like queries.

This is for example something that is really nice when wanting to search for a name in all users.

Which possible solutions should be considered?

We currently have a @search directive, which is really nice. However this is only for Laravel Scout. A @like directive would be usable in many other cases.

So I think this directive should be configurable, so you can define where you want a percentage sign. My thought were the following

users(name: String @like(percentage: BOTH)): [User!]! @all

So we make a ENUMfor percentage, with the following options

enum PercentageLocation {
    START
    END
    BOTH
    NONE
}

We might consider not having NONE as an option and making BOTH the default.

[olivernybroe]

Implementation is quite simple, it's basically copy/pasting EqDirective and making a check for where we want the percentage sign.

Possibly also escaping % already present in the values string.

[spawnia]

Great suggestion, I like it!

We might consider not having NONE as an option and making BOTH the default.

Using NONE is the same as just using @eq, so I agree we should not have that option. BOTH seems like a reasonable default.

[olivernybroe]

@spawnia I think this could also be a good first issue. Relatively straight forward.

[brunobg]

I'm getting this one. PR soon.

[olivernybroe]

@brunobg Amazing! If you need any help, just say so 👍

[brunobg]

Yes @olivernybroe thanks! is there any documentation about what writing/running tests? I'm getting a lot of errors here from existing tests.

[olivernybroe]

@brunobg Only documentation about it is what is in https://github.com/nuwave/lighthouse

However you can draw a lot of inspiration from the tests here
https://github.com/nuwave/lighthouse/blob/master/tests/Integration/Execution/ArgBuilderDirectiveTest.php

The test suite should work, are you running it via docker or without?
When running without you need to change connection information to the database in phpunit.xml.

[brunobg]

I mean the structure for running the test itself, like creating a database, etc. I'm running without docker. I found some expected env vars at https://github.com/nuwave/lighthouse/blob/507a45268dccbb95674a8b14234858b27b7179b2/tests/DBTestCase.php. I could write a small doc about this on CONTRIBUTING or somewhere like that if you give me some pointers.

[olivernybroe]

@brunobg Yeah, a PR for that would be very welcome. Hmm think there might be more needed for the local setup easily. It looks like the env function is not working correctly.

Also the following line
https://github.com/nuwave/lighthouse/blob/507a45268dccbb95674a8b14234858b27b7179b2/tests/DBTestCase.php#L32
Is actually locking the database to be named test.

For using locally, so you can make the pr for @like directive, just change the variables directly in DBTestCase and don't commit that file.

We need a separate pr so env can be used and an update in contributing.md so users know how to set it up.

[brunobg]

sorry @olivernybroe but I couldn't run the test here. I'll try with docker later, but I was going to spend a few minutes to implement this and spent more time to try to make it run. Some docs about it would really be helpful, including on how to automatically style the code.

[olivernybroe]

@brunobg Sorry for that, can hear we definitely have to spend some time improving our workflow so it's easier for new users to contribute :)

You can run composer stan for static analysis check and then we are using Styleci for styling

[spawnia]

The project setup only has a single dependency on docker-compose, optionally providing a Makefile for convenience. That is described in https://github.com/nuwave/lighthouse/blob/master/CONTRIBUTING.md#setup.

I don't see how this could be any simpler?

[brunobg]

Well, it could be simpler. Here's a list.

1. my docker was not compatible with the version requested. I could have easily run the tests without docker if there were any instructions about it, however, and I was willing to do it and even help document it.
2. there was no list of what static analysis tools should be ran. I could easily run stan and whatever style fixes if there was a simple "run this and that". .editorconfig was not enough to catch "we don't use spaces with the . operator".
3. no clear standards for variables names, documentation, graphql description etc. Particularly adding directives should have a clear guide for new comers or for 3rd party packages providing interesting directives but that are not general enough to include in the main package.
4. from your review: so I can't add the enum to the directive class, but I couldn't find documentation on where to add it. It seems logical to aggregate it there.

And I am reasonably familiar with lighthouse since I'm integrating it with another project of mine, so I already had a pretty good idea of what to do for this issue. Avoiding the "it couldn't be simpler because I know it" is a good way to ensure you'll get more help from external developers. I took this issue, which is not useful to me at all, just to give something back since I was asking questions on Slack (which you always answer, kudos) and saw it there. I could send more PRs if the project was more welcoming.

[brunobg]

Also docker is failing for me: https://github.com/nuwave/lighthouse/issues/1648

[spawnia]

Avoiding the "it couldn't be simpler because I know it" is a good way to ensure you'll get more help from external developers.

I will admit that the way i phrased my response was a bit provocative. It provoked a great response out of you though ;)

This project is certainly not perfect and thus open to improvements.

[olivernybroe]

@brunobg thanks for the very valuable feedback.

We are grateful for you wanting to help out, and hopefully we can find some time to make the developer experience nicer for contributers

@spawnia I think we should create some documentation issues for helping newcomers contribute. Particularly how to contribute with new directives is probably the most common scenario.

[brunobg]

Thanks for the thoughtful answers @spawnia and @olivernybroe. If I can help in some way let me know. This is a great project and thanks for all the effort.

[spawnia]

As I commented in https://github.com/nuwave/lighthouse/pull/1645#pullrequestreview-557797097, the existence of the wildcard _ and the multitude of combination options really makes me question the usefulness of hardcoding the % location on the server. A simpler and more flexible solution would be to simply allow clients to send the entire query string, including wildcard characters.

[olivernybroe]

@brunobg We spend a little time improving testing setup 😄

Feel free to come with any feedback on improvements. https://github.com/nuwave/lighthouse/blob/master/CONTRIBUTING.md#native-tools

[brunobg]

Much better, thanks!