Skip to content

Commit

Permalink
[ResponseOps][ES Query] Grouped over field is not populated correctly…
Browse files Browse the repository at this point in the history
… when editing a rule (elastic#192297)

Resolves elastic#191077

## Summary

This PR fixes a bug loading the `groupby` field when editing an ES query
rule. While the form was loading fields from the index, the `fields`
array was empty ([]). This wasn't caught by a truthy check, so
`termField` was set to undefined as the fields array didn’t contain the
selected field. As a result, the form failed to populate correctly. To
fix this, I updated it to only check if the array contains the selected
field when the array is defined and also not empty.

I tested this for creating and editing the rule and it seemed to work,
pls let me know if there is another scenario I am missing with this fix.


### To verify

- Create an ES query rule with the `groupby` field set. See example
below of what the bug looked like before.
- Edit the rule and verify that the grouped by field is populated
correctly. I would test it a couple times to verify

**Example of Before:**
<img width="895" alt="Screen Shot 2024-09-06 at 2 09 43 PM"
src="https://github.com/user-attachments/assets/a03cebb5-4cc5-4d26-9e07-11e0ed120aef">
  • Loading branch information
doakalexi authored and gergoabraham committed Sep 13, 2024
1 parent 7c5a6a3 commit 89d2ff1
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ describe('group by expression', () => {
expect(onChangeSelectedTermField).toHaveBeenCalledWith(['field1', 'field2']);
});

it('do NOT clears selected agg field if fields is undefined', async () => {
it('do NOT clear selected agg field if fields is undefined', async () => {
const onChangeSelectedTermField = jest.fn();
render(
<IntlProvider locale="en">
Expand All @@ -236,4 +236,22 @@ describe('group by expression', () => {
);
expect(onChangeSelectedTermField).toHaveBeenCalledTimes(0);
});

it('do NOT clear selected agg field if fields is an empty array', async () => {
const onChangeSelectedTermField = jest.fn();
render(
<IntlProvider locale="en">
<GroupByExpression
errors={{ termSize: [], termField: [] }}
fields={[]}
termField={['test', 'unknown']}
groupBy={'top'}
onChangeSelectedGroupBy={() => {}}
onChangeSelectedTermSize={() => {}}
onChangeSelectedTermField={onChangeSelectedTermField}
/>
</IntlProvider>
);
expect(onChangeSelectedTermField).toHaveBeenCalledTimes(0);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export const GroupByExpression = ({
}, [selectedTermsFieldsOptions, groupBy, onChangeSelectedTermField]);

useEffect(() => {
if (fields) {
if (fields && fields.length > 0) {
// if current field set doesn't contain selected field, clear selection
const hasUnknownField = selectedTermsFieldsOptions.some(
(fieldOption) => !fields.some((field) => field.name === fieldOption.label)
Expand Down

0 comments on commit 89d2ff1

Please sign in to comment.