Add knowledgebase tab + storybook journey and tests#272
Merged
apinkert merged 3 commits intoRedHatInsights:masterfrom Mar 23, 2026
Merged
Add knowledgebase tab + storybook journey and tests#272apinkert merged 3 commits intoRedHatInsights:masterfrom
apinkert merged 3 commits intoRedHatInsights:masterfrom
Conversation
Summary by CodeRabbit
WalkthroughAdds a Knowledgebase feature to the Help Panel: feature-flag support in Storybook, new i18n messages, a rebuilt KBPanel with search, bundle-scoped filtering and pagination, mock-data support, and extensive Storybook component and user-journey tests. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (1)
src/components/HelpPanel/HelpPanelTabs/KBPanel.stories.tsx (1)
107-115: Keep one Storybook case on the real article source.
KBPanelWrapperalways injectsmockKBArticles, so every play test skipsgetAllKBArticles()and the real tag mapping fromrecommendedContentConfig—the code path the live Help Panel uses. Adding one story withoutmockArticleswould catch source/filter regressions much earlier.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/HelpPanel/HelpPanelTabs/KBPanel.stories.tsx` around lines 107 - 115, The Story currently always injects mockKBArticles into KBPanel (via the story using <KBPanel setNewActionTitle={() => {}} mockArticles={mockKBArticles} />) which bypasses getAllKBArticles() and the real tag mapping from recommendedContentConfig; add one Storybook case that does NOT pass mockArticles (i.e., render <KBPanel setNewActionTitle={...} />) so KBPanel uses its real data source and exercises getAllKBArticles() and the recommendedContentConfig tag/filter logic, keeping the existing mock-based story for isolated tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/HelpPanel/HelpPanelCustomTabs.tsx`:
- Around line 91-94: Replace the hard-coded title 'Knowledgebase' used for the
TabType.kb entry in HelpPanelCustomTabs with the i18n message key defined in
Messages.ts (helpPanel.kb.title) and use that same localized string for the
tab's aria-label; update the object that sets title: 'Knowledgebase' (for
TabType.kb) to pull the translated message via the app's i18n helper (the same
accessor used elsewhere) and ensure aria-label references that translated value
instead of a literal.
In `@src/components/HelpPanel/HelpPanelTabs/KBPanel.tsx`:
- Around line 41-59: getAllKBArticles currently flattens
bundleRecommendedContent 1:1 and yields duplicate KB entries when the same
article appears under multiple bundle keys; update getAllKBArticles to
deduplicate by a stable key (e.g., item.url or item.id) and, when duplicates are
found, merge/union their bundleTags into a single KnowledgebaseArticle before
pushing to the kbArticles array so downstream bundle filtering works correctly
(ensure the generated id remains stable, e.g., derived from the URL or article
identifier rather than bundleId-index). Keep the "static" && "kb" filter but
perform the dedupe-and-merge step while iterating bundleRecommendedContent so
returned KnowledgebaseArticle objects have combined bundleTags and no duplicate
entries.
In `@src/user-journeys/HelpPanelKBPanel.stories.tsx`:
- Around line 129-136: The test currently only checks for the presence of the
count label using waitFor + canvas.queryByText(/Knowledgebase articles \(/i)
which can pass even when results are wrong; update the assertions to verify
filtered results themselves by asserting the exact expected count text (e.g.,
"Knowledgebase articles (N)") or by asserting a representative article title is
present after each search/toggle action. Locate the waitFor blocks and
assertions in HelpPanelKBPanel.stories.tsx (the calls using waitFor,
TEST_TIMEOUTS.ELEMENT_WAIT, and canvas.queryByText) and replace/augment the
generic existence checks with assertions that validate the actual result set for
the searches/toggles mentioned (also apply the same change to the other similar
blocks around the comment ranges 210-235 and 330-336). Ensure the assertions run
after the same waitFor so timing is preserved.
---
Nitpick comments:
In `@src/components/HelpPanel/HelpPanelTabs/KBPanel.stories.tsx`:
- Around line 107-115: The Story currently always injects mockKBArticles into
KBPanel (via the story using <KBPanel setNewActionTitle={() => {}}
mockArticles={mockKBArticles} />) which bypasses getAllKBArticles() and the real
tag mapping from recommendedContentConfig; add one Storybook case that does NOT
pass mockArticles (i.e., render <KBPanel setNewActionTitle={...} />) so KBPanel
uses its real data source and exercises getAllKBArticles() and the
recommendedContentConfig tag/filter logic, keeping the existing mock-based story
for isolated tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6456faeb-06b4-44bb-9ca0-b4a7da5ff000
📒 Files selected for processing (7)
.storybook/hooks/unleash.jssrc/Messages.tssrc/components/HelpPanel/HelpPanelCustomTabs.tsxsrc/components/HelpPanel/HelpPanelTabs/KBPanel.stories.tsxsrc/components/HelpPanel/HelpPanelTabs/KBPanel.tsxsrc/user-journeys/HelpPanelKBPanel.stories.tsxsrc/user-journeys/_shared/helpPanelJourneyHelpers.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/user-journeys/HelpPanelKBPanel.stories.tsx (1)
348-357:⚠️ Potential issue | 🟡 MinorAssert on filtered results, not just the count label.
This assertion only verifies the count label exists but doesn't validate the actual search results. If the search or bundle filter is broken, this test could still pass with incorrect results.
Consider asserting on a representative article title or the exact count like other steps do:
,💡 Suggested improvement
// Verify search is applied with bundle filter - // Note: Actual results depend on bundle KB articles and their tags await waitFor( () => { - // Just verify the search executed and count updated - const count = canvas.queryByText(/Knowledgebase articles \(/i); - expect(count).toBeInTheDocument(); + // Verify results contain expected article or count + const articles = canvas.queryAllByRole('link', { + name: /Red Hat/i, + }); + expect(articles.length).toBeGreaterThanOrEqual(0); // 0 is valid if no matches in bundle }, { timeout: TEST_TIMEOUTS.ELEMENT_WAIT } );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/user-journeys/HelpPanelKBPanel.stories.tsx` around lines 348 - 357, The test currently only checks that the "Knowledgebase articles (" label exists by using canvas.queryByText inside the waitFor (with TEST_TIMEOUTS.ELEMENT_WAIT), which doesn't validate filtered results; update the assertion in the waitFor used in HelpPanelKBPanel.stories.tsx to assert on the actual filtered output — for example, wait for a representative article title to appear (use canvas.getByText or canvas.findByText for a known article name) or assert the exact count in the label (e.g., match "Knowledgebase articles (N)") so the test fails if the search or bundle filter returns incorrect results; keep the waitFor wrapper and timeout intact but replace the loose queryByText/count existence check with a concrete result assertion.
🧹 Nitpick comments (3)
src/components/HelpPanel/HelpPanelTabs/KBPanel.tsx (2)
42-73: Deduplication logic looks good but ID generation could cause collisions.The deduplication by URL and bundleTags merging addresses the previous review feedback. However, the
urlHashderivation usingitem.url.split('/').pop()may not be unique across different KB articles if they have the same trailing path segment (e.g., different articles ending in/indexor numeric IDs).Consider using a hash of the full URL or the URL itself as the ID for guaranteed uniqueness:
♻️ Suggested improvement for stable unique IDs
} else { - // Create new entry with stable ID based on URL - const urlHash = item.url.split('/').pop() || item.url; + // Create new entry with stable ID based on full URL + const urlHash = encodeURIComponent(item.url); articlesByUrl.set(item.url, { id: `kb-${urlHash}`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/HelpPanel/HelpPanelTabs/KBPanel.tsx` around lines 42 - 73, In getAllKBArticles, the current id generation using url.split('/').pop() can collide; replace that logic to produce a stable unique id per article by either using the full URL string (e.g., id: `kb-${item.url}`) or a deterministic hash of the full URL (compute inside getAllKBArticles) when creating the new KnowledgebaseArticle entry; update the id assignment in the branch that sets articlesByUrl.set(item.url, { id: ..., title: item.title, url: item.url, bundleTags: ... }) so IDs are guaranteed unique across different URLs.
92-99: TypeScript suppressions are safe but should be tracked.The
@ts-ignorecomments forgetBundleDataandgetAvailableBundlesindicate missing type definitions in the chrome API types. The optional chaining with fallback values (|| {}and|| []) provides adequate runtime safety.Consider opening an issue to add proper type definitions for these methods to eliminate the suppressions in the future.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/HelpPanel/HelpPanelTabs/KBPanel.tsx` around lines 92 - 99, The code uses `@ts-ignore` for chrome?.getBundleData and chrome?.getAvailableBundles when extracting bundleId and availableBundles; instead of leaving suppressions, open an issue to add proper TypeScript definitions for these methods and then remove the `@ts-ignore` comments: add or augment the chrome namespace/type declarations to include getBundleData(): { bundleId?: string } | undefined and getAvailableBundles(): any[] (or a more specific type), update the KBPanel.tsx usages (bundleId and availableBundles) to rely on the new types, and remove the eslint/ts-ignore lines so the code is properly typed.src/components/HelpPanel/HelpPanelTabs/KBPanel.stories.tsx (1)
445-492: Consider extracting shared wrapper logic to reduce duplication.The
WithRealDatastory duplicates the context provider setup and chrome API mocking fromKBPanelWrapper. While this duplication is understandable given the differentmockArticlesbehavior, consider refactoring to share the common setup:♻️ Optional refactor to reduce duplication
+const KBPanelWrapperBase = ({ + bundle = 'insights', + useMockArticles = true, +}: { + bundle?: string; + useMockArticles?: boolean; +}) => { + // ... shared setup logic ... + return ( + <IntlProvider locale="en" defaultLocale="en"> + <QuickStartContextProvider value={quickStartContextValue}> + <div style={{ height: '600px', width: '400px' }}> + <KBPanel + setNewActionTitle={() => {}} + mockArticles={useMockArticles ? mockKBArticles : undefined} + /> + </div> + </QuickStartContextProvider> + </IntlProvider> + ); +}; export const WithRealData: Story = { - render: () => { - // ... duplicated setup ... - }, + render: () => <KBPanelWrapperBase useMockArticles={false} />, play: async ({ canvasElement }) => { // ... assertions ... }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/HelpPanel/HelpPanelTabs/KBPanel.stories.tsx` around lines 445 - 492, WithRealData duplicates the context/provider and chrome API mocking found in KBPanelWrapper; extract those common pieces into a shared helper/wrapper component (e.g., KBPanelStoryWrapper) that creates quickStartContextValue via useValuesForQuickStartContext, sets up the window.insights.chrome getBundleData/getAvailableBundles overrides, and renders IntlProvider + QuickStartContextProvider around KBPanel; update WithRealData to reuse KBPanelStoryWrapper and only pass the differing bits (like mockArticles or setNewActionTitle) so the chrome mocking and quick-start context logic is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/user-journeys/HelpPanelKBPanel.stories.tsx`:
- Around line 247-255: The conditional around the articles assertion is
redundant and doesn't validate bundle filtering; update the test in
HelpPanelKBPanel.stories.tsx to remove the if block and assert a meaningful
expectation directly (e.g., expect(articles.length).toBeGreaterThan(0) or a
specific minimum) after obtaining articles via
canvasElement.querySelectorAll('[data-ouia-component-id="help-panel-kb-articles-list"]
a'), so the test will fail if zero articles are returned when some are expected.
---
Duplicate comments:
In `@src/user-journeys/HelpPanelKBPanel.stories.tsx`:
- Around line 348-357: The test currently only checks that the "Knowledgebase
articles (" label exists by using canvas.queryByText inside the waitFor (with
TEST_TIMEOUTS.ELEMENT_WAIT), which doesn't validate filtered results; update the
assertion in the waitFor used in HelpPanelKBPanel.stories.tsx to assert on the
actual filtered output — for example, wait for a representative article title to
appear (use canvas.getByText or canvas.findByText for a known article name) or
assert the exact count in the label (e.g., match "Knowledgebase articles (N)")
so the test fails if the search or bundle filter returns incorrect results; keep
the waitFor wrapper and timeout intact but replace the loose queryByText/count
existence check with a concrete result assertion.
---
Nitpick comments:
In `@src/components/HelpPanel/HelpPanelTabs/KBPanel.stories.tsx`:
- Around line 445-492: WithRealData duplicates the context/provider and chrome
API mocking found in KBPanelWrapper; extract those common pieces into a shared
helper/wrapper component (e.g., KBPanelStoryWrapper) that creates
quickStartContextValue via useValuesForQuickStartContext, sets up the
window.insights.chrome getBundleData/getAvailableBundles overrides, and renders
IntlProvider + QuickStartContextProvider around KBPanel; update WithRealData to
reuse KBPanelStoryWrapper and only pass the differing bits (like mockArticles or
setNewActionTitle) so the chrome mocking and quick-start context logic is
centralized.
In `@src/components/HelpPanel/HelpPanelTabs/KBPanel.tsx`:
- Around line 42-73: In getAllKBArticles, the current id generation using
url.split('/').pop() can collide; replace that logic to produce a stable unique
id per article by either using the full URL string (e.g., id: `kb-${item.url}`)
or a deterministic hash of the full URL (compute inside getAllKBArticles) when
creating the new KnowledgebaseArticle entry; update the id assignment in the
branch that sets articlesByUrl.set(item.url, { id: ..., title: item.title, url:
item.url, bundleTags: ... }) so IDs are guaranteed unique across different URLs.
- Around line 92-99: The code uses `@ts-ignore` for chrome?.getBundleData and
chrome?.getAvailableBundles when extracting bundleId and availableBundles;
instead of leaving suppressions, open an issue to add proper TypeScript
definitions for these methods and then remove the `@ts-ignore` comments: add or
augment the chrome namespace/type declarations to include getBundleData(): {
bundleId?: string } | undefined and getAvailableBundles(): any[] (or a more
specific type), update the KBPanel.tsx usages (bundleId and availableBundles) to
rely on the new types, and remove the eslint/ts-ignore lines so the code is
properly typed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ceadb0f7-c2d2-4ee9-9245-c161497267d9
📒 Files selected for processing (4)
src/components/HelpPanel/HelpPanelCustomTabs.tsxsrc/components/HelpPanel/HelpPanelTabs/KBPanel.stories.tsxsrc/components/HelpPanel/HelpPanelTabs/KBPanel.tsxsrc/user-journeys/HelpPanelKBPanel.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/HelpPanel/HelpPanelCustomTabs.tsx
riccardo-forina
approved these changes
Mar 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For RHCLOUD-45777, RHCLOUD-45244, and RHCLOUD-45249. This tab is currently using mocked data as the API has not been implemented. I went ahead and added both the user journey and the stories related to it since it won't be visible on stage (hidden behind unleash feature flag)
Screen.Recording.2026-03-20.at.12.28.40.PM.mov