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

[SIEM] [Detection Engine] Update status on rule details page #55201

Merged
merged 12 commits into from
Jan 18, 2020

Conversation

dhurley14
Copy link
Contributor

@dhurley14 dhurley14 commented Jan 17, 2020

Summary

Fixes duplicated most recent failure appearing on failures tab when less than five failures present. Updates the status on rule details page automatically when user clicks enabled / disabled switch. Also adds refresh button to get new status on demand.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@dhurley14 dhurley14 added the release_note:skip Skip the PR/issue when compiling release notes label Jan 17, 2020
search: rule.id,
searchFields: ['alertId'],
});
}
Copy link
Contributor

@FrankHassanabad FrankHassanabad Jan 17, 2020

Choose a reason for hiding this comment

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

nit:

Edit, wrong operator, sorry, deleted that and added this:

  const ruleCurrentStatus =
    savedObjectsClient != null
      ? await savedObjectsClient.find<IRuleSavedAttributesSavedObjectAttributes>({
          type: ruleStatusSavedObjectType,
          perPage: 1,
          sortField: 'statusDate',
          sortOrder: 'desc',
          search: rule.id,
          searchFields: ['alertId'],
        })
      : null;

This will remove the let

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh nice thank you. I'll update that.

Copy link
Contributor

@FrankHassanabad FrankHassanabad Jan 17, 2020

Choose a reason for hiding this comment

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

Per conversations:

Make it mandatory in TypeScript and then add it everywhere else and remove the null check altogether

? 'danger'
: status === 'executing' || status === 'going to run'
? 'warning'
: 'subdued';
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This would really read much cleaner with a case statement, some switch statements , and an assert never through TypeScript.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I'll update this. Gracias.

Copy link
Contributor

@FrankHassanabad FrankHassanabad left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -42,7 +42,7 @@ export const usePrivilegeUser = (): Return => {
});

