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

Added RegExp parsing after GraphQL variable subsituition #4230

Merged
merged 4 commits into from
Nov 19, 2019

Conversation

harshil-goel
Copy link
Contributor

@harshil-goel harshil-goel commented Oct 31, 2019

Fixes #3268

Currently, after GraphQL variables were being substituted, RegExp was not being parsed. This PR adds parsing after the substitution of variables.


This change is Reviewable

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm: Please address comments then merge. Change title and add description.

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @harshil-goel and @pawanrawal)


go.mod, line 59 at r1 (raw file):

	google.golang.org/genproto v0.0.0-20190516172635-bb713bdc0e52 // indirect
	google.golang.org/grpc v1.23.0
	gopkg.in/DATA-DOG/go-sqlmock.v1 v1.3.0 // indirect

Do we need this?


go.sum, line 328 at r1 (raw file):

google.golang.org/grpc v1.23.0 h1:AzbTB6ux+okLTzP8Ru1Xs41C303zdcfEht7MQnYJt5A=
google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg=
gopkg.in/DATA-DOG/go-sqlmock.v1 v1.3.0 h1:FVCohIoYO7IJoDDVpV2pdq7SgrMH6wHnuTyrdrxJNoY=

Can you check if you need this thing?


gql/parser.go, line 454 at r1 (raw file):

			_, ok := vmap[v.Value]
			if f.Func.Name == "regexp" && ok {

Add a comment here that we need to parse the regexp after substituting it from a GraphQL Variable.

@harshil-goel harshil-goel merged commit a0514c4 into master Nov 19, 2019
@harshil-goel harshil-goel changed the title Added RegExp filter to func name Added RegExp parsing after GraphQL variable subsituition Nov 19, 2019
@harshil-goel harshil-goel deleted the harshil-goel/regexp branch November 26, 2019 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

RegExp Variable Replacement Not Working on Filters
3 participants