timgws / QueryBuilderParser

A simple to use query builder for the jQuery QueryBuilder plugin for use with Laravel.
MIT License
159 stars 65 forks source link

Added feature to use custom methods for filtering #23

Open xenco opened 6 years ago

xenco commented 6 years ago

See the added part in README for further explanation.

timgws commented 6 years ago

Hi xenco!

Thanks for your first commit to the project!

To help ensure that there is high quality code inside this project, you will note that we have automated testing. This is provided by a service called Travis.

When a new pull request is made (or changes are made to a branch with a pull request), Travis will automatically try running the code base in a few different configurations.

This automatic test has failed.

2) timgws\test\JoinSupportingQueryBuilderParserTest::testDateBetween
timgws\QBParseException: Field (dollar_amount) is in no list

/home/travis/build/timgws/QueryBuilderParser/src/QueryBuilderParser/QBPFunctions.php:276
/home/travis/build/timgws/QueryBuilderParser/src/QueryBuilderParser/QueryBuilderParser.php:251
/home/travis/build/timgws/QueryBuilderParser/src/QueryBuilderParser/JoinSupportingQueryBuilderParser.php:70
/home/travis/build/timgws/QueryBuilderParser/src/QueryBuilderParser/QueryBuilderParser.php:80
/home/travis/build/timgws/QueryBuilderParser/src/QueryBuilderParser/QueryBuilderParser.php:60
/home/travis/build/timgws/QueryBuilderParser/tests/JoinSupportingQueryBuilderParserTest.php:299

It appears that some of the logic for comparing fields in the database may be broken, as most of the tests have failed.

To run these tests manually on your computer when developing, run these commands inside the project folder:

composer install --prefer-source --no-interaction --dev
vendor/bin/phpunit --testsuite MySQL --coverage-clover ./tests/logs/clover.xml

Also, feel free to ask for any help if you need it!

xenco commented 6 years ago

It doesn't break the default functionality anymore. But I'm not sure how to test the added features. I can't just check the produced (prepared) queries, because they need the bindings, which themselves need some test data sets. How would I create a test database with migration and seeding for sqlite:memory in isolation? Any help is appreciated.

timgws commented 6 years ago

I can't just check the produced (prepared) queries, because they need the bindings

You actually should be able to do that. I don't think that doing a full test is really all that necessary.

We only need to make sure that we correctly generate queries that we expect. We don't need to do an end to end test, as that would be the responsibility of the project that is adding this library as a dependency.

Here is an example test that I have written: https://github.com/timgws/QueryBuilderParser/blob/master/tests/QueryBuilderParserTest.php#L272-L286

timgws commented 6 years ago

That being said, here is an instance where the project does test bindings:

https://github.com/timgws/QueryBuilderParser/blob/7c138423c15be3d77d0e1e66f3adc7d4666160dd/tests/JoinSupportingQueryBuilderParserTest.php#L310-L330