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.

Sign up for GitHub

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

Jump to bottom

[Security Solution] [Rules] Fixes bug where editing a rule with a data view throws an unhandled exception #145658

Merged
merged 6 commits into from
Nov 23, 2022
Merged

[Security Solution] [Rules] Fixes bug where editing a rule with a data view throws an unhandled exception #145658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9802e76
initial fix, need to determine why this fix was needed though
dhurley14 Nov 18, 2022
9ee05ae
Merge branch 'main' into fix-edit-rule-data-view
dhurley14 Nov 18, 2022
183fbd3
Merge branch 'main' into fix-edit-rule-data-view
dhurley14 Nov 22, 2022
295c411
display loader if waiting for kibanaDataViews promise to fulfill
dhurley14 Nov 22, 2022
6addd6b
adds cypress test to ensure the edit rules page doesn't break again
dhurley14 Nov 23, 2022
9f1b683
Merge branch 'main' into fix-edit-rule-data-view
dhurley14 Nov 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions ...k/plugins/security_solution/cypress/e2e/detection_rules/custom_query_rule_data_view.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ import {
RULE_SWITCH,
SEVERITY,
} from '../../screens/alerts_detection_rules';
import {
ABOUT_CONTINUE_BTN,
RULE_DESCRIPTION_INPUT,
RULE_NAME_INPUT,
} from '../../screens/create_new_rule';

import {
ADDITIONAL_LOOK_BACK_DETAILS,
Expand All @@ -44,13 +49,15 @@ import {
TAGS_DETAILS,
TIMELINE_TEMPLATE_DETAILS,
DATA_VIEW_DETAILS,
EDIT_RULE_SETTINGS_LINK,
} from '../../screens/rule_details';

import { goToRuleDetails } from '../../tasks/alerts_detection_rules';
import { createTimeline } from '../../tasks/api_calls/timelines';
import { postDataView } from '../../tasks/common';
import {
createAndEnableRule,
createRuleWithoutEnabling,
fillAboutRuleAndContinue,
fillDefineCustomRuleAndContinue,
fillScheduleRuleAndContinue,
Expand Down Expand Up @@ -158,5 +165,24 @@ describe('Custom query rules', () => {
.should('match', /^[1-9].+$/);
cy.get(ALERT_GRID_CELL).contains(this.rule.name);
});
it('Creates and edits a new rule with a data view', function () {
visit(RULE_CREATION);
fillDefineCustomRuleAndContinue(this.rule);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to this PR, but I've noticed that we access rules in our Cypress tests via this. And that leads to the type of this.rule being any. @dhurley14 Do you happen to know why we access rules using this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good question. It's not clear to me either. @MadameSheema?

cy.get(RULE_NAME_INPUT).clear({ force: true }).type(this.rule.name, { force: true });
cy.get(RULE_DESCRIPTION_INPUT)
.clear({ force: true })
.type(this.rule.description, { force: true });

cy.get(ABOUT_CONTINUE_BTN).should('exist').click({ force: true });

fillScheduleRuleAndContinue(this.rule);
createRuleWithoutEnabling();

goToRuleDetails();

cy.get(EDIT_RULE_SETTINGS_LINK).click({ force: true });

cy.get(RULE_NAME_HEADER).should('contain', 'Edit rule settings');
});
});
});
2 changes: 2 additions & 0 deletions x-pack/plugins/security_solution/cypress/screens/create_new_rule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ export const COMBO_BOX_CLEAR_BTN = '[data-test-subj="comboBoxClearButton"]';

export const CREATE_AND_ENABLE_BTN = '[data-test-subj="create-enable"]';

export const CREATE_WITHOUT_ENABLING_BTN = '[data-test-subj="create-enabled-false"]';

export const CUSTOM_QUERY_INPUT = '[data-test-subj="queryInput"]';

export const CUSTOM_QUERY_BAR = '[data-test-subj="detectionEngineStepDefineRuleQueryBar"]';
Expand Down
8 changes: 8 additions & 0 deletions x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ import {
NEW_TERMS_INPUT_AREA,
ACTIONS_THROTTLE_INPUT,
CONTINUE_BUTTON,
CREATE_WITHOUT_ENABLING_BTN,
} from '../screens/create_new_rule';
import {
INDEX_SELECTOR,
Expand All @@ -125,6 +126,13 @@ export const createAndEnableRule = () => {
cy.get(BACK_TO_ALL_RULES_LINK).should('not.exist');
};

export const createRuleWithoutEnabling = () => {
cy.get(CREATE_WITHOUT_ENABLING_BTN).click({ force: true });
cy.get(CREATE_WITHOUT_ENABLING_BTN).should('not.exist');
cy.get(BACK_TO_ALL_RULES_LINK).click({ force: true });
cy.get(BACK_TO_ALL_RULES_LINK).should('not.exist');
};

export const fillAboutRule = (
rule: CustomRule | MachineLearningRule | ThresholdRule | ThreatIndicatorRule
) => {
Expand Down
5 changes: 4 additions & 1 deletion ...k/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
EuiFlexGroup,
EuiFlexItem,
EuiFormRow,
EuiLoadingSpinner,
EuiSpacer,
EuiButtonGroup,
EuiText,
Expand Down Expand Up @@ -502,7 +503,9 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
);

const DataViewSelectorMemo = useMemo(() => {
return (
return kibanaDataViews == null || Object.keys(kibanaDataViews).length === 0 ? (
<EuiLoadingSpinner size="l" />
) : (
Comment on lines +506 to +508
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unsure why the kibanaDataViews passed down to the edit page started breaking the edit page this release compared to last release. Either way, this will fix the issue going forward.

<UseField
key="DataViewSelector"
path="dataViewId"
Expand Down
1 change: 1 addition & 0 deletions .../plugins/security_solution/public/detections/components/rules/step_rule_actions/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ const StepRuleActionsComponent: FC<StepRuleActionsProps> = ({
isDisabled={isLoading}
isLoading={isLoading}
onClick={() => handleSubmit(false)}
data-test-subj="create-enabled-false"
>
{I18n.COMPLETE_WITHOUT_ENABLING}
</EuiButton>
Expand Down