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

Adjust parsing of filter strings #2051

Merged
merged 5 commits into from
Mar 24, 2020

Conversation

sarahd93
Copy link
Contributor

@sarahd93 sarahd93 commented Mar 19, 2020

Parsing filter strings that include substrings in double quotes is causing issues. Adjust parsing functions to be able to handle double quotes.

Checklist:

@codecov
Copy link

codecov bot commented Mar 19, 2020

Codecov Report

Merging #2051 into gsa-9.0 will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           gsa-9.0    #2051      +/-   ##
===========================================
+ Coverage    50.24%   50.26%   +0.01%     
===========================================
  Files         1070     1070              
  Lines        25141    25149       +8     
  Branches      7133     7108      -25     
===========================================
+ Hits         12633    12641       +8     
  Misses       11366    11366              
  Partials      1142     1142              
Impacted Files Coverage Δ
gsa/src/gmp/models/filter.js 100.00% <100.00%> (ø)
gsa/src/gmp/models/filter/filterterm.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6b0b9e...c0e779f. Read the comment docs.

Copy link
Contributor

@bjoernricks bjoernricks left a comment

Choose a reason for hiding this comment

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

Nice! I would have written this code differently. Could you add some tests for the
foo="lorem ipsum" test case. Just test parsing of combinations like ~"foo bar", foo="lorem ipsum" etc.

Btw. I think "foo bar"=def will be parsed to at the moment. Not sure if we want to avoid that.

@sarahd93
Copy link
Contributor Author

I already started writing tests :)

You are right about "foo bar"=def, I am going to check again if I can catch some more of those edge cases.

@sarahd93 sarahd93 marked this pull request as ready for review March 24, 2020 12:50
@sarahd93 sarahd93 requested a review from a team March 24, 2020 12:50
@sarahd93 sarahd93 merged commit 1c6a6b1 into greenbone:gsa-9.0 Mar 24, 2020
@sarahd93 sarahd93 deleted the parse_filterstrings_gsa9 branch March 24, 2020 13:16
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.

2 participants