add logic to save extractor keys as array if string #1611
Conversation
There was a problem hiding this comment.
I'm not convinced we need to save extractor keys as an array/convert into a string.
I think we could get away with something like:
begin
filters['extractor_keys'] = JSON.parse(filters['extractor_keys'])
rescue JSON::ParserError, TypeError;
end
But happy to be wrong on this.
|
|
||
| def convert_to_array(input) | ||
| if input.strip.start_with?("[") && input.strip.end_with?("]") | ||
| json_string = input.gsub("'", '"') |
There was a problem hiding this comment.
I think this could line lead to weird behavior. As an eg. someone enters a single quote within a double quote within an array eg inputting ["'hello'"] . I think as you edit the reducer, this logic starts getting wonky.
Granted a person shouldnt be doing something like that anyway cuz I'm assuming the reducer won't run well 😆 , but technically via caesar ui is still a "valid" input.
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
I agree - looking again, the convert_to_array logic is an overkill. The values save as is when the exception is caught so your suggestion would fly. Pushing this update in a bit |
| @@ -96,6 +96,16 @@ | |||
| expect(workflow.reducers.first.filters['extractor_keys']).to eq(['test']) | |||
| end | |||
|
|
|||
| it 'saves extractor_keys as an array' do | |||
| nested_reducer_params[:extractor_keys] = 'test' | |||
There was a problem hiding this comment.
Thats weird. I would have expected if we set extractor keys input to a string that it would keep as string. Maybe you mean nested_reducer_params[:filters][:extractor_keys] ?
There was a problem hiding this comment.
Hey Toyosi! Some quick changes in the test and then you should be good.
I think your save extractor_keys as an array might not be tested correct behavior. I think you might need to update L100 to
nested_reducer_params[:filters][:extractor_keys] = 'test' .
Maybe nitpicky but I would also test the the case of '"test"'. (This is the case for the current behavior on caesar. Entering "test" on the input field actually sends nested_reducer_params[:filters][:extractor_keys] as '"test"' which is JSON parseable.)
Solves for this ticket