Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
592 changes: 592 additions & 0 deletions src/assets/rh_feedback-dark.svg
Copy link
Member

Choose a reason for hiding this comment

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

This svg doesn't looks right for dark theme. Collaborate with ux to improve the svg. You can also refer to the patternfly feedback form svg for light and dark theme

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
515 changes: 515 additions & 0 deletions src/assets/rh_feedback.svg
Copy link
Member

Choose a reason for hiding this comment

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

Primary color for konflux is of orange shade. This red color in svg might conflict with the overall theme of konflux

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
66 changes: 66 additions & 0 deletions src/components/FeedbackSection/FeedbackModal.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
.feedback-modal {
border-radius: 5px;
& > .pf-v5-c-modal-box__body {
padding: 0;
margin: 0;
}
&__feedback-flex {
margin-right: 0px !important;
}
&__feedback-description {
margin-inline-end: 0 !important;
}
&__image-flex {
background-color: var(--pf-v5-global--BackgroundColor--200);
}
&__feedback-image {
width: 100%;
height: 100%;
}
&__panel-content {
padding: var(--pf-v5-global--spacer--xl);
gap: var(--pf-v5-global--spacer--md);
height: 100%;
display: flex;
flex-direction: column;
align-items: flex-start;
}
&__panel-footer {
display: flex;
align-self: stretch;
flex-wrap: wrap;
padding: 0 0 var(--pf-v5-global--spacer--sm) 0 !important;
margin-top: auto;
box-shadow: none !important;
column-gap: var(--pf-v5-global--spacer--xl);
}
&__panel-header {
padding: 0 37px var(--pf-v5-global--spacer--sm) 0 !important;
}
&__content-main {
max-height: 450px !important;
display: flex;
flex-direction: column;
align-self: stretch;
}
&__cards-list {
gap: var(--pf-v5-global--spacer--md);
display: flex;
flex-direction: column;
align-items: flex-start;
padding: inherit !important;
}
&__section-card {
background-color: var(--pf-v5-global--BackgroundColor--200);
border-radius: 5px;
width: 100%;
&:hover {
cursor: pointer;
border: 1px solid var(--pf-v5-global--link--Color--light);
}
}

&__input-field {
padding-top: var(--pf-v5-global--spacer--md);
}
}
179 changes: 179 additions & 0 deletions src/components/FeedbackSection/FeedbackModal.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
import * as React from 'react';
import {
ModalVariant,
Flex,
FlexItem,
Bullseye,
Spinner,
Button,
ButtonType,
Panel,
PanelFooter,
} from '@patternfly/react-core';
import { ErrorBoundary } from '@sentry/react';
import { Formik, Form } from 'formik';
import { isEmpty } from 'lodash-es';
import { ComponentProps, createModalLauncher } from '~/components/modal/createModalLauncher';
import { useKonfluxPublicInfo } from '~/hooks/useKonfluxPublicInfo';
import RHsupportLight from '../../assets/rh_feedback.svg';
import BeginningSection from './components/BeginningSection';
import { FeedbackSections } from './consts';
import { BugInfo, FeatureInfo, getBugURL, getFeatureURL } from './feedback-utils';
import {
BugSectionValidationSchema,
FeatureSectionValidationSchema,
FeedbackSectionValidationSchema,
} from './FeedbackValidationSchemas';

const BugRFESection = React.lazy(() => import('./components/BugRFESection'));
const FeedbackSection = React.lazy(() => import('./components/FeedbackSection'));

import './FeedbackModal.scss';

export type FeedbackFormValues = {
bug?: BugInfo;
feature?: FeatureInfo;
feedback?: { description: string; scale: number };
};

const FeedbackModal: React.FC<React.PropsWithChildren<ComponentProps>> = ({ onClose }) => {
const [currentSection, setCurrentSection] = React.useState<FeedbackSections>(
FeedbackSections.BeginningSection,
);

const [konfluxInfo] = useKonfluxPublicInfo();
Comment on lines +39 to +44
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for feedback-related flags in flags.ts
rg -n 'feedback' --type=ts -i src/feature-flags/flags.ts

Repository: konflux-ci/konflux-ui

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Check the current content of flags.ts to see defined flags
head -100 src/feature-flags/flags.ts

Repository: konflux-ci/konflux-ui

Length of output: 3114


🏁 Script executed:

#!/bin/bash
# Search for IfFeature or useIsOnFeatureFlag usage in FeedbackModal.tsx
rg -n 'IfFeature|useIsOnFeatureFlag' src/components/FeedbackSection/FeedbackModal.tsx

Repository: konflux-ci/konflux-ui

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Check HelpDropdown.tsx for FeedbackModal usage and any feature flag wrapping
rg -n 'FeedbackModal|Feedback' src/components/HelpDropdown/ -A 3

Repository: konflux-ci/konflux-ui

Length of output: 133


🏁 Script executed:

#!/bin/bash
# Get the full FeedbackModal.tsx file to check for flag implementation
wc -l src/components/FeedbackSection/FeedbackModal.tsx

Repository: konflux-ci/konflux-ui

Length of output: 117


🏁 Script executed:

#!/bin/bash
# Get full flags.ts to see all defined flags
cat src/feature-flags/flags.ts | tail -50

Repository: konflux-ci/konflux-ui

Length of output: 1625


🏁 Script executed:

#!/bin/bash
# Read the FeedbackModal.tsx file to understand its content
cat -n src/components/FeedbackSection/FeedbackModal.tsx

Repository: konflux-ci/konflux-ui

Length of output: 8283


🏁 Script executed:

#!/bin/bash
# Search for where FeedbackModal is imported or used
rg -n 'FeedbackModal' --type=tsx --type=ts

Repository: konflux-ci/konflux-ui

Length of output: 92


🏁 Script executed:

#!/bin/bash
# Search for FeedbackModal imports and usage
rg -n 'FeedbackModal' --type=ts

Repository: konflux-ci/konflux-ui

Length of output: 808


🏁 Script executed:

#!/bin/bash
# Also search in tsx files using different approach
fd -e tsx -e ts -x rg -l 'FeedbackModal' {}

Repository: konflux-ci/konflux-ui

Length of output: 212


🏁 Script executed:

#!/bin/bash
# Search for where createFeedbackModal is used
rg -n 'createFeedbackModal'

Repository: konflux-ci/konflux-ui

Length of output: 383


🏁 Script executed:

#!/bin/bash
# Check HelpDropdown.tsx to see the full context
cat -n src/components/Header/HelpDropdown.tsx

Repository: konflux-ci/konflux-ui

Length of output: 3068


Wrap the FeedbackModal feature and its trigger behind a feature flag.

Per project conventions, new features must be gated using the IfFeature component or useIsOnFeatureFlag hook. Add a new flag to src/feature-flags/flags.ts (e.g., feedback-modal with defaultEnabled: false and status: 'wip'), then wrap both the FeedbackModal component and the "Share feedback" menu item in HelpDropdown.

🤖 Prompt for AI Agents
In `@src/components/FeedbackSection/FeedbackModal.tsx` around lines 39 - 44, Add a
new feature flag entry named feedback-modal in src/feature-flags/flags.ts with
defaultEnabled: false and status: 'wip', then gate the FeedbackModal and its UI
trigger by the feature flag: update the HelpDropdown menu item that renders the
"Share feedback" entry to only render when the flag is on (use IfFeature or
useIsOnFeatureFlag) and wrap or conditionally render the FeedbackModal component
itself (the FeedbackModal React component and its caller where onClose is
passed) behind the same flag so both the modal and its trigger are disabled by
default.


const handleSubmit = (values: { bug?: BugInfo; feature?: FeatureInfo }) => {
if (currentSection === FeedbackSections.BugSection) {
const { bug } = values;
const url = getBugURL(bug, konfluxInfo);
window.open(url, '_blank');
onClose(null, { submitClicked: true });
}

if (currentSection === FeedbackSections.FeatureSection) {
const { feature } = values;
const url = getFeatureURL(feature);
window.open(url, '_blank');
onClose(null, { submitClicked: true });
}
if (currentSection === FeedbackSections.FeedbackSection) {
// Add Sentry/Segment connection here
onClose(null, { submitClicked: true });
}
Comment on lines +46 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

handleSubmit destructures potentially undefined values without guards.

values.bug (line 48) and values.feature (line 55) could be undefined if Formik wasn't properly reinitialized (see prior comment). Even after fixing reinitialization, a defensive check is good practice since handleSubmit's parameter type allows all fields to be optional.

Proposed defensive check
   const handleSubmit = (values: { bug?: BugInfo; feature?: FeatureInfo }) => {
     if (currentSection === FeedbackSections.BugSection) {
       const { bug } = values;
+      if (!bug) return;
       const url = getBugURL(bug, konfluxInfo);
       window.open(url, '_blank');
       onClose(null, { submitClicked: true });
     }
 
     if (currentSection === FeedbackSections.FeatureSection) {
       const { feature } = values;
+      if (!feature) return;
       const url = getFeatureURL(feature);
📝 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.

Suggested change
const handleSubmit = (values: { bug?: BugInfo; feature?: FeatureInfo }) => {
if (currentSection === FeedbackSections.BugSection) {
const { bug } = values;
const url = getBugURL(bug, konfluxInfo);
window.open(url, '_blank');
onClose(null, { submitClicked: true });
}
if (currentSection === FeedbackSections.FeatureSection) {
const { feature } = values;
const url = getFeatureURL(feature);
window.open(url, '_blank');
onClose(null, { submitClicked: true });
}
if (currentSection === FeedbackSections.FeedbackSection) {
// Add Sentry/Segment connection here
onClose(null, { submitClicked: true });
}
const handleSubmit = (values: { bug?: BugInfo; feature?: FeatureInfo }) => {
if (currentSection === FeedbackSections.BugSection) {
const { bug } = values;
if (!bug) return;
const url = getBugURL(bug, konfluxInfo);
window.open(url, '_blank');
onClose(null, { submitClicked: true });
}
if (currentSection === FeedbackSections.FeatureSection) {
const { feature } = values;
if (!feature) return;
const url = getFeatureURL(feature);
window.open(url, '_blank');
onClose(null, { submitClicked: true });
}
if (currentSection === FeedbackSections.FeedbackSection) {
// Add Sentry/Segment connection here
onClose(null, { submitClicked: true });
}
🤖 Prompt for AI Agents
In `@src/components/FeedbackSection/FeedbackModal.tsx` around lines 46 - 63, The
handleSubmit function uses values.bug and values.feature without guarding for
undefined; update handleSubmit to defensively check that the extracted
bug/feature exists before calling getBugURL/getFeatureURL and window.open (e.g.,
if currentSection === FeedbackSections.BugSection then if (!values?.bug) {
onClose(null, { submitClicked: true }); return; } else const url =
getBugURL(values.bug, konfluxInfo) ...), and do the analogous guard for
FeedbackSections.FeatureSection; keep the existing onClose(null, {
submitClicked: true }) behavior when the field is missing or after successful
submission and avoid calling getBugURL/getFeatureURL with undefined.

};

return (
<Formik
Copy link
Member

Choose a reason for hiding this comment

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

I dont think you need formik for such simple forms. You can just use HTML forms here without formik

initialValues={
currentSection === FeedbackSections.BugSection
? {
bug: { title: '', description: '', getAdditionalInfo: false },
}
: currentSection === FeedbackSections.FeatureSection
? {
feature: { title: '', description: '' },
}
: {
feedback: {
description: '',
scale: 5,
},
}
}
onSubmit={handleSubmit}
validationSchema={
currentSection === FeedbackSections.BugSection
? BugSectionValidationSchema
: currentSection === FeedbackSections.FeatureSection
? FeatureSectionValidationSchema
: FeedbackSectionValidationSchema
}
>
Comment on lines +67 to +92
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Formik won't reinitialize when switching sections.

currentSection starts as BeginningSection, so initialValues is set to the feedback shape. When the user navigates to Bug or Feature, currentSection changes but Formik does not re-derive initialValues by default. The form fields for bug.* or feature.* will have no initial values, and handleSubmit will destructure undefined.

Fix: Either add enableReinitialize to <Formik>, or add a key={currentSection} to force a full remount when the section changes (which also clears touched/errors — likely the desired behavior here).

Recommended fix — add key to force remount
     <Formik
+      key={currentSection}
       initialValues={
         currentSection === FeedbackSections.BugSection
📝 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.

Suggested change
<Formik
initialValues={
currentSection === FeedbackSections.BugSection
? {
bug: { title: '', description: '', getAdditionalInfo: false },
}
: currentSection === FeedbackSections.FeatureSection
? {
feature: { title: '', description: '' },
}
: {
feedback: {
description: '',
scale: 5,
},
}
}
onSubmit={handleSubmit}
validationSchema={
currentSection === FeedbackSections.BugSection
? BugSectionValidationSchema
: currentSection === FeedbackSections.FeatureSection
? FeatureSectionValidationSchema
: FeedbackSectionValidationSchema
}
>
<Formik
key={currentSection}
initialValues={
currentSection === FeedbackSections.BugSection
? {
bug: { title: '', description: '', getAdditionalInfo: false },
}
: currentSection === FeedbackSections.FeatureSection
? {
feature: { title: '', description: '' },
}
: {
feedback: {
description: '',
scale: 5,
},
}
}
onSubmit={handleSubmit}
validationSchema={
currentSection === FeedbackSections.BugSection
? BugSectionValidationSchema
: currentSection === FeedbackSections.FeatureSection
? FeatureSectionValidationSchema
: FeedbackSectionValidationSchema
}
>
🤖 Prompt for AI Agents
In `@src/components/FeedbackSection/FeedbackModal.tsx` around lines 67 - 92, The
Formik instance in FeedbackModal does not reinitialize when currentSection
changes (so initialValues for bug/feature are not applied); update the Formik
usage to force a remount or reinitialize on section change by adding either
enableReinitialize={true} to the <Formik> props or adding key={currentSection}
to the <Formik> element so changing FeedbackSections
(currentSection/FeedbackSections) resets form state and prevents handleSubmit
from receiving undefined shapes.

{({ isSubmitting, errors, touched }) => {
return (
<Form>
<Flex
className="feedback-modal__feedback-flex"
direction={{ default: 'row' }}
alignItems={{ default: 'alignItemsStretch' }}
>
<FlexItem
className="feedback-modal__feedback-description"
flex={{ default: 'flex_2' }}
>
<Panel isScrollable className="feedback-modal__panel-content">
<ErrorBoundary>
<React.Suspense
fallback={
<Bullseye>
<Spinner size="xl" />
</Bullseye>
}
>
{currentSection === FeedbackSections.BeginningSection && (
<BeginningSection setCurrentSection={setCurrentSection} />
)}
{currentSection === FeedbackSections.FeedbackSection && <FeedbackSection />}
{currentSection === FeedbackSections.BugSection && (
<BugRFESection currentSection={FeedbackSections.BugSection} />
)}
{currentSection === FeedbackSections.FeatureSection && (
<BugRFESection currentSection={FeedbackSections.FeatureSection} />
)}
</React.Suspense>
</ErrorBoundary>

<PanelFooter className="feedback-modal__panel-footer">
{(currentSection === FeedbackSections.BugSection ||
currentSection === FeedbackSections.FeatureSection) && (
<Button
variant="primary"
type={ButtonType.submit}
isDisabled={isEmpty(touched) || !isEmpty(errors) || isSubmitting}
>
Preview on Github
</Button>
)}
{currentSection === FeedbackSections.FeedbackSection && (
<Button
variant="primary"
type={ButtonType.submit}
isDisabled={isEmpty(touched) || !isEmpty(errors) || isSubmitting}
>
Submit feedback
</Button>
)}
{currentSection !== FeedbackSections.BeginningSection && (
<Button
variant="secondary"
onClick={() => setCurrentSection(FeedbackSections.BeginningSection)}
>
Back
</Button>
)}
<Button variant="link" onClick={() => onClose(null, { submitClicked: false })}>
Cancel
</Button>
Comment on lines +130 to +157
Copy link
Member

Choose a reason for hiding this comment

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

These buttons should be part of their own forms

</PanelFooter>
</Panel>
</FlexItem>

<FlexItem className="feedback-modal__image-flex" flex={{ default: 'flex_1' }}>
<RHsupportLight className="feedback-modal__feedback-image" />
</FlexItem>
</Flex>
</Form>
);
}}
</Formik>
);
};

export const createFeedbackModal = createModalLauncher(FeedbackModal, {
'data-test': 'feedback-modal',
variant: ModalVariant.large,
className: 'feedback-modal',
});

export default FeedbackModal;
31 changes: 31 additions & 0 deletions src/components/FeedbackSection/FeedbackValidationSchemas.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import * as yup from 'yup';

export const BugSectionValidationSchema = yup.object({
bug: yup.object({
title: yup.string().required('Required').max(400, 'Should be less than 400 characters.'),
description: yup
.string()
.required('Required')
.max(2000, 'Should be less than 2000 characters.'),
}),
});

export const FeatureSectionValidationSchema = yup.object({
feature: yup.object({
title: yup.string().required('Required').max(400, 'Should be less than 400 characters.'),
description: yup
.string()
.required('Required')
.max(2000, 'Should be less than 2000 characters.'),
}),
});

export const FeedbackSectionValidationSchema = yup.object({
feedback: yup.object({
scale: yup.number().required('Required'),
description: yup
.string()
.required('Required')
.max(2000, 'Should be less than 2000 characters.'),
}),
});
Comment on lines +23 to +31
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing email validation in FeedbackSectionValidationSchema.

The FeedbackSection.tsx component renders an email field (feedback.email), but this schema doesn't validate it. If the user provides an email, there's no format validation. Consider adding an optional email field with format check:

Proposed addition
 export const FeedbackSectionValidationSchema = yup.object({
   feedback: yup.object({
     scale: yup.number().required('Required'),
     description: yup
       .string()
       .required('Required')
       .max(2000, 'Should be less than 2000 characters.'),
+    email: yup.string().email('Must be a valid email address'),
   }),
 });
📝 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.

Suggested change
export const FeedbackSectionValidationSchema = yup.object({
feedback: yup.object({
scale: yup.number().required('Required'),
description: yup
.string()
.required('Required')
.max(2000, 'Should be less than 2000 characters.'),
}),
});
export const FeedbackSectionValidationSchema = yup.object({
feedback: yup.object({
scale: yup.number().required('Required'),
description: yup
.string()
.required('Required')
.max(2000, 'Should be less than 2000 characters.'),
email: yup.string().email('Must be a valid email address'),
}),
});
🤖 Prompt for AI Agents
In `@src/components/FeedbackSection/FeedbackValidationSchemas.ts` around lines 23
- 31, The schema FeedbackSectionValidationSchema is missing validation for the
email input rendered as feedback.email in FeedbackSection.tsx; add an optional
email rule under the feedback object (e.g., add feedback.email:
yup.string().email('Invalid email').notRequired() or .nullable() so empty values
pass but any provided value is checked for proper email format), updating the
FeedbackSectionValidationSchema object to include this new field.

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { render } from '@testing-library/react';
import BeginningSection from '../components/BeginningSection';

const setCurrentSectionMock = jest.fn();

describe('BeginningSection', () => {
it('should initialize with BeginingSection', () => {
const screen = render(<BeginningSection setCurrentSection={setCurrentSectionMock} />);
screen.getByText('Share feedback');
screen.getByText('Report a bug');
screen.getByText('Request a new feature');
});
});
25 changes: 25 additions & 0 deletions src/components/FeedbackSection/__tests__/BugRFESection.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import BugRFESection from '../components/BugRFESection';
import { FeedbackSections } from '../consts';
import { formikRenderer } from '~/unit-test-utils';

Check failure on line 3 in src/components/FeedbackSection/__tests__/BugRFESection.spec.tsx

View workflow job for this annotation

GitHub Actions / test (20.x)

`~/unit-test-utils` import should occur before import of `../components/BugRFESection`
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix import order per ESLint rule.

The ~/unit-test-utils absolute import should precede the relative imports.

Proposed fix
-import BugRFESection from '../components/BugRFESection';
-import { FeedbackSections } from '../consts';
-import { formikRenderer } from '~/unit-test-utils';
+import { formikRenderer } from '~/unit-test-utils';
+import BugRFESection from '../components/BugRFESection';
+import { FeedbackSections } from '../consts';
📝 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.

Suggested change
import BugRFESection from '../components/BugRFESection';
import { FeedbackSections } from '../consts';
import { formikRenderer } from '~/unit-test-utils';
import { formikRenderer } from '~/unit-test-utils';
import BugRFESection from '../components/BugRFESection';
import { FeedbackSections } from '../consts';
🧰 Tools
🪛 ESLint

[error] 3-3: ~/unit-test-utils import should occur before import of ../components/BugRFESection

(import/order)

🤖 Prompt for AI Agents
In `@src/components/FeedbackSection/__tests__/BugRFESection.spec.tsx` around lines
1 - 3, Reorder the import statements so the absolute import comes before
relative imports: move the import of formikRenderer (from '~/unit-test-utils')
above the imports of BugRFESection and FeedbackSections; ensure the import list
reads with the absolute module first, then the relative module imports
(references: formikRenderer, BugRFESection, FeedbackSections).


describe('BugRFESection', () => {
it('should show BugForm when current Section is BugSection', () => {
const screen = formikRenderer(<BugRFESection currentSection={FeedbackSections.BugSection} />);
screen.getByText('Report a bug');
screen.getByText(
'Describe the bug you encountered. For urgent issues, use #konflux-user-forum instead',
);
screen.getByText('Title');
screen.getByText('Please provide detailed description of the bug');
});

it('should show RFEForm when current Section is FeatureSection', () => {
const screen = formikRenderer(
<BugRFESection currentSection={FeedbackSections.FeatureSection} />,
);
screen.getByText('Request a new feature');
screen.getByText(/Please provide detailed description of the feature/);
screen.getByText('Title');
screen.getByText(/Please provide detailed description of the bug/);
});
Comment on lines +16 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, find and display the BugRFESection component
fd 'BugRFESection.tsx' --exclude '__tests__' -x cat {}

Repository: konflux-ci/konflux-ui

Length of output: 2882


🏁 Script executed:

#!/bin/bash
# Also examine the full test file to understand the context
fd 'BugRFESection.spec.tsx' -x wc -l {} && fd 'BugRFESection.spec.tsx' -x cat {}

Repository: konflux-ci/konflux-ui

Length of output: 1191


Remove the incorrect assertion on line 23.

The test asserts 'Please provide detailed description of the bug' in the FeatureSection test, but the component renders 'Please provide detailed description of the feature' when currentSection === FeedbackSections.FeatureSection. This assertion will fail or is a leftover copy-paste error from the BugSection test. Delete line 23 or replace it with the correct feature-related assertion.

🤖 Prompt for AI Agents
In `@src/components/FeedbackSection/__tests__/BugRFESection.spec.tsx` around lines
16 - 24, In the BugRFESection.spec.tsx test for "should show RFEForm when
current Section is FeatureSection", remove the incorrect assertion that checks
for 'Please provide detailed description of the bug' and either delete that line
or replace it with the correct feature text assertion (matching the component
output when currentSection === FeedbackSections.FeatureSection, e.g., the
feature description string rendered by BugRFESection); target the test block
that renders <BugRFESection currentSection={FeedbackSections.FeatureSection} />
to make the correction.

});
19 changes: 19 additions & 0 deletions src/components/FeedbackSection/__tests__/FeedbackModal.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { render } from '@testing-library/react';
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Locate and examine FeedbackModal component
fd 'FeedbackModal\.tsx' --exclude '__tests__' --type f

Repository: konflux-ci/konflux-ui

Length of output: 113


🏁 Script executed:

#!/bin/bash
# Check the test file content
cat -n src/components/FeedbackSection/__tests__/FeedbackModal.spec.tsx

Repository: konflux-ci/konflux-ui

Length of output: 871


🏁 Script executed:

#!/bin/bash
# Examine FeedbackModal component content
cat -n src/components/FeedbackSection/FeedbackModal.tsx

Repository: konflux-ci/konflux-ui

Length of output: 8315


🏁 Script executed:

#!/bin/bash
# Check what rendering utilities exist in unit-test-utils
fd 'rendering-utils\|test.*utils' --type f | grep -E 'unit-test-utils|test-utils'

Repository: konflux-ci/konflux-ui

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Look for formikRenderer usage in existing tests
rg 'formikRenderer|renderWithQueryClientAndRouter|renderWithQueryClient' --type tsx --type ts -A 2

Repository: konflux-ci/konflux-ui

Length of output: 92


🏁 Script executed:

#!/bin/bash
# Find unit-test-utils directory and rendering utilities
find . -path '*unit-test-utils*' -name '*.ts' -o -path '*unit-test-utils*' -name '*.tsx' | head -20

Repository: konflux-ci/konflux-ui

Length of output: 749


🏁 Script executed:

#!/bin/bash
# Search for formikRenderer and other rendering utilities
rg 'formikRenderer|renderWithQueryClientAndRouter|renderWithQueryClient' -l --max-count 20

Repository: konflux-ci/konflux-ui

Length of output: 6617


🏁 Script executed:

#!/bin/bash
# Examine the rendering-utils.tsx file
cat -n src/unit-test-utils/rendering-utils.tsx

Repository: konflux-ci/konflux-ui

Length of output: 3271


🏁 Script executed:

#!/bin/bash
# Check other FeedbackSection test files to see rendering patterns
find src/components/FeedbackSection -name '*.spec.tsx' -o -name '*.spec.ts'

Repository: konflux-ci/konflux-ui

Length of output: 259


🏁 Script executed:

#!/bin/bash
# Look at BeginningSection to see if it needs React Query or Router
cat -n src/components/FeedbackSection/components/BeginningSection.tsx

Repository: konflux-ci/konflux-ui

Length of output: 3516


🏁 Script executed:

#!/bin/bash
# Check what rendering utility is used in other FeedbackSection tests
head -20 src/components/FeedbackSection/__tests__/BeginningSection.spec.tsx

Repository: konflux-ci/konflux-ui

Length of output: 537


🏁 Script executed:

#!/bin/bash
# Check BugRFESection test
head -30 src/components/FeedbackSection/__tests__/BugRFESection.spec.tsx

Repository: konflux-ci/konflux-ui

Length of output: 1122


Use formikRenderer from ~/unit-test-utils to wrap FeedbackModal.

FeedbackModal wraps its content in a <Formik> context (lines 67–92). Child components like BugRFESection, which depend on Formik context, require this wrapper. The BugRFESection test correctly uses formikRenderer from ~/unit-test-utils/rendering-utils, but FeedbackModal's test does not. For consistency and proper context setup, use formikRenderer instead of bare render:

import { formikRenderer } from '~/unit-test-utils';

it('should initialize with BeginingSection Cards', () => {
  const screen = formikRenderer(<FeedbackModal />);
  // ... rest of test
});
🤖 Prompt for AI Agents
In `@src/components/FeedbackSection/__tests__/FeedbackModal.spec.tsx` at line 1,
Replace the bare render call with the shared Formik-aware renderer so
FeedbackModal tests run inside a Formik context: import and use formikRenderer
from '~/unit-test-utils' (like the BugRFESection tests do) when rendering
<FeedbackModal /> in FeedbackModal.spec.tsx so components nested in Formik
(FeedbackModal, BugRFESection, BeginingSection, etc.) receive the Formik context
during the test.

import FeedbackModal from '../FeedbackModal';

jest.mock('~/hooks/useKonfluxPublicInfo', () => ({
useKonfluxPublicInfo: jest.fn(() => [{ visibility: 'private' }]),
}));

describe('FeedbackModal', () => {
it('should initialize with BeginingSection Cards', () => {
const screen = render(<FeedbackModal />);
screen.getByText('Share feedback');
screen.getByText('Report a bug');
screen.getByText('Request a new feature');
Comment on lines +9 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Typo in test name and inconsistent assertion style.

  1. "BeginingSection" → "BeginningSection" (missing 'n').
  2. Lines 11-13 rely on implicit throw-on-miss from getByText, while line 17 wraps the query in expect(...).toBeInTheDocument(). Pick one style consistently — the explicit expect form is preferred for readability.
Proposed fix
-  it('should initialize with BeginingSection Cards', () => {
-    const screen = render(<FeedbackModal />);
-    screen.getByText('Share feedback');
-    screen.getByText('Report a bug');
-    screen.getByText('Request a new feature');
+  it('should initialize with BeginningSection Cards', () => {
+    const { getByText } = render(<FeedbackModal />);
+    expect(getByText('Share feedback')).toBeInTheDocument();
+    expect(getByText('Report a bug')).toBeInTheDocument();
+    expect(getByText('Request a new feature')).toBeInTheDocument();
📝 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.

Suggested change
it('should initialize with BeginingSection Cards', () => {
const screen = render(<FeedbackModal />);
screen.getByText('Share feedback');
screen.getByText('Report a bug');
screen.getByText('Request a new feature');
it('should initialize with BeginningSection Cards', () => {
const { getByText } = render(<FeedbackModal />);
expect(getByText('Share feedback')).toBeInTheDocument();
expect(getByText('Report a bug')).toBeInTheDocument();
expect(getByText('Request a new feature')).toBeInTheDocument();
🤖 Prompt for AI Agents
In `@src/components/FeedbackSection/__tests__/FeedbackModal.spec.tsx` around lines
9 - 13, Rename the test "should initialize with BeginingSection Cards" to
"should initialize with BeginningSection Cards" and make the assertions
consistent by replacing raw getByText calls with explicit expect assertions; in
the "FeedbackModal" test use render(<FeedbackModal />) as before, but change
screen.getByText('...') to expect(screen.getByText('Share
feedback')).toBeInTheDocument(), expect(screen.getByText('Report a
bug')).toBeInTheDocument(), and expect(screen.getByText('Request a new
feature')).toBeInTheDocument() so all assertions use the explicit
expect(...).toBeInTheDocument() style.

});
it('should have a Cancel button', () => {
const screen = render(<FeedbackModal />);
expect(screen.getByRole('button', { name: /Cancel/i })).toBeInTheDocument();
});
});
Loading
Loading