[Automatic Migrations] Migrations page with nav#260379
[Automatic Migrations] Migrations page with nav#260379kqualters-elastic merged 12 commits intoelastic:mainfrom
Conversation
kqualters-elastic
added
backport:skip
kqualters-elastic
force-pushed
the
migrations-page-with-nav
branch
from
4a82562 to
b2467c2
Compare
| @@ -34,6 +36,18 @@ const SiemMigrationsLandingRoutes = () => { | |||
| ); | |||
| }; | |||
|
|
|||
| const SiemMigrationsCreateRoutes = () => { | |||
| return ( | |||
| <PluginTemplateWrapper> | |||
| <SecurityRoutePageWrapper pageName={SecurityPageName.siemMigrationsCreate}> | |||
| <Routes> | |||
| <Route path={'*'} component={SiemMigrationsCreatePage} /> | |||
| </Routes> | |||
| </SecurityRoutePageWrapper> | |||
| </PluginTemplateWrapper> | |||
| ); | |||
| }; | |||
|
|
|||
There was a problem hiding this comment.
I would suggest rename siemMigrationsCreate back to siemMigrationsLanding.
Also, do we need to siem_migrations/create route, since we already have /siem_migrationsas landing route.
Please correct me if i am missing something.
There was a problem hiding this comment.
The reason I did that is because landing lives at /siem_migrations/landing, and having a card (typically pointing to a child route), there that links to the parent route of /siem_migrations/ seemed odd/could cause issues in the future with nav/breadcrumbs etc. I can change if you'd like.
| siemMigrationsLanding = 'siem_migrations', | ||
| siemMigrationsRules = 'siem_migrations-rules', | ||
| siemMigrationsDashboards = 'siem_migrations-dashboards', | ||
| siemMigrationsCreate = 'siem_migrations-create', |
There was a problem hiding this comment.
Refering to this comment. Why do we need this new page/link?
There was a problem hiding this comment.
We don't need it, as all of this is arbitrary, I just was thinking that a page of link cards, at migrations/landing, should not link to it's "parent" route at migrations/, as this doesn't make a whole lot of sense from an information hierarchy perspective, can cause issues with hypothetical future breadcrumb or other nav changes, etc, but it's all arbitrary and I can change it if you'd like
|
Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations) |
kqualters-elastic
changed the title
kqualters-elastic
added
the
release_note:skip
logeekal
added
reviewer:coderabbit
There was a problem hiding this comment.
I did Desk Testing and here are some issues:
Links
-
I am not sure of the difference between
/app/security/siem_migrations&/app/security/siem_migrations/landing. imolandingis redundant -
/siem_migrations/createpage makes sense but in url imostartormanagemay fit better since we can also see the results of the migrations, delete them as well, etc.
Redirection.
-
If I open kibana with URL
/app/security/siem_migrations, they are redirected to/app/get_started#migration_rules. -
From Translated Rules Page, when user clicks on
Add another Migration, they are redirected to/siem_migrations/create, it should besiem_migration/create#migrate_rule.
Demo
Screen.Recording.2026-04-01.at.12.58.14.mov
-
Same as point 2 for dashboards.
-
Nit:Start Automatic Migrationshould also redirect tosiem_migrations/create#migration_rules.
UI
-
Cards are of full width on new pages while they are of proper readability with on original Get Started Page. I think that is better.
-
Too much gap below
Automatic Migrationsheader ( screenshot below ). It will be great add a divider like other pages such asTranslated Rules.
| Automatic Migrations | Translated Rules |
|---|---|
 |
 |
