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

Can't create a string map from a string literal during CONTAINS flow #11

Open
dseabolt opened this issue Dec 17, 2019 · 5 comments
Open

Comments

@dseabolt
Copy link

This is related to this libraries use of Flagr, but I don't feel like the Flagr code is treating it incorrectly.

Essentially, if you do a CONTAINS test with a string (e.g. Property CONTAINS substring), the substring part of the expression comes in as a StringLiteral and fails the applyIN operation due to the string literal not being converted to a string map.

I could see two different fixes; 1.) Convert incoming string literals to SliceStringLiterals as a rule, but I wasn't convinced that that was the correct fix. The solution I ended up doing, (and am willing to submit as a PR), was to support the type StringLiteral in the method getMapString and to convert it into a string map in there, since it has the smallest surface area and doesn't require supporting a new expression type. This makes the applyIN method work and conversely makes the applyCONTAINS and applyNOTCONTAINS work as well.

If there is a better solution or a use case I'm misunderstanding, please let me know.

@zhouzhuojie
Copy link
Owner

How about using regex match for substring?
https://github.com/zhouzhuojie/conditions/blob/master/parser_test.go#L168-L170

CONTAINS are only used for slice literal.

@dseabolt
Copy link
Author

Hmm, that would work, what would be the best way to represent a string slice literal in the flagr UI though? It is there a way to do it without regex? My hope is pass segment definitions to a non-dev at some point and I was hoping to avoid regex as it's difficult to teach to product managers.

@zhouzhuojie
Copy link
Owner

zhouzhuojie commented Dec 23, 2019

So flagr UI reflects the syntax of this conditions package. For example:

property CONTAINS "foo"    =======> {"property": ["foo", "bar"]} -> True
                                    {"property": "foobar"}       -> False

property IN ["foo", "bar"] =======> {"property": ["foo", "bar"]} -> False
                                    {"property": "foo"}          -> True

property =~ "foo"          =======> {"property": ["foo", "bar"]} -> False
                                    {"property": "foobar"}       -> True

In terms regex complexity, I think you may still need some documentation and training for your non-dev team.

Example: email =~ "@gmail.com"
image

@dseabolt
Copy link
Author

Ah I didn't realize that's what that operator did, thanks. As far as regex is concerned, my concern is teaching regex to non devs would be difficult. Ideally the segment management isn't handled by the devs, but rather analysts and product managers.

Repository owner deleted a comment from zmh-program Feb 23, 2024
Repository owner deleted a comment from Nitesh639 Feb 26, 2024
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

No branches or pull requests

4 participants
@zhouzhuojie @dseabolt and others