Cast datetimes and add 'not between' operator

[sgjackman]

Datetimes were not being cast, so would fail to work for BETWEEN clauses.

NOT BETWEEN was not present, so I added this in.

[coveralls]

Coverage decreased (-6.6%) to 93.443% when pulling 406dbf15559e3412b8a197061ce896d2f665e5b3 on sgjackman:master into 254486179ba8806e068f030715d67778f034e5e9 on timgws:master.

[coveralls]

Coverage decreased (-6.6%) to 93.443% when pulling 406dbf15559e3412b8a197061ce896d2f665e5b3 on sgjackman:master into 254486179ba8806e068f030715d67778f034e5e9 on timgws:master.

[timgws]

Hi @sgjackman!

I see that this is your first GitHub contribution on this account 🎉

Thanks very much for your pull request! I have a few comments for the pull request that you have made. If you can perform the three changes mentioned above, the code will be merged.

1. Please try and keep to the coding style. We try to keep to PSR-2. This means not using tabs, but 4 spaces. Also, it means ensuring that the parenthesis are in the right place.
2. The check for $rule->type == 'datetime' might be better placed inside the getCorrectValue function. getCorrectValue should probably have a check for the type (ie, the if) and call a new function (which should probably be placed in QBPFunctions.php) to perform the cast.
3. A test should be added in tests/QBPFunctionsTest.php so that the code for casting the value to a Carbon item is covered. We want to have tests for every single line of code in the repository!

---

https://github.com/timgws/QueryBuilderParser/blob/406dbf15559e3412b8a197061ce896d2f665e5b3/src/QueryBuilderParser/QBPFunctions.php#L23-L24

https://github.com/timgws/QueryBuilderParser/blob/406dbf15559e3412b8a197061ce896d2f665e5b3/src/QueryBuilderParser/QBPFunctions.php#L46-L47

For point 1, above; when making changes to existing files (similar to QBPFunctions.php above) please try and make sure that the lines are consistent.

---

Making these changes will ensure that the code base is consistent and bug free!

[wpanec-uno]

Hi @sgjackman, I'm extremely new to laravel and PHP (I am learning for a project in a class) so please forgive the naiveness. I branched your changes and have made the changes @timgws recommended except for the test portion as I'm not sure how to do that yet.

However, when I try to use the new code for a between using a datetime, it is not generating the between as expected. The following JSON does not populate the value portion of my between: { "condition": "AND", "rules": [ { "id": "dollar_amount", "field": "dollar_amount", "type": "double", "input": "number", "operator": "less", "value": "546" }, { "id": "needed_by_date", "field": "needed_by_date", "type": "date", "input": "text", "operator": "between", "value": [ "10/22/2017", "10/28/2017" ] } ], "not": false, "valid": true }

Instead, this is the result:

Do you have any ideas?

[timgws]

Sorry - the state of this is incorrect. It is not merged, my bad.

To answer your questions @wpanec-uno, this is how you write the tests: https://github.com/timgws/qbpclone/commit/0fcff992a9bf4ba176e04bd3be022e80b4edab19

To cover your new code for the datetime, I changed the second test: https://github.com/timgws/qbpclone/blob/master/tests/JoinSupportingQueryBuilderParserTest.php#L275-L295

$this->assertEquals('select * where `needed_by_date` not between ? and ?', $builder->toSql());

This ensures that the SQL query generated by the QBP is correct.

It also ensures that all the values that are sent to the QueryBuilder (from Laravel) are as they should be.

The biggest change needed was this one line:

https://github.com/timgws/qbpclone/commit/843440f4c4347b891ccb4035cbae26a1b0654bce#diff-d7a109249daf3e1d5f9edcaae4e5dbf8

Because the ->rule value we check against is not the datetime column type from MySQL, but rather the date from the client, the value of the string needs to be updated.

Running the test allowed me to see that straight away 🎉

If you could please pull those changes into your repo, and create a new pull request, that would be awesome!

[timgws]

Also, just as a quick side note, I have not actually tested this inside the query builder on a web page. I will do that a little later.

I have a feeling that the QueryBuilder may not be converting the Object (the Carbon object, that is) to a String.

If this is the case, change the function here: https://github.com/timgws/qbpclone/blob/master/src/QueryBuilderParser/QBPFunctions.php#L163-L179

where the lines state:

return new Carbon($value);

to:

return (new Carbon($value))->toDateTimeString();

[sgjackman]

Hi @timgws, I've been using the dates inside a project and they are working great - so not sure if the conversion is needed?

On another note - I'm really struggling to get the JoinSupportingQueryBuilderParser working. It seems like your example code is not quite right - are you able to give a more detailed example of how this works?

Much appreciated!

[timgws]

OK, cool. Thanks @sgjackman! It looks like it should work (the unit tests pass!), but I have not yet been able to test it on MySQL & Elasticsearch. I will get around to that soon-ish.

wrt JoinSupportingQueryBuilderParser, what are you trying to do?

[sgjackman]

I was trying to do left / inner joins but I realised that it's probably a bit out of scope for the query builder.

For the project I'm working on I had to end up filtering out certain rules and writing my own parser for them, then adding these queries onto the query builder query.

Theres a ton of tables I'm joining in, but it was easy enough to add these as joins in the same fashion.

[timgws]

hey @sgjackman, would love to know what the specific use case was, especially if it's something that was integrated directly with the QB UI.