| <> | ||
| <OnboardingHeader /> | ||
| <OnboardingBody /> | ||
| <OnboardingBody topicId={OnboardingTopicId.default} /> |
There was a problem hiding this comment.
Previously Onboarding body was implementing 2 tabs ( based on topics ). Is that supported with this change. It seems now there is not ability to add more topics to the Get started Page.
| <SecuritySolutionPageWrapper> | ||
| <Title title={SIEM_MIGRATIONS_PAGE_TITLE} /> | ||
| <EuiSpacer size="xl" /> | ||
| <OnboardingBody topicId={OnboardingTopicId.siemMigrations} /> |
There was a problem hiding this comment.
it seems concept of topic changed from being a tab to a separate page. I think this breaks Onboarding.
Do you think we can reuse Onboarding Config by passing topic Map. Get Started page can then manage its own topics with interference from other pages.
|
Additionally, It seems navigation is also not working in Solution view. Screen.Recording.2026-04-01.at.13.43.10.mov |
📝 WalkthroughWalkthroughThis pull request restructures the onboarding system by removing header card components (DemoCard, TeammatesCard, VideoCard) and their associated translations across multiple languages, alongside their styling and tests. The ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/cards/common/integrations/callouts/integration_card_top_callout.tsx (1)
37-44:⚠️ Potential issue | 🟡 Minor
MigrationsCalloutrendered unconditionally but gated by unrelated early return.The early return (Line 37-39) doesn't account for
MigrationsCallout. IfshowAgentlessCallout,showEndpointCallout, andshowActiveCalloutare all false, the component returnsnull—MigrationsCalloutnever renders.If
MigrationsCalloutshould always appear, update the early return:- if (!showAgentlessCallout && !showEndpointCallout && !showActiveCallout) { + const showMigrationsCallout = true; // or actual condition + if (!showAgentlessCallout && !showEndpointCallout && !showActiveCallout && !showMigrationsCallout) { return null; }If this is intentional (show migrations callout only alongside other callouts), please confirm.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/cards/common/integrations/callouts/integration_card_top_callout.tsx` around lines 37 - 44, The early return currently checks showAgentlessCallout, showEndpointCallout, and showActiveCallout and returns null, which prevents MigrationsCallout from ever rendering; update the conditional logic in the component that returns early (the block surrounding showAgentlessCallout, showEndpointCallout, showActiveCallout) so that MigrationsCallout is rendered unconditionally (or adjust the condition to include a showMigrationsCallout flag) — specifically, modify the early-return condition used before rendering MigrationsCallout/EuiSpacer or add an explicit check for MigrationsCallout so MigrationsCallout is not skipped when the other three flags are false.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_body_config.test.ts`:
- Around line 40-43: The test in use_body_config.test.ts asserts the hook result
against the wrong variable; update the expectation to compare result.current to
the config entry used by the map (defaultBodyConfig) instead of bodyConfig.
Locate the test that calls renderHook(() =>
useBodyConfig(OnboardingTopicId.default)) and replace the expected value to
defaultBodyConfig so the assertion matches the actual config returned by
useBodyConfig.
In
`@x-pack/solutions/security/plugins/security_solution/public/siem_migrations/links.ts`:
- Around line 27-40: The new nav link object for
SecurityPageName.siemMigrationsCreate is missing the hideWhenExperimentalKey
used by other SIEM migrations links; add hideWhenExperimentalKey:
'siemMigrationsDisabled' to the link definition (the same key used on the other
sub-links) so the link is hidden when the feature flag for SIEM migrations is
disabled; update the object that contains id:
SecurityPageName.siemMigrationsCreate, title, path: SIEM_MIGRATIONS_CREATE_PATH
and capabilities referencing SIEM_MIGRATIONS_FEATURE_ID to include
hideWhenExperimentalKey.
---
Outside diff comments:
In
`@x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/cards/common/integrations/callouts/integration_card_top_callout.tsx`:
- Around line 37-44: The early return currently checks showAgentlessCallout,
showEndpointCallout, and showActiveCallout and returns null, which prevents
MigrationsCallout from ever rendering; update the conditional logic in the
component that returns early (the block surrounding showAgentlessCallout,
showEndpointCallout, showActiveCallout) so that MigrationsCallout is rendered
unconditionally (or adjust the condition to include a showMigrationsCallout
flag) — specifically, modify the early-return condition used before rendering
MigrationsCallout/EuiSpacer or add an explicit check for MigrationsCallout so
MigrationsCallout is not skipped when the other three flags are false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 7b0bc200-33d6-41db-99d4-fff0790ba7ce
⛔ Files ignored due to path filters (8)
x-pack/solutions/security/plugins/security_solution/public/common/icons/agent/icon.svgis excluded by!**/*.svgx-pack/solutions/security/plugins/security_solution/public/common/icons/agent/icon_dark.svgis excluded by!**/*.svgx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/images/demo_card.pngis excluded by!**/*.pngx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/images/demo_card_dark.pngis excluded by!**/*.pngx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/images/teammates_card.pngis excluded by!**/*.pngx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/images/teammates_card_dark.pngis excluded by!**/*.pngx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/images/video_card.pngis excluded by!**/*.pngx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/images/video_card_dark.pngis excluded by!**/*.png
📒 Files selected for processing (35)
src/platform/packages/shared/deeplinks/security/deep_links.tsx-pack/platform/plugins/private/translations/translations/de-DE.jsonx-pack/platform/plugins/private/translations/translations/fr-FR.jsonx-pack/platform/plugins/private/translations/translations/ja-JP.jsonx-pack/platform/plugins/private/translations/translations/zh-CN.jsonx-pack/solutions/security/plugins/security_solution/common/constants.tsx-pack/solutions/security/plugins/security_solution/public/common/icons/agent/index.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/body_config.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/cards/common/integrations/callouts/integration_card_top_callout.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/cards/common/integrations/callouts/migrations_callout.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_body_config.test.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_body_config.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/onboarding_body.test.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/onboarding_body.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/common/link_card.styles.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/common/link_card.test.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/common/link_card.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/demo_card.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/index.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/translations.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/index.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/teammates_card.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/translations.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/index.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/translations.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/video_card.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/video_modal.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/onboarding_header.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/onboarding_header_topic_selector.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_router.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/constants.tsx-pack/solutions/security/plugins/security_solution/public/siem_migrations/create.tsxx-pack/solutions/security/plugins/security_solution/public/siem_migrations/links.tsx-pack/solutions/security/plugins/security_solution/public/siem_migrations/routes.tsxx-pack/solutions/security/plugins/security_solution_serverless/public/navigation/navigation_tree.ts
💤 Files with no reviewable changes (19)
- x-pack/platform/plugins/private/translations/translations/de-DE.json
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/index.ts
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/index.ts
- x-pack/platform/plugins/private/translations/translations/zh-CN.json
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/common/link_card.styles.ts
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/demo_card.tsx
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/translations.ts
- x-pack/platform/plugins/private/translations/translations/fr-FR.json
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/onboarding_header.tsx
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/common/link_card.test.tsx
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/translations.ts
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/index.ts
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/video_card.tsx
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/onboarding_header_topic_selector.tsx
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/translations.ts
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/teammates_card.tsx
- x-pack/platform/plugins/private/translations/translations/ja-JP.json
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/common/link_card.tsx
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/video_modal.tsx
| it('should return the body config for the selected topic', () => { | ||
| const { result } = renderHook(() => useBodyConfig()); | ||
| const { result } = renderHook(() => useBodyConfig(OnboardingTopicId.default)); | ||
| expect(result.current).toEqual(bodyConfig); | ||
| }); |
There was a problem hiding this comment.
Test assertion uses wrong variable.
The config map contains defaultBodyConfig, but the assertion compares against bodyConfig. This will cause the test to fail.
it('should return the body config for the selected topic', () => {
const { result } = renderHook(() => useBodyConfig(OnboardingTopicId.default));
- expect(result.current).toEqual(bodyConfig);
+ expect(result.current).toEqual(defaultBodyConfig);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should return the body config for the selected topic', () => { | |
| const { result } = renderHook(() => useBodyConfig()); | |
| const { result } = renderHook(() => useBodyConfig(OnboardingTopicId.default)); | |
| expect(result.current).toEqual(bodyConfig); | |
| }); | |
| it('should return the body config for the selected topic', () => { | |
| const { result } = renderHook(() => useBodyConfig(OnboardingTopicId.default)); | |
| expect(result.current).toEqual(defaultBodyConfig); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_body_config.test.ts`
around lines 40 - 43, The test in use_body_config.test.ts asserts the hook
result against the wrong variable; update the expectation to compare
result.current to the config entry used by the map (defaultBodyConfig) instead
of bodyConfig. Locate the test that calls renderHook(() =>
useBodyConfig(OnboardingTopicId.default)) and replace the expected value to
defaultBodyConfig so the assertion matches the actual config returned by
useBodyConfig.
| { | ||
| id: SecurityPageName.siemMigrationsCreate, | ||
| title: i18n.translate('xpack.securitySolution.appLinks.automaticMigrations.title', { | ||
| defaultMessage: 'Automatic migrations', | ||
| }), | ||
| description: i18n.translate('xpack.securitySolution.appLinks.automaticMigrations.description', { | ||
| defaultMessage: 'Migrate your SIEM to Elastic using AI powered Automatic migration.', | ||
| }), | ||
| landingIcon: IconAgent, | ||
| path: SIEM_MIGRATIONS_CREATE_PATH, | ||
| capabilities: [[SECURITY_UI_SHOW_PRIVILEGE, `${SIEM_MIGRATIONS_FEATURE_ID}.all`]], | ||
| skipUrlState: true, | ||
| hideTimeline: true, | ||
| }, |
There was a problem hiding this comment.
Missing hideWhenExperimentalKey may expose link when feature is disabled.
Other SIEM migrations sub-links include hideWhenExperimentalKey: 'siemMigrationsDisabled' (lines 54, 72), but this new link doesn't. When the feature flag is disabled, the route won't exist but the nav link may still appear.
path: SIEM_MIGRATIONS_CREATE_PATH,
capabilities: [[SECURITY_UI_SHOW_PRIVILEGE, `${SIEM_MIGRATIONS_FEATURE_ID}.all`]],
skipUrlState: true,
hideTimeline: true,
+ hideWhenExperimentalKey: 'siemMigrationsDisabled',
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@x-pack/solutions/security/plugins/security_solution/public/siem_migrations/links.ts`
around lines 27 - 40, The new nav link object for
SecurityPageName.siemMigrationsCreate is missing the hideWhenExperimentalKey
used by other SIEM migrations links; add hideWhenExperimentalKey:
'siemMigrationsDisabled' to the link definition (the same key used on the other
sub-links) so the link is hidden when the feature flag for SIEM migrations is
disabled; update the object that contains id:
SecurityPageName.siemMigrationsCreate, title, path: SIEM_MIGRATIONS_CREATE_PATH
and capabilities referencing SIEM_MIGRATIONS_FEATURE_ID to include
hideWhenExperimentalKey.
📝 WalkthroughWalkthroughThis change introduces a new SIEM migrations create page ( ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/cards/common/integrations/callouts/integration_card_top_callout.tsx (1)
37-39:⚠️ Potential issue | 🟠 MajorMigrations callout is blocked by the existing early return
On Line 37, the
return nullpath still triggers when the three legacy callouts are false, so the newMigrationsCallout(Line 43) never renders in that state.Suggested fix
- if (!showAgentlessCallout && !showEndpointCallout && !showActiveCallout) { - return null; - } + const showAnyLegacyCallout = showAgentlessCallout || showEndpointCallout || showActiveCallout; return ( <> <MigrationsCallout /> - <EuiSpacer size="s" /> + {showAnyLegacyCallout && <EuiSpacer size="s" />} {showEndpointCallout && <EndpointCallout />} {showAgentlessCallout && <AgentlessAvailableCallout />} {showActiveCallout && ( <ActiveIntegrationsCallout isAgentRequired={isAgentRequired} activeIntegrationsCount={activeIntegrationsCount} /> )} </> );Also applies to: 43-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/cards/common/integrations/callouts/integration_card_top_callout.tsx` around lines 37 - 39, The early return currently checks only showAgentlessCallout, showEndpointCallout, and showActiveCallout so MigrationsCallout never renders; update the conditional that returns null to also consider the migrations flag (e.g., include showMigrationsCallout) or compute/inline the same predicate used to decide rendering of MigrationsCallout and include it (so change "if (!showAgentlessCallout && !showEndpointCallout && !showActiveCallout) return null" to also check the migrations condition), or alternatively move the MigrationsCallout render above the early return; refer to the showAgentlessCallout, showEndpointCallout, showActiveCallout, and MigrationsCallout symbols to locate the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_body_config.test.ts`:
- Around line 40-43: The test for useBodyConfig is asserting the wrong expected
value: when calling useBodyConfig(OnboardingTopicId.default) the hook returns
the mapped config defaultBodyConfig (mapped under OnboardingTopicId.default) but
the test asserts against bodyConfig; update the assertion to expect
defaultBodyConfig instead of bodyConfig so the titles match (refer to
useBodyConfig, OnboardingTopicId.default, bodyConfig, and defaultBodyConfig).
In
`@x-pack/solutions/security/plugins/security_solution/public/siem_migrations/links.ts`:
- Around line 29-31: The i18n translate key
xpack.securitySolution.appLinks.automaticMigrations.title is duplicated with
conflicting defaultMessage values in the i18n.translate calls around the title
for "Automatic migrations" and the other "Migrations"; change one of the keys to
a new unique key (e.g. xpack.securitySolution.appLinks.migrations.title or
xpack.securitySolution.appLinks.automaticMigrations.title_short) in the
i18n.translate invocation(s) in links.ts, update the corresponding
defaultMessage to match the intended text, and then update any usages or imports
that reference the old key so all references use the new unique key.
---
Outside diff comments:
In
`@x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/cards/common/integrations/callouts/integration_card_top_callout.tsx`:
- Around line 37-39: The early return currently checks only
showAgentlessCallout, showEndpointCallout, and showActiveCallout so
MigrationsCallout never renders; update the conditional that returns null to
also consider the migrations flag (e.g., include showMigrationsCallout) or
compute/inline the same predicate used to decide rendering of MigrationsCallout
and include it (so change "if (!showAgentlessCallout && !showEndpointCallout &&
!showActiveCallout) return null" to also check the migrations condition), or
alternatively move the MigrationsCallout render above the early return; refer to
the showAgentlessCallout, showEndpointCallout, showActiveCallout, and
MigrationsCallout symbols to locate the changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 81e04755-e664-407e-8730-981075cfd641
⛔ Files ignored due to path filters (8)
x-pack/solutions/security/plugins/security_solution/public/common/icons/agent/icon.svgis excluded by!**/*.svgx-pack/solutions/security/plugins/security_solution/public/common/icons/agent/icon_dark.svgis excluded by!**/*.svgx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/images/demo_card.pngis excluded by!**/*.pngx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/images/demo_card_dark.pngis excluded by!**/*.pngx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/images/teammates_card.pngis excluded by!**/*.pngx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/images/teammates_card_dark.pngis excluded by!**/*.pngx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/images/video_card.pngis excluded by!**/*.pngx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/images/video_card_dark.pngis excluded by!**/*.png
📒 Files selected for processing (35)
src/platform/packages/shared/deeplinks/security/deep_links.tsx-pack/platform/plugins/private/translations/translations/de-DE.jsonx-pack/platform/plugins/private/translations/translations/fr-FR.jsonx-pack/platform/plugins/private/translations/translations/ja-JP.jsonx-pack/platform/plugins/private/translations/translations/zh-CN.jsonx-pack/solutions/security/plugins/security_solution/common/constants.tsx-pack/solutions/security/plugins/security_solution/public/common/icons/agent/index.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/body_config.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/cards/common/integrations/callouts/integration_card_top_callout.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/cards/common/integrations/callouts/migrations_callout.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_body_config.test.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_body_config.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/onboarding_body.test.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/onboarding_body.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/common/link_card.styles.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/common/link_card.test.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/common/link_card.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/demo_card.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/index.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/translations.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/index.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/teammates_card.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/translations.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/index.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/translations.tsx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/video_card.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/video_modal.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/onboarding_header.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/onboarding_header_topic_selector.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_router.tsxx-pack/solutions/security/plugins/security_solution/public/onboarding/constants.tsx-pack/solutions/security/plugins/security_solution/public/siem_migrations/create.tsxx-pack/solutions/security/plugins/security_solution/public/siem_migrations/links.tsx-pack/solutions/security/plugins/security_solution/public/siem_migrations/routes.tsxx-pack/solutions/security/plugins/security_solution_serverless/public/navigation/navigation_tree.ts
💤 Files with no reviewable changes (19)
- x-pack/platform/plugins/private/translations/translations/ja-JP.json
- x-pack/platform/plugins/private/translations/translations/zh-CN.json
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/index.ts
- x-pack/platform/plugins/private/translations/translations/fr-FR.json
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/index.ts
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/onboarding_header.tsx
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/common/link_card.styles.ts
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/index.ts
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/translations.ts
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/teammates_card.tsx
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/demo_card/demo_card.tsx
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/teammates_card/translations.ts
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/common/link_card.tsx
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/translations.ts
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/common/link_card.test.tsx
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/video_card.tsx
- x-pack/platform/plugins/private/translations/translations/de-DE.json
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/cards/video_card/video_modal.tsx
- x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_header/onboarding_header_topic_selector.tsx
| it('should return the body config for the selected topic', () => { | ||
| const { result } = renderHook(() => useBodyConfig()); | ||
| const { result } = renderHook(() => useBodyConfig(OnboardingTopicId.default)); | ||
| expect(result.current).toEqual(bodyConfig); | ||
| }); |
There was a problem hiding this comment.
Test assertion uses wrong variable — will fail.
The config on line 15 maps OnboardingTopicId.default to { body: defaultBodyConfig }, but line 42 asserts against bodyConfig. These have different titles ('Default Group 1' vs 'Group 1').
Proposed fix
it('should return the body config for the selected topic', () => {
const { result } = renderHook(() => useBodyConfig(OnboardingTopicId.default));
- expect(result.current).toEqual(bodyConfig);
+ expect(result.current).toEqual(defaultBodyConfig);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should return the body config for the selected topic', () => { | |
| const { result } = renderHook(() => useBodyConfig()); | |
| const { result } = renderHook(() => useBodyConfig(OnboardingTopicId.default)); | |
| expect(result.current).toEqual(bodyConfig); | |
| }); | |
| it('should return the body config for the selected topic', () => { | |
| const { result } = renderHook(() => useBodyConfig(OnboardingTopicId.default)); | |
| expect(result.current).toEqual(defaultBodyConfig); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@x-pack/solutions/security/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_body_config.test.ts`
around lines 40 - 43, The test for useBodyConfig is asserting the wrong expected
value: when calling useBodyConfig(OnboardingTopicId.default) the hook returns
the mapped config defaultBodyConfig (mapped under OnboardingTopicId.default) but
the test asserts against bodyConfig; update the assertion to expect
defaultBodyConfig instead of bodyConfig so the titles match (refer to
useBodyConfig, OnboardingTopicId.default, bodyConfig, and defaultBodyConfig).
| title: i18n.translate('xpack.securitySolution.appLinks.automaticMigrations.title', { | ||
| defaultMessage: 'Automatic migrations', | ||
| }), |
There was a problem hiding this comment.
Duplicate i18n key with conflicting default messages.
Line 29 and line 85 both use xpack.securitySolution.appLinks.automaticMigrations.title but with different defaultMessage values:
- Line 29:
'Automatic migrations' - Line 85:
'Migrations'
This causes non-deterministic text at runtime. Use distinct keys.
Proposed fix
{
id: SecurityPageName.siemMigrationsCreate,
- title: i18n.translate('xpack.securitySolution.appLinks.automaticMigrations.title', {
+ title: i18n.translate('xpack.securitySolution.appLinks.siemMigrationsCreate.title', {
defaultMessage: 'Automatic migrations',
}),
- description: i18n.translate('xpack.securitySolution.appLinks.automaticMigrations.description', {
+ description: i18n.translate('xpack.securitySolution.appLinks.siemMigrationsCreate.description', {
defaultMessage: 'Migrate your SIEM to Elastic using AI powered Automatic migration.',
}),Also applies to: 85-87
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@x-pack/solutions/security/plugins/security_solution/public/siem_migrations/links.ts`
around lines 29 - 31, The i18n translate key
xpack.securitySolution.appLinks.automaticMigrations.title is duplicated with
conflicting defaultMessage values in the i18n.translate calls around the title
for "Automatic migrations" and the other "Migrations"; change one of the keys to
a new unique key (e.g. xpack.securitySolution.appLinks.migrations.title or
xpack.securitySolution.appLinks.automaticMigrations.title_short) in the
i18n.translate invocation(s) in links.ts, update the corresponding
defaultMessage to match the intended text, and then update any usages or imports
that reference the old key so all references use the new unique key.
|
@kqualters-elastic , i have done fixes in below PR and reverted Onboarding changes. After merging above PR in your branch overall changes should look much less( like this, only 24 files + 8 Cypress ). But Please let me know if you see any issues.
|
|
What do you think about this? I would really like your opinion here as well @ferenrigue. Screenshot is what is there in the system after our changes. In Red, i suggest some modifications. What do you think? 
|
There are some changes in that pr that I think were requested, such as removing the header cards (see Figma), and I think the bulk of the changes (in terms of LOC) are just adding that back. The main change is how topic id works, which is fine with me, adding the wrapper component instead of passing as a prop to potentially allow future onboarding topics, I'm going to ask about this in the sync, how likely it is that more topics are added, and decide based on that info. |
kqualters-elastic
added
release_note:feature
Yes, sorry that my bad. Let me remove those. |
|
@logeekal I just pushed what I think is a best of both worlds approach. topicId is now an optional parameter to OnboardingBody, if not supplied everything works as before, if it is, this value is used. This lets us use the component as is without the wrapper approach in your pr, and allows any hypothetical future onboarding topic to work as expected, you can see that the useBodyConfig.ts, and OnboardingBody component unit tests are now unchanged (or will be). I also fixed the hash redirects in OnboardingRouter, so that old flows work perfectly, even in cypress tests, instead of creating a new url SIEM_MIGRATIONS_URL, we just use the old |
💔 Build Failed
Failed CI Steps
Test Failures
Metrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsmiscellaneous assets size
History
|
logeekal
left a comment
logeekal
left a comment
There was a problem hiding this comment.
Thanks @kqualters-elastic for addressing the feedback... this looks better as we not breaking existing code.
3 participants
Summary
This pr moves the migration creation flow from a tab within onboarding to it's own dedicated page, keeping as much of the logic from onboarding in place as possible, including using the context provider. Also adds a redirect from the old route, and a link from the card page for migrations, and removes the 3 top level cards from onboarding per UX.
There will be 1 small follow up pr to restructure the navigation tree with these new changes and the soon to come launchpad section.
Checklist
Check the PR satisfies following conditions.
Reviewers should verify this PR satisfies this list as well.
release_note:*label is applied per the guidelines