if (isSubscribed && privilege != null) {
setAuthenticated(privilege.isAuthenticated);
setAuthenticated(privilege.is_authenticated);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Thank you 👍

…ager exits because of api key error it won't write the error status so the actual status is 'going to run' on the next interval. This is more accurate than being stuck on 'executing' because of an error we don't control and can't write a status for.
@dhurley14 dhurley14 force-pushed the fix-status-on-update branch from 1378107 to 45971da Compare January 18, 2020 02:12
@dhurley14 dhurley14 requested a review from a team as a code owner January 18, 2020 13:10
@XavierM XavierM force-pushed the fix-status-on-update branch from 410ea92 to b76b5eb Compare January 18, 2020 16:22
@XavierM XavierM removed the request for review from a team January 18, 2020 16:22
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

dhurley14 added a commit to dhurley14/kibana that referenced this pull request Jan 18, 2020
…#55201)

* adds logic for returning / updating status when a rule is switched from enabled to disabled and vice versa.

* update response for find rules statuses to include current status and failures

* update status on demand and on enable/disable

* adds ternary to allow removal of 'let'

* adds savedObjectsClient to the add and upate prepackaged rules and import rules route.

* fix bug where convertToSnakeCase would throw error 'cannot convert null or undefined to object' if passed null

* genericize snake_case converter and updates isAuthorized to snake_case (different situation)

* renaming to 'going to run' instead of executing because when task manager exits because of api key error it won't write the error status so the actual status is 'going to run' on the next interval. This is more accurate than being stuck on 'executing' because of an error we don't control and can't write a status for.

* fix missed merge conflict

Co-authored-by: Xavier Mouligneau <189600+XavierM@users.noreply.github.com>
dhurley14 added a commit to dhurley14/kibana that referenced this pull request Jan 18, 2020
…#55201)

* adds logic for returning / updating status when a rule is switched from enabled to disabled and vice versa.

* update response for find rules statuses to include current status and failures

* update status on demand and on enable/disable

* adds ternary to allow removal of 'let'

* adds savedObjectsClient to the add and upate prepackaged rules and import rules route.

* fix bug where convertToSnakeCase would throw error 'cannot convert null or undefined to object' if passed null

* genericize snake_case converter and updates isAuthorized to snake_case (different situation)

* renaming to 'going to run' instead of executing because when task manager exits because of api key error it won't write the error status so the actual status is 'going to run' on the next interval. This is more accurate than being stuck on 'executing' because of an error we don't control and can't write a status for.

* fix missed merge conflict

Co-authored-by: Xavier Mouligneau <189600+XavierM@users.noreply.github.com>
dhurley14 added a commit that referenced this pull request Jan 18, 2020
…#55277)

* adds logic for returning / updating status when a rule is switched from enabled to disabled and vice versa.

* update response for find rules statuses to include current status and failures

* update status on demand and on enable/disable

* adds ternary to allow removal of 'let'

* adds savedObjectsClient to the add and upate prepackaged rules and import rules route.

* fix bug where convertToSnakeCase would throw error 'cannot convert null or undefined to object' if passed null

* genericize snake_case converter and updates isAuthorized to snake_case (different situation)

* renaming to 'going to run' instead of executing because when task manager exits because of api key error it won't write the error status so the actual status is 'going to run' on the next interval. This is more accurate than being stuck on 'executing' because of an error we don't control and can't write a status for.

* fix missed merge conflict

Co-authored-by: Xavier Mouligneau <189600+XavierM@users.noreply.github.com>

Co-authored-by: Xavier Mouligneau <189600+XavierM@users.noreply.github.com>
dhurley14 added a commit that referenced this pull request Jan 18, 2020
…#55276)

* adds logic for returning / updating status when a rule is switched from enabled to disabled and vice versa.

* update response for find rules statuses to include current status and failures

* update status on demand and on enable/disable

* adds ternary to allow removal of 'let'

* adds savedObjectsClient to the add and upate prepackaged rules and import rules route.

* fix bug where convertToSnakeCase would throw error 'cannot convert null or undefined to object' if passed null

* genericize snake_case converter and updates isAuthorized to snake_case (different situation)

* renaming to 'going to run' instead of executing because when task manager exits because of api key error it won't write the error status so the actual status is 'going to run' on the next interval. This is more accurate than being stuck on 'executing' because of an error we don't control and can't write a status for.

* fix missed merge conflict

Co-authored-by: Xavier Mouligneau <189600+XavierM@users.noreply.github.com>

Co-authored-by: Xavier Mouligneau <189600+XavierM@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 20, 2020
* upstream/master: (24 commits)
  Show error page when accessing unavailable app (elastic#54656)
  [ML] Improving job wizards with datafeed aggregations (elastic#55180)
  remove flaly assetion. a license presence tested anyway (elastic#55289)
  fix commonly used ranges uptime (elastic#54930)
  [SIEM] Use proper icons on Detections view (elastic#55215)
  Fix: invalid translation referenced (elastic#54901)
  [State Management] Remove AppState from edit_index_pattern page (elastic#54104)
  Implements `getStartServices` on server-side (elastic#55156)
  Move vis_vega_type/data_model tests to jest (elastic#55186)
  [SIEM] [Detection Engine] Update status on rule details page (elastic#55201)
  Fix KQL value suggestions for nested fields (elastic#54820)
  Enforce camelCase format for a plugin id (elastic#53759)
  [SIEM] Detection engine cleanup for rule details/creation/edit page (elastic#55069)
  Remove nested root from index pattern (elastic#54978)
  [Reporting/Migration] ReportingSetup, LegacySetup (elastic#54198)
  [SIEM] [Detection Engine] Fixes duplicate rule action (elastic#55252)
  [SIEM] Detections add alert & signal tab (elastic#55127)
  Management API - redirect on disabled app path (elastic#55136)
  [SIEM][Detection Engine] Fixes critical regression on the backend with immutable and tags
  update local (elastic#55177)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 20, 2020
* master: (108 commits)
  [ML] Single Metric Viewer: Fix job check. (elastic#55191)
  Show error page when accessing unavailable app (elastic#54656)
  [ML] Improving job wizards with datafeed aggregations (elastic#55180)
  remove flaly assetion. a license presence tested anyway (elastic#55289)
  fix commonly used ranges uptime (elastic#54930)
  [SIEM] Use proper icons on Detections view (elastic#55215)
  Fix: invalid translation referenced (elastic#54901)
  [State Management] Remove AppState from edit_index_pattern page (elastic#54104)
  Implements `getStartServices` on server-side (elastic#55156)
  Move vis_vega_type/data_model tests to jest (elastic#55186)
  [SIEM] [Detection Engine] Update status on rule details page (elastic#55201)
  Fix KQL value suggestions for nested fields (elastic#54820)
  Enforce camelCase format for a plugin id (elastic#53759)
  [SIEM] Detection engine cleanup for rule details/creation/edit page (elastic#55069)
  Remove nested root from index pattern (elastic#54978)
  [Reporting/Migration] ReportingSetup, LegacySetup (elastic#54198)
  [SIEM] [Detection Engine] Fixes duplicate rule action (elastic#55252)
  [SIEM] Detections add alert & signal tab (elastic#55127)
  Management API - redirect on disabled app path (elastic#55136)
  [SIEM][Detection Engine] Fixes critical regression on the backend with immutable and tags
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:SIEM v7.6.0 v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants