Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix notice Undefined index: operator #69 #193

Merged
merged 2 commits into from
Nov 28, 2018

Conversation

tegos
Copy link
Contributor

@tegos tegos commented Nov 23, 2018

No description provided.

Copy link

@stryaponoff stryaponoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix works for me in Laravel 5.7.14

@oliversarfas
Copy link

Tested as working for Laravel 5.7.14. Please merge in and address the tests as I guess they do not account for "InRaw"

@mikebronner
Copy link
Owner

HI everyone, thanks for submitting this. I will take a look as soon as I can. At the moment I am very busy and can't promise I will get to it right away.

@mikebronner mikebronner self-assigned this Nov 27, 2018
@drbyte
Copy link
Contributor

drbyte commented Nov 27, 2018

A report for investigating is posted in #195

@@ -97,7 +97,7 @@ protected function getQueryColumns(array $columns) : string

protected function getTypeClause($where) : string
{
$type = in_array($where["type"], ["In", "NotIn", "Null", "NotNull", "between", "NotInSub", "InSub", "JsonContains"])
$type = in_array($where["type"], ["InRaw", "In", "NotIn", "Null", "NotNull", "between", "NotInSub", "InSub", "JsonContains"])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the other new types should be supported here too:
"InRaw", "NotInRaw", "IntegerInRaw", "IntegerNotInRaw"

And for completeness probably should be added to the array in getOtherClauses too.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will also need to get the tests written.

@mikebronner
Copy link
Owner

Working on implementing this now.

@mikebronner mikebronner merged commit d4b4f34 into mikebronner:develop Nov 28, 2018
@mikebronner
Copy link
Owner

@tegos @oliversarfas @stryaponoff @drbyte Could you guys provide eloquent queries that trigger each of the new types? I have been playing around with it, but so far my queries aren't triggering the new types. (Please reply here in form of comments.)

@mikebronner
Copy link
Owner

@drbyte I wasn't able to find any references to the other types you mentioned above in the Laravel framework code.

Also, I have been able to replicate the triggering the whereInRaw query type, and have written a test for it. Update coming shortly.

@drbyte
Copy link
Contributor

drbyte commented Nov 28, 2018

@mikebronner here are screenshots from the 5.7.13 to 5.7.14 diff I linked to:

screen shot 2018-11-28 at 4 04 44 pm

screen shot 2018-11-28 at 4 04 20 pm

screen shot 2018-11-28 at 4 03 26 pm

@drbyte
Copy link
Contributor

drbyte commented Nov 28, 2018

@mikebronner BTW thanks for the speedy new release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants