Query Editor: UI improvements and Lucene query type selection improvements #583
Conversation
idastambuk requested review from kevinwcyu and njvrzm and removed request for a team 
| | ||
| export const UPDATE_LUCENE_TYPE_AND_METRICS = 'update_lucene_type_and_metrics'; | ||
| | ||
| export const updateLuceneTypeAndMetrics = createAction<{ |
There was a problem hiding this comment.
updateLuceneTypeAndMetrics updates two reducers (this one and metricsReducer) at the same time,.
This is because, if I triggered onChange(query) from the component and then changeMetricType, one would overwrite the other (changeMetricType calls onChange in the background)
iwysiu left a comment
iwysiu left a comment There was a problem hiding this comment.
A couple style nits, but I tried it and it looks good!
| </> | ||
| {props.query.luceneQueryType === LuceneQueryType.Traces && ( | ||
| <EditorRow> | ||
| <InlineField label="Service Map" tooltip={'Request and display service map data for trace(s)'}> |
There was a problem hiding this comment.
Probably could be a separate pr, but we could probably update these to match the other query type options more
There was a problem hiding this comment.
Makes sense, did it in this one!
| )} | ||
| </> | ||
| )} | ||
| {metric.type === 'logs' && ( |
There was a problem hiding this comment.
nit: If this is the same as the size field above would it make sense to have the condition for that be (metric.type === 'raw_data' || metric.type === 'raw_document' || metric.type === 'logs') instead repeating it again?
| expect(screen.queryByLabelText('Alias')).toBeNull(); | ||
| }); | ||
| | ||
| it('Should be shown if last bucket aggregation is Date Histogram', () => { |
There was a problem hiding this comment.
Should this test have more than 1 bucket arg?
There was a problem hiding this comment.
Good catch, fixed!
| return getNewMetrics(state || [], action.payload.id, action.payload.type); | ||
| } | ||
| if (updateLuceneTypeAndMetrics.match(action)) { | ||
| return action.payload.metrics; |
There was a problem hiding this comment.
nit: from a code organization perspective, it feels a little weird that changeMetricType calls getNewMetrics here and updateLuceneTypeAndMetrics calls it in the LuceneQueryTypeSelector when it dispatches the action
There was a problem hiding this comment.
Updated to send metric type and id instead of metrics, so just reducer will call getNewMetrics
2 participants
What this PR does / why we need it:
Our current editor hides Logs, Raw Data and Raw Document under Metrics dropdown, which isn't very intuitive. This refactor the editor to add a few new things:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Testing: