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

[Security Solution] Adds tour for new upgrade flyout diff features #176767

Merged
merged 10 commits into from
Feb 14, 2024

Conversation

dplumlee
Copy link
Contributor

@dplumlee dplumlee commented Feb 12, 2024

Summary

Issue: #166489

Adds a tour and tooltips highlighting and describing the new diff features of the prebuilt rule update flyout.

To test:

Enable the jsonPrebuiltRulesDiffingEnabled and/or perFieldPrebuiltRulesDiffingEnabled feature flags and clear your browser of the local storage securitySolution.rulesManagementPage.newFeaturesTour.v8.13 token. This should allow you to see the tour and both new tabs highlighted by tooltips as shown in the screenshots below.

Screenshots

Upgrade tour
Screenshot 2024-02-13 at 2 14 07 PM

Diff tab tooltips
Screenshot 2024-02-13 at 2 14 22 PM

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@dplumlee dplumlee added release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area v8.13.0 labels Feb 12, 2024
@dplumlee dplumlee self-assigned this Feb 12, 2024
@dplumlee
Copy link
Contributor Author

buildkite test this

@dplumlee dplumlee marked this pull request as ready for review February 12, 2024 22:02
@dplumlee dplumlee requested review from a team as code owners February 12, 2024 22:02
@dplumlee dplumlee requested a review from banderror February 12, 2024 22:02
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@dplumlee dplumlee requested a review from joepeeples February 12, 2024 22:21
Comment on lines 87 to 111
{tourSteps.length > 1 && (
<>
<EuiSpacer size="s" />
<EuiFlexGroup responsive={false} gutterSize="s" alignItems="center">
<EuiFlexItem grow={false}>
<EuiButtonIcon
iconType="arrowLeft"
aria-label={'previous'}
display="empty"
disabled={index === 0}
onClick={tourActions.decrementStep}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonIcon
iconType="arrowRight"
aria-label={'next'}
display="base"
disabled={index === tourSteps.length - 1}
onClick={tourActions.incrementStep}
/>
</EuiFlexItem>
</EuiFlexGroup>
</>
)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the tour only has one step, you don't need all of this code. This can be simply:

  const enhancedSteps = useMemo(
    () =>
      tourSteps.map((item) => ({
        ...item,
        content: item.content,
      })),
    [tourSteps]
  );

Copy link
Contributor Author

@dplumlee dplumlee Feb 13, 2024

Choose a reason for hiding this comment

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

Since we are switching back to just modifying the existing rules_feature_tour.tsx file in a0a9587, I went ahead and left this multi-step logic in the file for a future use case if we do want to add another new feature tour and there are multiple steps.

@dplumlee dplumlee requested a review from jpdjere February 13, 2024 16:02
Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

Tested locally and reviewed the changes. ++ to @jpdjere're comments. Also, found a few more issues.

@dplumlee dplumlee requested a review from banderror February 13, 2024 17:03
Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

tab navigation changes LGTM

Copy link
Contributor

@joepeeples joepeeples left a comment

Choose a reason for hiding this comment

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

Added a couple suggestions, thanks! Feel free to ping again if you need another look.

Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

Re-tested and re-reviewed, the updated version looks great 👍 Thank you @dplumlee!

Comment on lines -77 to -98
const [shouldShowSearchCapabilitiesTour, setShouldShowSearchCapabilitiesTour] = useState(false);

useEffect(() => {
/**
* Wait until the tour target elements are visible on the page and mount
* EuiTourStep components only after that. Otherwise, the tours would never
* show up on the page.
*/
const observer = new MutationObserver(() => {
if (document.querySelector(`#${SEARCH_CAPABILITIES_TOUR_ANCHOR}`)) {
setShouldShowSearchCapabilitiesTour(true);
observer.disconnect();
}
});

observer.observe(document.body, {
childList: true,
subtree: true,
});

return () => observer.disconnect();
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

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

Replacement of this block with const isTourAnchorMounted = useIsElementMounted(PER_FIELD_UPGRADE_TOUR_ANCHOR); is very nice 👍

@dplumlee dplumlee enabled auto-merge (squash) February 13, 2024 23:55
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Investigations - Security Solution Cypress Tests #7 / Basic esql search and filter operations "before each" hook for "should remove the query when the back button is pressed after adding a query" "before each" hook for "should remove the query when the back button is pressed after adding a query"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 4973 4975 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 11.5MB 11.5MB +2.3KB

History

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

cc @dplumlee

@dplumlee dplumlee merged commit db513b7 into elastic:main Feb 14, 2024
36 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Feb 14, 2024
@dplumlee dplumlee deleted the per-fields-diffs-tour branch February 14, 2024 01:18
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…lastic#176767)

## Summary
Issue: elastic#166489

Adds a tour and tooltips highlighting and describing the new diff
features of the prebuilt rule update flyout.

#### To test: 
Enable the `jsonPrebuiltRulesDiffingEnabled` and/or
`perFieldPrebuiltRulesDiffingEnabled` feature flags and clear your
browser of the local storage
`securitySolution.rulesManagementPage.newFeaturesTour.v8.13` token. This
should allow you to see the tour and both new tabs highlighted by
tooltips as shown in the screenshots below.

### Screenshots
**Upgrade tour**
<img width="1406" alt="Screenshot 2024-02-13 at 2 14 07 PM"
src="https://github.com/elastic/kibana/assets/56367316/20d3fd8e-fc39-4ae2-a627-272ae14e9aac">

**Diff tab tooltips**
<img width="1176" alt="Screenshot 2024-02-13 at 2 14 22 PM"
src="https://github.com/elastic/kibana/assets/56367316/8810c17e-4dfb-4758-b4c7-199a44531a4e">



### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))



### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
…lastic#176767)

## Summary
Issue: elastic#166489

Adds a tour and tooltips highlighting and describing the new diff
features of the prebuilt rule update flyout.

#### To test: 
Enable the `jsonPrebuiltRulesDiffingEnabled` and/or
`perFieldPrebuiltRulesDiffingEnabled` feature flags and clear your
browser of the local storage
`securitySolution.rulesManagementPage.newFeaturesTour.v8.13` token. This
should allow you to see the tour and both new tabs highlighted by
tooltips as shown in the screenshots below.

### Screenshots
**Upgrade tour**
<img width="1406" alt="Screenshot 2024-02-13 at 2 14 07 PM"
src="https://github.com/elastic/kibana/assets/56367316/20d3fd8e-fc39-4ae2-a627-272ae14e9aac">

**Diff tab tooltips**
<img width="1176" alt="Screenshot 2024-02-13 at 2 14 22 PM"
src="https://github.com/elastic/kibana/assets/56367316/8810c17e-4dfb-4758-b4c7-199a44531a4e">



### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))



### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
dplumlee added a commit that referenced this pull request Oct 21, 2024
…rkflow (#196731)

## Summary

Removes the tour for rule features implemented in
#176767 as they are no longer new.
Keeps the component and logic to use for future tours, just removes
reference to the specific rules management tour.

### Steps to test

1. Make sure the
`securitySolution.rulesManagementPage.newFeaturesTour.v8.13` local
storage key is cleared from your kibana page (this will make the tour
show up normally)
2. Notice that navigating to the rule management page does not display
the rule tour

Co-authored-by: Elastic Machine <[email protected]>
dplumlee added a commit to dplumlee/kibana that referenced this pull request Oct 21, 2024
…rkflow (elastic#196731)

## Summary

Removes the tour for rule features implemented in
elastic#176767 as they are no longer new.
Keeps the component and logic to use for future tours, just removes
reference to the specific rules management tour.

### Steps to test

1. Make sure the
`securitySolution.rulesManagementPage.newFeaturesTour.v8.13` local
storage key is cleared from your kibana page (this will make the tour
show up normally)
2. Notice that navigating to the rule management page does not display
the rule tour

Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit 3547e15)

# Conflicts:
#	x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/pages/rule_management/index.tsx
dplumlee added a commit to dplumlee/kibana that referenced this pull request Oct 21, 2024
…rkflow (elastic#196731)

## Summary

Removes the tour for rule features implemented in
elastic#176767 as they are no longer new.
Keeps the component and logic to use for future tours, just removes
reference to the specific rules management tour.

### Steps to test

1. Make sure the
`securitySolution.rulesManagementPage.newFeaturesTour.v8.13` local
storage key is cleared from your kibana page (this will make the tour
show up normally)
2. Notice that navigating to the rule management page does not display
the rule tour

Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit 3547e15)

# Conflicts:
#	x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/pages/rule_management/index.tsx
dplumlee added a commit that referenced this pull request Oct 21, 2024
…ment workflow (#196731) (#197080)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[Security Solution] Removes outdated rule tour for rule management
workflow (#196731)](#196731)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Davis
Plumlee","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-21T14:33:26Z","message":"[Security
Solution] Removes outdated rule tour for rule management workflow
(#196731)\n\n## Summary\r\n\r\nRemoves the tour for rule features
implemented in\r\nhttps://github.com//pull/176767 as they
are no longer new.\r\nKeeps the component and logic to use for future
tours, just removes\r\nreference to the specific rules management
tour.\r\n\r\n### Steps to test\r\n\r\n1. Make sure
the\r\n`securitySolution.rulesManagementPage.newFeaturesTour.v8.13`
local\r\nstorage key is cleared from your kibana page (this will make
the tour\r\nshow up normally)\r\n2. Notice that navigating to the rule
management page does not display\r\nthe rule tour\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"3547e154b57adbdbf36abda5c4571452b4ee0b26","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Detections
and Resp","Team: SecuritySolution","Feature:Rule
Management","Team:Detection Rule
Management","v8.16.0","backport:version"],"number":196731,"url":"https://github.com/elastic/kibana/pull/196731","mergeCommit":{"message":"[Security
Solution] Removes outdated rule tour for rule management workflow
(#196731)\n\n## Summary\r\n\r\nRemoves the tour for rule features
implemented in\r\nhttps://github.com//pull/176767 as they
are no longer new.\r\nKeeps the component and logic to use for future
tours, just removes\r\nreference to the specific rules management
tour.\r\n\r\n### Steps to test\r\n\r\n1. Make sure
the\r\n`securitySolution.rulesManagementPage.newFeaturesTour.v8.13`
local\r\nstorage key is cleared from your kibana page (this will make
the tour\r\nshow up normally)\r\n2. Notice that navigating to the rule
management page does not display\r\nthe rule tour\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"3547e154b57adbdbf36abda5c4571452b4ee0b26"}},"sourceBranch":"main","suggestedTargetBranches":["8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196731","number":196731,"mergeCommit":{"message":"[Security
Solution] Removes outdated rule tour for rule management workflow
(#196731)\n\n## Summary\r\n\r\nRemoves the tour for rule features
implemented in\r\nhttps://github.com//pull/176767 as they
are no longer new.\r\nKeeps the component and logic to use for future
tours, just removes\r\nreference to the specific rules management
tour.\r\n\r\n### Steps to test\r\n\r\n1. Make sure
the\r\n`securitySolution.rulesManagementPage.newFeaturesTour.v8.13`
local\r\nstorage key is cleared from your kibana page (this will make
the tour\r\nshow up normally)\r\n2. Notice that navigating to the rule
management page does not display\r\nthe rule tour\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"3547e154b57adbdbf36abda5c4571452b4ee0b26"}},{"branch":"8.16","label":"v8.16.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
dplumlee added a commit that referenced this pull request Oct 21, 2024
…ent workflow (#196731) (#197086)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution] Removes outdated rule tour for rule management
workflow (#196731)](#196731)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Davis
Plumlee","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-21T14:33:26Z","message":"[Security
Solution] Removes outdated rule tour for rule management workflow
(#196731)\n\n## Summary\r\n\r\nRemoves the tour for rule features
implemented in\r\nhttps://github.com//pull/176767 as they
are no longer new.\r\nKeeps the component and logic to use for future
tours, just removes\r\nreference to the specific rules management
tour.\r\n\r\n### Steps to test\r\n\r\n1. Make sure
the\r\n`securitySolution.rulesManagementPage.newFeaturesTour.v8.13`
local\r\nstorage key is cleared from your kibana page (this will make
the tour\r\nshow up normally)\r\n2. Notice that navigating to the rule
management page does not display\r\nthe rule tour\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"3547e154b57adbdbf36abda5c4571452b4ee0b26","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Detections
and Resp","Team: SecuritySolution","Feature:Rule
Management","Team:Detection Rule
Management","v8.16.0","backport:version"],"number":196731,"url":"https://github.com/elastic/kibana/pull/196731","mergeCommit":{"message":"[Security
Solution] Removes outdated rule tour for rule management workflow
(#196731)\n\n## Summary\r\n\r\nRemoves the tour for rule features
implemented in\r\nhttps://github.com//pull/176767 as they
are no longer new.\r\nKeeps the component and logic to use for future
tours, just removes\r\nreference to the specific rules management
tour.\r\n\r\n### Steps to test\r\n\r\n1. Make sure
the\r\n`securitySolution.rulesManagementPage.newFeaturesTour.v8.13`
local\r\nstorage key is cleared from your kibana page (this will make
the tour\r\nshow up normally)\r\n2. Notice that navigating to the rule
management page does not display\r\nthe rule tour\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"3547e154b57adbdbf36abda5c4571452b4ee0b26"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196731","number":196731,"mergeCommit":{"message":"[Security
Solution] Removes outdated rule tour for rule management workflow
(#196731)\n\n## Summary\r\n\r\nRemoves the tour for rule features
implemented in\r\nhttps://github.com//pull/176767 as they
are no longer new.\r\nKeeps the component and logic to use for future
tours, just removes\r\nreference to the specific rules management
tour.\r\n\r\n### Steps to test\r\n\r\n1. Make sure
the\r\n`securitySolution.rulesManagementPage.newFeaturesTour.v8.13`
local\r\nstorage key is cleared from your kibana page (this will make
the tour\r\nshow up normally)\r\n2. Notice that navigating to the rule
management page does not display\r\nthe rule tour\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"3547e154b57adbdbf36abda5c4571452b4ee0b26"}},{"branch":"8.16","label":"v8.16.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/197080","number":197080,"state":"OPEN"}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area release_note:skip Skip the PR/issue when compiling release notes Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants