Add feedback tab user journey#267
Merged
apinkert merged 2 commits intoRedHatInsights:masterfrom Mar 16, 2026
Merged
Conversation
Summary by CodeRabbit
WalkthroughAdds a new Storybook user-journey test suite that defines a default meta and eight stories to automate seven feedback-related workflows plus a manual testing entry point for the Help Panel Feedback Panel. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
📝 Coding Plan
Comment Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
![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
🧹 Nitpick comments (1)
src/user-journeys/HelpPanelFeedbackPanel.stories.tsx (1)
149-150: Replace global DOM selectors with canvas-scoped semantic queries.Using
document.getElementById/querySelectorin Storybook play functions is less idiomatic and brittle. The file already importswithinand usescanvas.findByRoleandcanvas.getByTextelsewhere—apply this pattern consistently by usingcanvas.findByRole,canvas.findByLabelText,canvas.findByText, or stabledata-testidattributes throughout.Applies to: lines 149, 156, 165, 173, 203, 210, 219, 227, 271, 278, 288, 299, 331, 340, 378, 382, 390, 405
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/user-journeys/HelpPanelFeedbackPanel.stories.tsx` around lines 149 - 150, Replace global DOM selectors (e.g., the getElementById call that targets 'feedback-description-text' and other document.querySelector/document.getElementById usages) with canvas-scoped semantic queries used elsewhere in this file: use canvas.findByLabelText, canvas.findByRole, canvas.findByText or canvas.findByTestId (or within(canvas).getBy*) to locate elements inside the story canvas; update every instance listed (including the 'feedback-description-text' lookup and the other occurrences on the lines noted) to use those canvas-scoped queries so tests are stable and consistent with the existing canvas.findByRole/canvas.getByText pattern.
🤖 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/HelpPanelFeedbackPanel.stories.tsx`:
- Line 396: Update the misleading console.log in the HelpPanelFeedbackPanel
story: replace the message "UJ: ✅ Email display before checkbox checked" (the
console.log call that runs during the Step07 state transition) with a message
that correctly reflects it executes after the checkbox is checked, e.g. "UJ: ✅
Email display after checkbox checked" so the log matches the actual state and
flow.
- Around line 340-356: The test currently guards breadcrumbLink with an if,
allowing the step to silently skip when the element is missing; instead, assert
its presence and fail fast: replace the conditional guard around the
document.querySelector('.feedback-breadcrumb-link') (breadcrumbLink) with an
explicit presence check (e.g., expect(breadcrumbLink).toBeTruthy() or throw if
null) before calling userEvent.click and delay, keeping the rest of the flow
(userEvent.click, delay(TEST_TIMEOUTS.AFTER_CLICK), waitFor and
canvas.getByText(/Tell us about your experience/i)) unchanged so the test fails
when the breadcrumb is not found.
---
Nitpick comments:
In `@src/user-journeys/HelpPanelFeedbackPanel.stories.tsx`:
- Around line 149-150: Replace global DOM selectors (e.g., the getElementById
call that targets 'feedback-description-text' and other
document.querySelector/document.getElementById usages) with canvas-scoped
semantic queries used elsewhere in this file: use canvas.findByLabelText,
canvas.findByRole, canvas.findByText or canvas.findByTestId (or
within(canvas).getBy*) to locate elements inside the story canvas; update every
instance listed (including the 'feedback-description-text' lookup and the other
occurrences on the lines noted) to use those canvas-scoped queries so tests are
stable and consistent with the existing canvas.findByRole/canvas.getByText
pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4443477b-569a-4ed0-abb4-43188d9065ad
📒 Files selected for processing (1)
src/user-journeys/HelpPanelFeedbackPanel.stories.tsx
InsaneZein
approved these changes
Mar 16, 2026
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/user-journeys/HelpPanelFeedbackPanel.stories.tsx (2)
381-381:⚠️ Potential issue | 🟡 MinorCorrect the Step07 log message to match actual state.
This log runs after the checkbox is checked, so the message is currently misleading.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/user-journeys/HelpPanelFeedbackPanel.stories.tsx` at line 381, The Step07 console log in HelpPanelFeedbackPanel.stories.tsx is misleading because it says "Email display before checkbox checked" but executes after the checkbox is checked; update the console.log in the Step07 handler (the log near the checkbox interaction in the HelpPanelFeedbackPanel story) to reflect the actual state (e.g., "UJ: ✅ Email display after checkbox checked" or equivalent) so the message matches the action performed.
325-341:⚠️ Potential issue | 🟠 MajorFail fast when breadcrumb link is missing.
The
if (breadcrumbLink)guard allows this journey to pass even if breadcrumb navigation is broken. Make breadcrumb presence mandatory before clicking.Suggested fix
// Click the breadcrumb to go back to home const breadcrumbLink = document.querySelector('.feedback-breadcrumb-link'); - - if (breadcrumbLink) { - await userEvent.click(breadcrumbLink as HTMLElement); - await delay(TEST_TIMEOUTS.AFTER_CLICK); - - // Verify we're back at home - await waitFor( - () => { - const title = canvas.getByText(/Tell us about your experience/i); - expect(title).toBeInTheDocument(); - }, - { timeout: 5000 } - ); - - console.log('UJ: ✅ Breadcrumb navigation back to feedback home'); - } + expect(breadcrumbLink).toBeInTheDocument(); + await userEvent.click(breadcrumbLink as HTMLElement); + await delay(TEST_TIMEOUTS.AFTER_CLICK); + + // Verify we're back at home + await waitFor( + () => { + const title = canvas.getByText(/Tell us about your experience/i); + expect(title).toBeInTheDocument(); + }, + { timeout: 5000 } + ); + + console.log('UJ: ✅ Breadcrumb navigation back to feedback home');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/user-journeys/HelpPanelFeedbackPanel.stories.tsx` around lines 325 - 341, The test currently skips breadcrumb verification when document.querySelector('.feedback-breadcrumb-link') returns null; make the breadcrumb presence mandatory by replacing the optional guard with an explicit assertion (e.g., expect(breadcrumbLink).toBeTruthy() or throw an Error) immediately after querying it, so that if breadcrumbLink is null the test fails fast before calling userEvent.click; update the block around breadcrumbLink, userEvent.click(breadcrumbLink as HTMLElement), delay(TEST_TIMEOUTS.AFTER_CLICK) and the following waitFor to rely on that assertion.
🧹 Nitpick comments (1)
src/user-journeys/HelpPanelFeedbackPanel.stories.tsx (1)
93-94: Prefer shared timeout constant over repeated5000literals.Using
TEST_TIMEOUTS.ELEMENT_WAIThere improves consistency and centralizes tuning.Also applies to: 137-138, 191-192, 229-230, 259-260, 287-288, 321-322, 337-338, 378-379, 393-394
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/user-journeys/HelpPanelFeedbackPanel.stories.tsx` around lines 93 - 94, Replace all hard-coded 5000 timeout literals in HelpPanelFeedbackPanel.stories.tsx with the shared constant TEST_TIMEOUTS.ELEMENT_WAIT; update each waitFor/wait/expect call (the sites noted around the occurrences) to use TEST_TIMEOUTS.ELEMENT_WAIT, and add the necessary import for TEST_TIMEOUTS at the top of the file if missing so the file compiles and all referenced places (the clusters near lines 93-94, 137-138, 191-192, 229-230, 259-260, 287-288, 321-322, 337-338, 378-379, 393-394) consistently use the centralized timeout constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/user-journeys/HelpPanelFeedbackPanel.stories.tsx`:
- Line 381: The Step07 console log in HelpPanelFeedbackPanel.stories.tsx is
misleading because it says "Email display before checkbox checked" but executes
after the checkbox is checked; update the console.log in the Step07 handler (the
log near the checkbox interaction in the HelpPanelFeedbackPanel story) to
reflect the actual state (e.g., "UJ: ✅ Email display after checkbox checked" or
equivalent) so the message matches the action performed.
- Around line 325-341: The test currently skips breadcrumb verification when
document.querySelector('.feedback-breadcrumb-link') returns null; make the
breadcrumb presence mandatory by replacing the optional guard with an explicit
assertion (e.g., expect(breadcrumbLink).toBeTruthy() or throw an Error)
immediately after querying it, so that if breadcrumbLink is null the test fails
fast before calling userEvent.click; update the block around breadcrumbLink,
userEvent.click(breadcrumbLink as HTMLElement), delay(TEST_TIMEOUTS.AFTER_CLICK)
and the following waitFor to rely on that assertion.
---
Nitpick comments:
In `@src/user-journeys/HelpPanelFeedbackPanel.stories.tsx`:
- Around line 93-94: Replace all hard-coded 5000 timeout literals in
HelpPanelFeedbackPanel.stories.tsx with the shared constant
TEST_TIMEOUTS.ELEMENT_WAIT; update each waitFor/wait/expect call (the sites
noted around the occurrences) to use TEST_TIMEOUTS.ELEMENT_WAIT, and add the
necessary import for TEST_TIMEOUTS at the top of the file if missing so the file
compiles and all referenced places (the clusters near lines 93-94, 137-138,
191-192, 229-230, 259-260, 287-288, 321-322, 337-338, 378-379, 393-394)
consistently use the centralized timeout constant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dff20ce4-a2e7-45d3-9b18-301be5773e7c
📒 Files selected for processing (1)
src/user-journeys/HelpPanelFeedbackPanel.stories.tsx
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-45772
Screen.Recording.2026-03-16.at.10.19.59.AM.mov